[V2] Upgrade to react 16 #1408
[V2] Upgrade to react 16 #1408
Conversation
|
@fsteger do you know why these tests may be failing on upgrade to React 16:
|
|
I had a look into this and it probably depends on some deprecated lifecycle methods used. I can rewrite everything to functional components using hooks as I already have some experience doing that on a large scale. Will be done by today and open a PR then. |
|
It is actually more work than expected. I'm still working on that, biggest problem is that keys were used wrong. Seems like react 15 is passing the key prop to the child although it is only for internal use. With react 16 that prop is not accessible. E.g. this is not accessible here But getting there. |
|
@zepatrik thanks for the update. What exactly is the change between react 15 and react 16 regarding keys? Is it that you can't pass on a key prop to a child? (that sounds strange, because if the key prop is accessible, then you can pass it on, right...?) |
|
I could not really find a breaking change in reacts release notes but as you can see here (also look into the console) it is not possible to access the key prop (and apparently it was before) |
|
Also |
|
What do you mean by "the derived state was not used"? We did use |
|
well because it appeared to not have been called, the state differed on the react version |
|
Here are some debug information. Logging the This is also what the first failing test detects. react 15: react 16: Steps to reproduce:
As you can see for some reason the component updates twice, once with an updated state and once with updated props. |
|
tracked that as issue #1427 |
componentWillReceiveProps is not called after setState, but getDerivedStateFromProps is called after setState. This caused the array key tests to fail, because when onAddIndexClick is called, it sets the state first to update keyedFormData. Only afterwards is the onChange handler called with the new form data. And, getDerivedStateFromProps is called in between, so it ends up being called with the old form data instead of the new one. Used the technique in https://stackoverflow.com/a/57208053 to work around this, by not calculating derived state in getDerivedStateFromProps when keyed form data is updated.
epicfaace
force-pushed
the
v2-react-upgrade
branch
from
2adb0dc to
b484511
Compare
|
Very cool ! |
* upgrade to react 16 * bump react peer dependency to 16 * remove react-lifecycles polyfill * unskip react 16 test * Fix array tests, fixes #1427 componentWillReceiveProps is not called after setState, but getDerivedStateFromProps is called after setState. This caused the array key tests to fail, because when onAddIndexClick is called, it sets the state first to update keyedFormData. Only afterwards is the onChange handler called with the new form data. And, getDerivedStateFromProps is called in between, so it ends up being called with the old form data instead of the new one. Used the technique in https://stackoverflow.com/a/57208053 to work around this, by not calculating derived state in getDerivedStateFromProps when keyed form data is updated. * Remove console.error stub to fix tests * Fix getWidget behavior for forwarded refs * Replace componentWillReceiveProps with UNSAFE_componentWillReceiveProps
Reasons for making this change
Upgrade to react 16, fixes #1275, fixes #1427 .
If this is related to existing tickets, include links to them as well.
Checklist