refactor: migrate to new jsx runtime#7647
refactor: migrate to new jsx runtime#7647Daniel-Mendes wants to merge 1 commit intodecaporg:mainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request migrates the codebase from React's classic JSX runtime to the new automatic JSX runtime, following the official React guidance. This modernization removes the need to import React in every file that uses JSX and enables better integration with Emotion's CSS-in-JS functionality.
Key changes:
- Configured the new JSX runtime with automatic imports via
@babel/preset-reactand TypeScript settings - Replaced
@emotion/babel-preset-css-propwith@emotion/babel-pluginfor better Emotion integration - Removed unnecessary
Reactimports and replaced them with specific named imports (Component,createElement,Fragment,useState, etc.) where needed - Updated ESLint rules to disable checks that are no longer needed with the new JSX transform
Reviewed Changes
Copilot reviewed 139 out of 142 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Configured JSX runtime to use react-jsx with @emotion/react as import source |
| babel.config.js | Updated React preset to use automatic runtime, replaced emotion preset with plugin, added override for slate tests |
| .eslintrc.js | Disabled react/jsx-uses-react and react/react-in-jsx-scope rules for new JSX transform |
| package.json | Moved @emotion/babel-plugin to devDependencies, removed @emotion/babel-preset-css-prop, updated lint-quiet script |
| package-lock.json | Updated dependency tree to reflect package.json changes |
| packages/decap-cms/src/index.js | Changed from importing React to importing createElement directly |
| packages/decap-cms-widget-/src/.js | Removed unused React imports, replaced React.Component with Component import |
| packages/decap-cms-core/src/components/**/*.js | Replaced full React imports with specific named imports, updated Fragment usage |
| packages/decap-cms-ui-/src/.js | Similar React import optimizations across UI components |
| packages/decap-cms-backend-/src/.js | Updated authentication pages to use Fragment and Component imports |
| packages/decap-cms-backend-/-lfs-client.ts | Updated minimatch imports to use named export (correct for v7+) |
| packages/decap-cms-default-exports/src/index.js | Changed React import to namespace import to maintain export compatibility |
| test snapshots | Updated to reflect simplified SVG output from React rendering changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export class Modal extends Component { | ||
| static propTypes = { | ||
| children: PropTypes.node.isRequired, | ||
| children: PropTypes.any.isRequired, |
There was a problem hiding this comment.
The change from PropTypes.node to PropTypes.any for the children prop is too permissive. PropTypes.node is more specific and appropriate for React children, which can be strings, numbers, elements, fragments, or arrays of these. Using PropTypes.any removes type safety without a clear benefit.
Consider keeping PropTypes.node.isRequired instead of PropTypes.any.isRequired to maintain better prop type validation.
There was a problem hiding this comment.
@Daniel-Mendes I agree, is there a reason you switched to any?
| export class ErrorBoundary extends Component { | ||
| static propTypes = { | ||
| children: PropTypes.node, | ||
| children: PropTypes.any, |
There was a problem hiding this comment.
The change from PropTypes.node to PropTypes.any for the children prop is too permissive. PropTypes.node is more specific and appropriate for React children, which can be strings, numbers, elements, fragments, or arrays of these. Using PropTypes.any removes type safety without a clear benefit.
Consider keeping PropTypes.node instead of PropTypes.any to maintain better prop type validation.
packages/decap-cms-core/src/components/Editor/EditorControlPane/EditorControl.js
Show resolved
Hide resolved
| "mock:server:stop": "node -e 'require(\"./cypress/utils/mock-server\").stop()'", | ||
| "lint": "run-p -c --aggregate-output \"lint:*\"", | ||
| "lint-quiet": "run-p -c --aggregate-output \"lint:* --quiet\"", | ||
| "lint-quiet": "run-p -c --aggregate-output \"lint:*\"", |
There was a problem hiding this comment.
put back --quiet. Use lint for the non-quiet mode.
There was a problem hiding this comment.
Ahh yes, I removed it because the npm run test:all failed on windows
| width="16" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| > | ||
| <svg> |
There was a problem hiding this comment.
Why are attributes removed from svgs?
There was a problem hiding this comment.
I think it's the new @emotion/babel-plugin that doesn't copy the attributes. I need to check.
|
Thanks @Daniel-Mendes, are you still interested in getting this PR merged? When possible, could you take a look and implement the suggested changes and fix the merge conflicts? |
Summary
I followed the official instructions from the react blog: https://legacy.reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html
I tried keeping the minimum changes, but the codemod impacts a lot of files.
Test plan
I build the project and run the test:ci locally.
Checklist
Please add a
xinside each checkbox:A picture of a cute animal (not mandatory but encouraged)