-
Notifications
You must be signed in to change notification settings - Fork 46.4k
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
Spurious context warning when getChildContext isn't pure #3709
Comments
Fixed by https://github.com/facebook/react/pull/3615/files which removes the warning entirely as part of the switch to parent context. |
Going to leave this open in case we want to do a patch release for 0.13. |
We won't take #3615 in 0.13 so it would need to be something different. |
getChildContext is supposed to be idempotent because it is recalled in the render phase but referential equality might screw up the warning even though they're idempotent - assuming value semantics. |
Yes, sorry for being unclear – the random() call was just to demonstrate; the original case here instantiated a value-like object each time with the same arguments. |
The issue is that one context comes from the owner hierarchy and one comes from the parent hierarchy. With value semantics, two distinct objects could represent the same "value" if the auxiliary data doesn't contribute to the user's interpretation of value (eg. one of the values on an object might be a logger or something). We could add some hacks to attempt to be smarter about our definition of equality, but ultimately it is going to be a futile effort to catch everything. We could also add a hack to find the component that provided the variable and bail if it was the same component, but this also has potential issues and might lead to this warning popping up in even more subtle situations. In terms of prioritization, this is a non fatal warning that only occurs in rare cases (thus we haven't seen it until now). There isn't really a great solution, and any fix would only be useful for a point release before the code is removed entirely. My intuition is that (unless a surprising number of users are hitting this), we might just let people know what's going on and teach them about making their code more idempotent. Either way, I'll update the Gist so people know what's going on. @sebmarkbage, seem sufficient, or should we patch this for a point release? |
Gist updated. |
Fix is in 0.13.3. |
@sahrens found a case where we had a component that was creating a new object each time getChildContext was called. This isn't optimal for performance because getChildContext is called for each render, but it looks like we're actually calling it twice for each render and warning if the two results aren't the same. This code
produces warnings like
The text was updated successfully, but these errors were encountered: