Skip to content
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

useId not stable after upgrade to Remix 2.4.1 (from 2.1) introduced with fetcher key feature. #8393

Closed
Phoenixmatrix opened this issue Dec 28, 2023 · 11 comments · Fixed by remix-run/react-router#11166
Labels
bug Something isn't working feat:routing

Comments

@Phoenixmatrix
Copy link

Phoenixmatrix commented Dec 28, 2023

Reproduction

Edit: I was able to repro using a default Remix template. See repro here: https://github.com/Phoenixmatrix/remix-unstable-useid

https://github.com/Phoenixmatrix/remix-unstable-useid

Repro steps if not using the above repo:

  • Create a project using a default Remix template (I used the Express one)
  • Create a new route
  • In the route, use useId and useFetcher
  • create a new component
  • In the component, use useId
  • Import and use the component from the route
  • Run the project and notice there's an hydration mismatch error.

The issue doesn't happen in older versions of Remix (only tested 2.1.X)

System Info

System:
    OS: Linux 5.15 Ubuntu 20.04 LTS (Focal Fossa)
    CPU: (20) x64 12th Gen Intel(R) Core(TM) i7-12700H
    Memory: 26.26 GB / 31.20 GB
    Container: Yes
    Shell: 3.6.1 - /usr/bin/fish
  Binaries:
    Node: 20.10.0 - /mnt/wslg/runtime-dir/fnm_multishells/379_1702670699505/bin/node
    Yarn: 1.22.19 - /mnt/wslg/runtime-dir/fnm_multishells/379_1702670699505/bin/yarn
    npm: 10.2.3 - /mnt/wslg/runtime-dir/fnm_multishells/379_1702670699505/bin/npm
    pnpm: 8.13.1 - /mnt/wslg/runtime-dir/fnm_multishells/379_1702670699505/bin/pnpm
  Browsers:
    Chrome: 117.0.5938.88

Used Package Manager

pnpm

Expected Behavior

Stable useId, like in Remix 2.1

Actual Behavior

useId isn't stable in files using useFetcher and components that use useId

@Phoenixmatrix
Copy link
Author

Ok, the monorepo is a red herring. It happens as soon as I do useId in a separate component.

@sergiodxa
Copy link
Member

sergiodxa commented Dec 28, 2023

Setup a pnpm monorepo with 2 separate packages (a Remix app, and a component library)

I’m 99% sure this is the issue, you’re most likely using two copies of React.

@Phoenixmatrix
Copy link
Author

That was my original guess too, so I put a console.log inside the useId function in React in node_modules, and the singular console.log call logged both the stables and unstable useIds on both the client and server, leading me to be fairly confident there's only one copy (I also checked with pnpm why) and went through my package lock, just in case). There's really only one copy.

I was also able to eventually repro without the monorepo setup.

I unfortunately CANNOT repro in a simple app created from the Remix template, so there's more to it unfortunately.

@Phoenixmatrix
Copy link
Author

Ok, I was able to do a minimal repro. It seems to only happen when a file has useFetcher, useId, and imports another component using useId.

Reproduced with a default Remix template and only the changes above:

https://github.com/Phoenixmatrix/remix-unstable-useid

@Phoenixmatrix
Copy link
Author

Ok, narrowed it down to a bug with the new fetcher key implementation, though I don't fully understand what's happening.

https://github.com/remix-run/react-router/blob/e4f663cec9f79150933127e53c7fdacf87eaab6e/packages/react-router-dom/index.tsx#L1504-L1505

The default fetcher key generation will behave fairly differently server side than client side. Server side the module might be a singleton (eg: in Express) while on the client side it would start over with every request.

If you actually set a key on the fetcher, the hydration mismatch no longer happen with useId, because it's now stable. Maybe because useId is interlinked with useState in some ways? The issue happens even if you don't do anything with the fetcher (just calling useFetcher causes the problem, and call it with a key resolves it. Even if the dom is exactly the same minus useId)

My assumption is that if the default fetcher key used useId (or something similar) itself, then the bug would be fixed. The singleton id generation seems to be the root cause.

@Phoenixmatrix
Copy link
Author

Ok, final update for tonight. I'm wondering if the default fetcher key shouldn't just use useId itself (maybe it doesn't because of rule of hooks and it's inside an if statement?).

If in my code I do the following, then everything works:

const key = useId();
const fetcher = useFetcher({key));

Which essentially replaces the internal singleton key generator with useId without needing to hardcode a key. No more hydration fetcher. Using this workaround for now until there's a better fix.

@Phoenixmatrix Phoenixmatrix changed the title useId not stable after upgrade to Remix 2.4.1 (from 2.1) useId not stable after upgrade to Remix 2.4.1 (from 2.1) introduced with fetcher key feature. Dec 28, 2023
@kiliman
Copy link
Collaborator

kiliman commented Dec 29, 2023

Wow. Awesome bit of detective work. I'm glad you were able to find a workaround for now. I'm sure the team will resolve this.

@mxp-qk
Copy link

mxp-qk commented Dec 29, 2023

I run into the same issue, as soon as you bring a dependency that manage id with useId internally and useFetcher in the same route the issue occurs. It was with react-aria-component in my case. As mentioned by @Phoenixmatrix, bringing useId in your component will cause the same problem.

Some additional context :

  • Navigating to the route doesn't seems to trigger the issue, loading the page does
  • Using a Form with navigate={false} was also a workaround for me instead of using useFetcher hook

Here a Stackblitz

@brophdawg11
Copy link
Contributor

Thanks for the deep dive! I'll take a look into this.

I'm wondering if the default fetcher key shouldn't just use useId itself

Unfortunately it's not this simple because React Router still supports React 17. I will see if feature detection is a possibility though.

@brophdawg11 brophdawg11 self-assigned this Jan 3, 2024
@Phoenixmatrix
Copy link
Author

Oh boy yeah. I didn't do SSR when I was on React 17, so I never thought about how to generate stable IDs dynamically like this without useId.

@brophdawg11
Copy link
Contributor

This is resolved by remix-run/react-router#11166 and will be available in the next release

@brophdawg11 brophdawg11 added the awaiting release This issue has been fixed and will be released soon label Jan 9, 2024
@brophdawg11 brophdawg11 removed their assignment Jan 9, 2024
@brophdawg11 brophdawg11 removed the awaiting release This issue has been fixed and will be released soon label Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feat:routing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants