Port more tests to the Scheduler.unstable_yieldValue pattern and drop internal.js#18549
Conversation
Otherwise any warnings get silenced if they're deduped.
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 7e2d244:
|
Comparing the elementType doesn't work for this because it will never be the same for a simple element. This caused us to double validate these. This was covered up because in internal tests this was deduped since they shared the prop types cache but since we now inline it, it doesn't get deduped.
| 'prop', | ||
| getComponentName(outerMemoType), | ||
| ); | ||
| } |
There was a problem hiding this comment.
@gaearon This avoids checking prop-types unless it's lazy. Comparing the elementType doesn't work for this because it will never be the same for a simple memo component.
This caused us to double validate these. This was covered up because in internal tests this was deduped since they shared the prop types cache but since we now inline it, it doesn't get deduped. Since the dedupe cache is not shared by the renderer and isomorphic. Now that we're running the tests against builds, we caught this.
There was a problem hiding this comment.
This just shows how dangerous it can be to have module state in /shared/.
This rewrites all the tests that overrides debugRenderPhaseSideEffectsForStrictMode to be agnostic of if they double render. The easiest way to do that is to use Scheduler.unstable_yieldValue pattern. So I converted a bunch of
ops.push(...)to that.We also use the pattern where we store
thisin render as a side-effect that needed to be ported too.Then drops the .internal.js suffix from a few more files that don't need it anymore.
This also surfaced that we call the fake double render on setState updater on classes before the main one. This causes deduping to silence warnings. So I changed it to happen after the main one instead.