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 |
| @@ -119,22 +119,18 @@ export function StaticRouterProvider({ | |||
|
|
|||
| 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
| @@ -98,6 +97,7 @@ export function StaticRouterProvider({ | |||
| 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
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