-
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
Backwards compat fix for ReactCurrentDispatcher on older react versions #14770
Conversation
f1c26c2
to
8ab0da5
Compare
8ab0da5
to
e8404c7
Compare
ReactDOM: size: 🔺+0.1%, gzip: 🔺+0.1% Details of bundled changes.Comparing: 0975ea3...6e638c9 react-dom
react-art
react-native-renderer
react-test-renderer
react-reconciler
react-debug-tools
Generated by 🚫 dangerJS |
// but PR #14548 split them out to better support the react-debug-tools package. | ||
if (!ReactSharedInternals.hasOwnProperty('ReactCurrentDispatcher')) { | ||
const {ReactCurrentOwner} = ReactSharedInternals; | ||
ReactSharedInternals.ReactCurrentDispatcher = { |
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.
Can't this just be {current: null}
? Why does it need to forward to the owner object?
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.
Because I wasn't convinced that no 16.8+ renderer would ever have a valid use case for accessing current dispatcher?
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.
e.g. react-cache uses it
Okay, I've replaced the forwarding getter/setter with |
d586b2b
to
6e638c9
Compare
We'll publish a bugfix release with this fix shortly |
Goddamn! |
@bvaughn do you think that this fix wouldn't happen if we could just test software a little bit more? |
I’m not sure if you’re genuinely asking or implying we didn’t test React. We spent weeks ironing out remaining issues in this release and at some point you just gotta publish. (You were welcome to try alphas which have been available for months, and the latest one had the same issue.) We’ll keep this particular situation in mind for the future (mismatching versions of React and ReactDOM). But we’re also humans and can make mistakes. If you don’t want our mistakes to break your code then you should be using lockfiles in a production environment. |
Potential fix for #14763.
Current owner and dispatcher used to share the same ref, but PR #14548 split them out to better support the
react-debug-tools
package. This had the unintended side effect of newer renderers incompatible with versions of thereact
package that predate this change.This PR adds a fallback property.