-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add jsx-runtime and jsx-dev-runtime (#18299) #1
base: main
Are you sure you want to change the base?
Conversation
This PR adds the jsx-runtime and jsx-dev-runtime modules for the JSX Babel Plugin. WWW still relies on jsx/jsxs/jsxDEV from the "react" module, so once we refactor the code to point to the runtime modules we will remove jsx/jsxs/jsxDEV from the "react" module.
c5a7667
to
5efacb5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR continues the implementation of jsx-runtime and jsx-dev-runtime modules for the new JSX Babel Plugin. The key changes focus on:
- Modified
packages/react/src/ReactDebugCurrentFrame.js
to addsetCurrentlyValidatingElement
method for debugging - Updated
packages/react/src/ReactElementValidator.js
by removing JSX validation functions - Adjusted
scripts/rollup/forks.js
to support new JSX runtime modules - Added 'react/jsx-dev-runtime' to
importSideEffects
inscripts/rollup/modules.js
These changes further refine the JSX handling in React, improving performance and preparing for future optimizations. The modifications appear consistent with the PR's goals of implementing the new JSX transform while maintaining backwards compatibility.
19 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile
if (__DEV__) { | ||
currentlyValidatingElement = element; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: The __DEV__
check here is redundant since the entire block is already within an if (__DEV__)
condition.
* @param {object} props | ||
* @param {string} key | ||
*/ | ||
export function jsx(type, config, maybeKey) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: The jsx
function doesn't handle the __self
and __source
props, which are present in the ReactElement
function. This might lead to inconsistencies.
} | ||
if (hasValidRef(config)) { | ||
ref = config.ref; | ||
warnIfStringRefCannotBeAutoConverted(config); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: This warning is only called in jsxDEV
but not in jsx
. Consider if it should also be added to the jsx
function for consistency.
if (__DEV__) { | ||
const type = element.type; | ||
if (type === null || type === undefined || typeof type === 'string') { | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: This check might skip validation for string literals. Ensure this is intended behavior
); | ||
} | ||
} | ||
const element = jsxDEV(type, props, key, source, self); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: jsxDEV is used here but not imported. Ensure it's properly imported
This PR adds the jsx-runtime and jsx-dev-runtime modules for the JSX Babel Plugin. WWW still relies on jsx/jsxs/jsxDEV from the "react" module, so once we refactor the code to point to the runtime modules we will remove jsx/jsxs/jsxDEV from the "react" module.