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
Setup JSX runtime even if the file doesn't contain JSX #12479
Setup JSX runtime even if the file doesn't contain JSX #12479
Conversation
@@ -0,0 +1 @@ | |||
const foo = /*#__PURE__*/undefined.jsx("p", {}); |
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.
This is a different unrelated bug: #12467
I'm preparing another PR for it, but it's lower priority since it's not a regression.
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/34726/ |
Btw, we have this test that ensures that even if we are always setting up the runtime, we don't inject it when it's not needed: https://github.com/babel/babel/tree/main/packages/babel-plugin-transform-react-jsx/test/fixtures/nextAutoImport/no-jsx |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 2cbe36d:
|
@@ -138,136 +138,134 @@ You can set \`throwIfNamespace: false\` to bypass this warning.`, | |||
|
|||
Program: { | |||
enter(path, state) { | |||
if (hasJSX(path)) { |
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.
I'm assuming the reason why this was done originally was for performance? And that there aren't plugins that convert non-jsx stuff but that I guess isn't the case then
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.
Yeah probably for performance, but doing a deep traversal to check if there is JSX isn't going to be fast anyway.
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.
Makes me think this sort of thing shouldn't need to be set every file if it's the same for all of them (for the most part unless you use comments or overrides)?
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.
Yeah, usually you just set it once in the config file.
Please review with whitespaces diff off