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

fix(react-router): improve memoization for context providers #9983

Merged
merged 7 commits into from
Mar 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/popular-timers-sneeze.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"react-router": patch
---

Improve memoization for context providers to avoid unnecessary re-renders
1 change: 1 addition & 0 deletions contributors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- alexlbr
- AmRo045
- andreiduca
- appden
- arnassavickas
- aroyan
- avipatel97
Expand Down
52 changes: 28 additions & 24 deletions packages/react-router/lib/components.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,16 @@ export function RouterProvider({
fallbackElement,
router,
}: RouterProviderProps): React.ReactElement {
let getState = React.useCallback(() => router.state, [router]);

// Sync router state to our component state to force re-renders
let state: RouterState = useSyncExternalStoreShim(
router.subscribe,
() => router.state,
getState,
// We have to provide this so React@18 doesn't complain during hydration,
// but we pass our serialized hydration data into the router so state here
// is already synced with what the server saw
() => router.state
getState
);

let navigator = React.useMemo((): Navigator => {
Expand All @@ -87,6 +89,16 @@ export function RouterProvider({

let basename = router.basename || "/";

let dataRouterContext = React.useMemo(
() => ({
router,
navigator,
static: false,
basename,
}),
[router, navigator, basename]
);

// The fragment and {null} here are important! We need them to keep React 18's
// useId happy when we are server-rendering since we may have a <script> here
// containing the hydrated server-side staticContext (from StaticRouterProvider).
Expand All @@ -95,15 +107,7 @@ export function RouterProvider({
// we don't need the <script> tag
return (
<>
<DataRouterContext.Provider
value={{
router,
navigator,
static: false,
// Do we need this?
basename,
}}
>
<DataRouterContext.Provider value={dataRouterContext}>
<DataRouterStateContext.Provider value={state}>
<Router
basename={router.basename}
Expand Down Expand Up @@ -330,39 +334,39 @@ export function Router({
key = "default",
} = locationProp;

let location = React.useMemo(() => {
let locationContext = React.useMemo(() => {
let trailingPathname = stripBasename(pathname, basename);

if (trailingPathname == null) {
return null;
}

return {
pathname: trailingPathname,
search,
hash,
state,
key,
location: {
pathname: trailingPathname,
search,
hash,
state,
key,
},
navigationType,
};
}, [basename, pathname, search, hash, state, key]);
}, [basename, pathname, search, hash, state, key, navigationType]);

warning(
location != null,
locationContext != null,
`<Router basename="${basename}"> is not able to match the URL ` +
`"${pathname}${search}${hash}" because it does not start with the ` +
`basename, so the <Router> won't render anything.`
);

if (location == null) {
if (locationContext == null) {
return null;
}

return (
<NavigationContext.Provider value={navigationContext}>
<LocationContext.Provider
children={children}
value={{ location, navigationType }}
/>
<LocationContext.Provider children={children} value={locationContext} />
</NavigationContext.Provider>
);
}
Expand Down
6 changes: 5 additions & 1 deletion packages/react-router/lib/hooks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,7 @@ enum DataRouterHook {
}

enum DataRouterStateHook {
UseBlocker = "useBlocker",
UseLoaderData = "useLoaderData",
UseActionData = "useActionData",
UseRouteError = "useRouteError",
Expand Down Expand Up @@ -841,6 +842,7 @@ let blockerId = 0;
*/
export function useBlocker(shouldBlock: boolean | BlockerFunction): Blocker {
let { router } = useDataRouterContext(DataRouterHook.UseBlocker);
let state = useDataRouterState(DataRouterStateHook.UseBlocker);
let [blockerKey] = React.useState(() => String(++blockerId));

let blockerFunction = React.useCallback<BlockerFunction>(
Expand All @@ -860,7 +862,9 @@ export function useBlocker(shouldBlock: boolean | BlockerFunction): Blocker {
[router, blockerKey]
);

return blocker;
// Prefer the blocker from state since DataRouterContext is memoized so this
// ensures we update on blocker state updates
return state.blockers.get(blockerKey) || blocker;
}

const alreadyWarned: Record<string, boolean> = {};
Expand Down