Fix React SRR useLayoutEffect console.error when using CompatRouter #9820
Conversation
|
|
Hi @brookslybrand, Welcome, and thank you for contributing to React Router! Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once. You may review the CLA and sign it by adding your name to contributors.yml. Once the CLA is signed, the If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at [email protected]. Thanks! - The Remix team |
| /** | ||
| * @jest-environment node | ||
| */ |
There was a problem hiding this comment.
If there is a better/preferred way to set the environment in jest, please let me know 🙂. Without this comment this test will have a false-positive, since it assumes we are in a browser environment where this isn't a problem.
| // Have to mock react-router-dom to have a comparable API to v5, otherwise it will | ||
| // be using v6's API and fail | ||
| jest.mock("react-router-dom", () => ({ | ||
| useHistory: () => ({ location: "/" }), | ||
| })); |
There was a problem hiding this comment.
Don't super love having to mock this out, but without this mock CompatRouter was trying to use useHistory from the react-router-dom version it had (v6, where it doesn't exist). Since this test is pretty simple I figured this would be fine, but let me know otherwise.
| // Copied with 💜 from https://github.com/bvaughn/react-resizable-panels/blob/main/packages/react-resizable-panels/src/hooks/useIsomorphicEffect.ts | ||
| const canUseEffectHooks = !!( | ||
| typeof window !== "undefined" && | ||
| typeof window.document !== "undefined" && | ||
| typeof window.document.createElement !== "undefined" | ||
| ); | ||
|
|
||
| const useIsomorphicLayoutEffect = canUseEffectHooks | ||
| ? React.useLayoutEffect | ||
| : () => {}; | ||
|
|
There was a problem hiding this comment.
In the words of @ryanflorence
The warning react throws is so dang stupid. We shouldn’t need this.
|
Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳 |
|
Thanks! Confirmed fthis is good with Ryan as well 👍 |
Sweet! Thanks 😄 |
|
🤖 Hello there, We just published version Thanks! |
|
🤖 Hello there, We just published version Thanks! |
Currently when using
react-router-dom-v5-compat'sCompatRouterwith SSR, you get this fun React warning in your console:This is something my team has ignored, but it's pretty obnoxious. Additionally, even Ryan believes this warning is unnecessary.
This PR simply adds a fix previous React core team member Bri implemented, as well as a test to verify that warning does not show up in a jest node environment