-
Notifications
You must be signed in to change notification settings - Fork 46.8k
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
Warn about rendering Generators #13312
Conversation
Could be nice to add a test that we still support generators as the iterator of an iterable. |
We've got some. react/packages/react-dom/src/__tests__/ReactMultiChildReconcile-test.js Lines 251 to 260 in 46d5afc
|
Details of bundled changes.Comparing: 46d5afc...5808caa react-dom
react-art
react-test-renderer
react-reconciler
react-native-renderer
Generated by 🚫 dangerJS |
We should also test one with a modern |
) { | ||
warning( | ||
didWarnAboutGenerators, | ||
'Using Generators as children is unsupported and will likely yield ' + |
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.
yield unexpected results
nice pun 😄
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.
Probably just https://en.m.wikipedia.org/wiki/Priming_(psychology)
It might not be obvious but by rendering a Generator you're asking React to perform a mutation during the render phase. This already doesn't work in development because we do key validation, thus consuming the generator. It led to some confusion: #11864, #12995, #11502, #7536.
Let's warn about this pattern right as we encounter it. The new warning shouldn't be noisy because the pattern is already broken in development (and as you can see in #12995 (comment), it's fragile in production).
I'm not sure how to detect generators in browsers that don't support
toStringTag
but I figured warning in modern browsers is sufficient.