Double-render function components in DEV in StrictMode#14639
Closed
gaearon wants to merge 3 commits intofacebook:masterfrom
Closed
Double-render function components in DEV in StrictMode#14639gaearon wants to merge 3 commits intofacebook:masterfrom
gaearon wants to merge 3 commits intofacebook:masterfrom
Conversation
|
ReactDOM: size: 0.0%, gzip: 0.0% Details of bundled changes.Comparing: 10a7a5b...f836352 react-dom
react-art
react-native-renderer
react-test-renderer
react-reconciler
Generated by 🚫 dangerJS |
TestRenderer is built with strict mode doublerender off. We could change that but I'm not sure we want to. So I'll just flip the flag off for this test.
Contributor
|
This change might be observable in a way that breaks existing tests (e.g. if I mock a module/component and then assert on the number of renders). Not sure how common this is, and also– I think this change might still be the right thing to do in a minor. Just seemed worth considering. |
Collaborator
Author
|
We can do that but for now I'll go with #14643 instead |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@acdlite noticed we're not doing this, but we should. It's DEV-only. Function components already need to not have observable side effects — so I think this is perfectly reasonable to do in a minor.
The test changes are a bit wonky. Maybe we could abstract this away but I felt it's only a few files for now and not too bad.