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
Doesn't demand null
be returned
#13
Comments
This raises an interesting question. How closely should the polyfill mirror future releases of React in terms of DEV-mode warnings? I'm not convinced this is necessary, for a couple of reasons:
|
I'd say as close as possible. Isn't it the sole purpose of a "polyfill".
I'd say the polyfill would need to adapt to the latest version. Or release a new breaking version of itself, though I don't think changing warning's text would break anyone's code.
I'm not testing with new versions of React. |
Let's leave this issue open a bit and see what others think. I don't feel very strongly about this.
The purpose of this polyfill, as mentioned in the README, is to enable library authors to avoid forking their libraries for React < 16.3 and >= 16.3. DEV warnings are not required to do this. 😄
I believe you misunderstand me. Let's say I'm a library author and I want to use the new React lifecycles so that I can remove the deprecated ones before my users see any deprecation warnings. I trust that I can use these new lifecycles and this polyfill will work the same for older and newer versions of React- so people can continue to use newer my library regardless of whether they're using React 15 or 16, etc. (This is true.) This doesn't mean that I shouldn't also test my library with a newer version of React to make sure nothing else has changed or been deprecated. While doing this testing- if I return undefined from the new lifecycle- I'd see a warning. This is all I meant. The lifecycle exists to provide library maintainers more flexibility. It does not remove the necessity of testing. 😄 |
Yeah, and that's a good thing.
Well, strictly speaking it doesn't affect operation, but it would enable developers catch these "doesn't return null" bugs earlier, instead of reacting when someone posts an issue saying that there's a warning.
Ok.
You aren't ought to. |
The polyfill should match the "real thing" as close as possible IMHO, however I do agree with @bvaughn on the fact that if the wording changed in future versions there would be some kind of confusion as to what version this polyfill would be targeting/supporting. I'm sure library(at least the most used ones) authors are for sure testing against newer versions, but a regular Joe trying out the polyfill might not actually go ahead and test against the newest version, therefore not knowing about the warning. I feel like displaying a warning does not add benefit to people experienced in React, but it does for those who are maybe exploring the waters and either didn't read the docs or aren't that much familiarized with React overall. |
The current implementation of
getDerivedStateFromProps()
polyfill is not fully correct: it doesn't demandnull
be returned which is required by the official spec.Instead it should be:
The text was updated successfully, but these errors were encountered: