-
Notifications
You must be signed in to change notification settings - Fork 46.9k
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
Deduplication of warning messages in nested updates (#11081) #11113
Deduplication of warning messages in nested updates (#11081) #11113
Conversation
Deploy preview ready! Built with commit f06d412 |
Thanks for the PR!
Can you give me more details about this? How was this problem manifesting itself? |
I wrote this component to recreate the warnings: class App extends Component {
constructor() {
super();
this.state = {
value: null,
};
}
render() {
this.setState({
value: 'test',
});
return <div />;
}
} And in the process found that this error came up:
But the execution did not stop. It was going in an infinite loop and printing 2 variants of the same error one after another. I tested it in older versions of react and this problem is not present there. Please let me know if you want me to file an issue for this @gaearon . |
I have created issue #11136 for this. Please take a look. |
Do you think you could add a separate test that verifies the warning is deduplicated? We typically do this by writing some test causing it to fire twice, and then asserting there has only been one warning call. |
This looks good to me. The test isn't super important tbh, as we don't test deduplication everywhere, and the code is pretty trivial. |
Thanks! |
@gaearon Thanks a lot Dan for merging my first PR to react ! :) |
@gaearon I have added flags for preventing duplication of both the warning messages so they print only once now.
But I noticed that the error related to 'Maximum update depth exceeded .... ' was going in infinite loop. Not sure if its an issue or I am missing something.