Ensure consistent component tree for useId #9805
Conversation
🦋 Changeset detectedLatest commit: 190c776 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
||
| return ( | ||
| <> | ||
| <DataStaticRouterContext.Provider value={context}> |
There was a problem hiding this comment.
This outer <DataStaticRouterContext> is removed in favor of a staticContext field on dataRouterContext. Easiest to see with whitespace hidden 😉
| </DataRouterStateContext.Provider> | ||
| </DataRouterContext.Provider> | ||
| {null} | ||
| </> |
There was a problem hiding this comment.
Ensure the structure generated by RouterProvider matches that generated by StaticRouterProvider
There was a problem hiding this comment.
I'm a little confused why we need the null since the docs indicate that only the parent path matters, and our useId calls would be down inside <Routes/>. But without the null I constantly ran into hydration mismatches 😕
There was a problem hiding this comment.
Mind filing a question / bug over on the react repo?
There was a problem hiding this comment.
This seems expected - it might just be a docs issue: facebook/react#22733. Added my own comment in there suggesting a docs update
| if (__DEV__) { | ||
| DataStaticRouterContext.displayName = "DataStaticRouterContext"; | ||
| } | ||
|
|
There was a problem hiding this comment.
This was an UNSAFE export so ok to remove and add onto DataRouterContext (which is also exported as UNSAFE)
| dataRouterContext.staticContext && | ||
| match.route.errorElement | ||
| ) { | ||
| dataRouterContext.staticContext._deepestRenderedBoundaryId = match.route.id; |
There was a problem hiding this comment.
This is the only reason we need staticContext around in the component tree - so we can track render depth to bubble SSR render errors
| let dataRouterContext = { | ||
| router, | ||
| navigator: getStatelessNavigator(), | ||
| static: true, |
There was a problem hiding this comment.
Can we get rid of the static prop in favor of the existence of the static context?
There was a problem hiding this comment.
Not quite yet - we still need static for non-data-routers doing SSR
Co-authored-by: Jacob Ebey <[email protected]>
We found a useId mismatch when testing Remix
1.10.0-pre.5. It came down to 2 separate issues:<DataStaticRouterContext>during SSR. This has now been collapsed into an optionalstaticContextfield on<DataRouterContext><script>/nullinStaticRouterProviderthat were not reflected inRouterProvider