-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
@babel/plugin-transform-react-constant-elements@7.13.10 broken #13051
Comments
Hey @cgood92! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly. If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite." |
This is an interaction with |
Wow, we overlooked a really simple case: function Example(){
const nodes = []
for (var i = 0; i < 10; i++) {
nodes.push(<div key={i} children={i}></div>)
}
return nodes;
}
function Example2(){
const nodes = []
let i = 0;
for (; i < 10; i++) {
nodes.push(<div key={i} children={i}></div>)
}
return nodes;
} Neither of these should be constant. @cgood92: You've marked that you'd like to fix this. It's actually really simple: When we find an
if (path.isIdentifier()) {
const binding = path.scope.getBinding(path.node.name);
if (binding.constant) return;
} |
Thank you very much for your suggestion @jridgewell. I have attempted my first PR here: #13054. Can you please give feedback if this is not what you intended? Thanks again. |
Bug Report
Current behavior
@babel/babel-plugin-transform-react-constant-elements@7.13.10 does not handle
let
variables that change correctly.Input Code
Expected behavior
In this case, this JSX should not be seen as constant
Babel Configuration (babel.config.js, .babelrc, package.json#babel, cli command, .eslintrc)
babel.config.js
Environment
Additional context
This is the commit that broke things: 8dacf85. V7.12.13 works fine.
The text was updated successfully, but these errors were encountered: