Skip to content

Stabilize router memo inputs#185

Draft
IzumiSy wants to merge 9 commits intomainfrom
refactor_route_registration
Draft

Stabilize router memo inputs#185
IzumiSy wants to merge 9 commits intomainfrom
refactor_route_registration

Conversation

@IzumiSy
Copy link
Copy Markdown
Contributor

@IzumiSy IzumiSy commented Apr 18, 2026

Summary

  • keep the current useMemo-based router creation approach
  • reduce router recreation triggers to route-defining inputs only
  • move shell rendering and auth guard wrapping out of the route definition path
  • add regression tests proving that shell children and guard wrapper changes do not recreate the router

Background

This PR is a follow-up to the auth router stability work in #184.

That earlier change made RouterContainer stable across auth-driven rerenders so the root loader would not re-process the same OAuth callback URL. The remaining concern was that the router was still being built from a mix of two different kinds of inputs:

  1. route-defining inputs such as modules, locale, basename, and root loader behavior
  2. render-time values such as shell children and the auth guard wrapper

Those render-time values are not part of the route structure itself. If they participate in router creation, then ordinary UI rerenders can still recreate the router instance even when the route definition has not meaningfully changed.

For this codebase, I did not want to introduce an external router factory API. The goal here is to keep the current design, but tighten the boundary around what actually counts as a router-defining change.

Purpose

The purpose of this change is to preserve the current RouterContainer API while making its memoization semantics more intentional.

Concretely, the router should be recreated when the route tree or loader behavior changes, but not when:

  • the shell element changes
  • the auth guard wrapper rerenders
  • ordinary React rerenders produce new render-time values

To achieve that, this PR keeps the root loader in the route definition path, but moves shell rendering and wrapper application into a separate internal React context that is consumed at render time.

Result

With this change:

  • router recreation is more closely aligned with actual route definition changes
  • auth-related UI rerenders are less likely to reset router state accidentally
  • the behavior is covered by regression tests, including shell-child updates and guard-wrapper updates

Copy link
Copy Markdown
Contributor Author

@IzumiSy IzumiSy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

The intent and the implementation are aligned and correct overall.

What the PR is doing

The change tightens the useMemo boundary in RouterContainer so that the router is only recreated when the route tree actually changes. To achieve that, children (the shell element) and wrapComponent (the auth guard wrapper) are moved out of the route definition path. Both values are now delivered to the root route element at render time via a local React context (RootRouteContext) instead of being baked into the RouteObject at creation time.

Why the approach is sound

RouteObject.element in React Router is resolved at render time, not at router-creation time — it is just a ReactNode. Putting render-time values (shell children, guard wrapper) into useMemo deps caused the router to be recreated on ordinary React rerenders that had nothing to do with route structure. The fix correctly recognises this boundary.

Key prerequisite — authLoader stability

The updated useMemo deps are [configurations, rootLoader, contentRoutes]. For this to fully solve the problem, rootLoader (i.e. authLoader from AuthProvider) must be referentially stable across rerenders that only change guardComponent.

I checked auth-context.tsx and confirmed this holds:

// auth-context.tsx
const authLoader = useCallback(
  async (requestUrl: URL) => { ... },
  [client],          // stable as long as client prop doesn't change
);

const rootRouteCtxValue = useMemo(
  () => ({ loader: authLoader, wrapComponent: guardComponentWrapper }),
  [authLoader, guardComponentWrapper],
);

authLoader depends only on client, which is expected to be module-level stable. When guardComponent changes, guardComponentWrapper changes but authLoader does not, so rootLoader stays the same reference and the router is not recreated. The regression tests verify exactly this scenario (does not recreate the router when guard wrapper changes), so the invariant is covered.

No concerns. The implementation is correct and the tests cover the intended invariants.

Base automatically changed from fix/auth-router-stability to main April 20, 2026 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant