Skip to content

Commit

Permalink
FIX [#58788]: Fixed useParams hook undesired re-renders and updated i…
Browse files Browse the repository at this point in the history
…t to use PathParamsContext in the app router. (#60708)

Moved app-dir path params parsing logic from `useParams` to `AppRouter`.
This allows for the use of `PathParamsContext` in the `useParams` hook
for both pages and app routers. In addition, this allows for memoization
of the layout tree which fixes undesired re-renders of the `useParams`
hook.

Fixes [#58788](#58788)
Closes NEXT-2434

---------

Co-authored-by: Zack Tanner <zacktanner@gmail.com>
  • Loading branch information
florian-lp and ztanner committed Feb 14, 2024
1 parent c48a0f8 commit 7ba4a0d
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 69 deletions.
94 changes: 67 additions & 27 deletions packages/next/src/client/components/app-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import { createHrefFromUrl } from './router-reducer/create-href-from-url'
import {
SearchParamsContext,
PathnameContext,
PathParamsContext,
} from '../../shared/lib/hooks-client-context.shared-runtime'
import {
useReducerWithReduxDevtools,
Expand All @@ -64,6 +65,8 @@ import { createInfinitePromise } from './infinite-promise'
import { NEXT_RSC_UNION_QUERY } from './app-router-headers'
import { removeBasePath } from '../remove-base-path'
import { hasBasePath } from '../has-base-path'
import { PAGE_SEGMENT_KEY } from '../../shared/lib/segment'
import type { Params } from '../../shared/lib/router/utils/route-matcher'
const isServer = typeof window === 'undefined'

// Ensure the initialParallelRoutes are not combined because of double-rendering in the browser with Strict Mode.
Expand Down Expand Up @@ -98,6 +101,36 @@ export function urlToUrlWithoutFlightMarker(url: string): URL {
return urlWithoutFlightParameters
}

// this function performs a depth-first search of the tree to find the selected
// params
function getSelectedParams(
currentTree: FlightRouterState,
params: Params = {}
): Params {
const parallelRoutes = currentTree[1]

for (const parallelRoute of Object.values(parallelRoutes)) {
const segment = parallelRoute[0]
const isDynamicParameter = Array.isArray(segment)
const segmentValue = isDynamicParameter ? segment[1] : segment
if (!segmentValue || segmentValue.startsWith(PAGE_SEGMENT_KEY)) continue

// Ensure catchAll and optional catchall are turned into an array
const isCatchAll =
isDynamicParameter && (segment[2] === 'c' || segment[2] === 'oc')

if (isCatchAll) {
params[segment[0]] = segment[1].split('/')
} else if (isDynamicParameter) {
params[segment[0]] = segment[1]
}

params = getSelectedParams(parallelRoute, params)
}

return params
}

type AppRouterProps = Omit<
Omit<InitialRouterStateParameters, 'isServer' | 'location'>,
'initialParallelRoutes'
Expand Down Expand Up @@ -585,6 +618,11 @@ function Router({
return findHeadInCache(cache, tree[1])
}, [cache, tree])

// Add memoized pathParams for useParams.
const pathParams = useMemo(() => {
return getSelectedParams(tree)
}, [tree])

let head
if (matchingHead !== null) {
// The head is wrapped in an extra component so we can use
Expand Down Expand Up @@ -631,33 +669,35 @@ function Router({
appRouterState={useUnwrapState(reducerState)}
sync={sync}
/>
<PathnameContext.Provider value={pathname}>
<SearchParamsContext.Provider value={searchParams}>
<GlobalLayoutRouterContext.Provider
value={{
buildId,
changeByServerResponse,
tree,
focusAndScrollRef,
nextUrl,
}}
>
<AppRouterContext.Provider value={appRouter}>
<LayoutRouterContext.Provider
value={{
childNodes: cache.parallelRoutes,
tree,
// Root node always has `url`
// Provided in AppTreeContext to ensure it can be overwritten in layout-router
url: canonicalUrl,
}}
>
{content}
</LayoutRouterContext.Provider>
</AppRouterContext.Provider>
</GlobalLayoutRouterContext.Provider>
</SearchParamsContext.Provider>
</PathnameContext.Provider>
<PathParamsContext.Provider value={pathParams}>
<PathnameContext.Provider value={pathname}>
<SearchParamsContext.Provider value={searchParams}>
<GlobalLayoutRouterContext.Provider
value={{
buildId,
changeByServerResponse,
tree,
focusAndScrollRef,
nextUrl,
}}
>
<AppRouterContext.Provider value={appRouter}>
<LayoutRouterContext.Provider
value={{
childNodes: cache.parallelRoutes,
tree,
// Root node always has `url`
// Provided in AppTreeContext to ensure it can be overwritten in layout-router
url: canonicalUrl,
}}
>
{content}
</LayoutRouterContext.Provider>
</AppRouterContext.Provider>
</GlobalLayoutRouterContext.Provider>
</SearchParamsContext.Provider>
</PathnameContext.Provider>
</PathParamsContext.Provider>
</>
)
}
Expand Down
43 changes: 1 addition & 42 deletions packages/next/src/client/components/navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { useContext, useMemo } from 'react'
import type { FlightRouterState } from '../../server/app-render/types'
import {
AppRouterContext,
GlobalLayoutRouterContext,
LayoutRouterContext,
type AppRouterInstance,
} from '../../shared/lib/app-router-context.shared-runtime'
Expand Down Expand Up @@ -152,36 +151,6 @@ interface Params {
[key: string]: string | string[]
}

// this function performs a depth-first search of the tree to find the selected
// params
function getSelectedParams(
tree: FlightRouterState,
params: Params = {}
): Params {
const parallelRoutes = tree[1]

for (const parallelRoute of Object.values(parallelRoutes)) {
const segment = parallelRoute[0]
const isDynamicParameter = Array.isArray(segment)
const segmentValue = isDynamicParameter ? segment[1] : segment
if (!segmentValue || segmentValue.startsWith(PAGE_SEGMENT_KEY)) continue

// Ensure catchAll and optional catchall are turned into an array
const isCatchAll =
isDynamicParameter && (segment[2] === 'c' || segment[2] === 'oc')

if (isCatchAll) {
params[segment[0]] = segment[1].split('/')
} else if (isDynamicParameter) {
params[segment[0]] = segment[1]
}

params = getSelectedParams(parallelRoute, params)
}

return params
}

/**
* A [Client Component](https://nextjs.org/docs/app/building-your-application/rendering/client-components) hook
* that lets you read a route's dynamic params filled in by the current URL.
Expand All @@ -201,18 +170,8 @@ function getSelectedParams(
*/
export function useParams<T extends Params = Params>(): T {
clientHookInServerComponentError('useParams')
const globalLayoutRouter = useContext(GlobalLayoutRouterContext)
const pathParams = useContext(PathParamsContext)

return useMemo(() => {
// When it's under app router
if (globalLayoutRouter?.tree) {
return getSelectedParams(globalLayoutRouter.tree) as T
}

// When it's under client side pages router
return pathParams as T
}, [globalLayoutRouter?.tree, pathParams])
return useContext(PathParamsContext) as T
}

/** Get the canonical parameters from the current level to the leaf node. */
Expand Down
14 changes: 14 additions & 0 deletions test/e2e/app-dir/use-params/app/rerenders/[dynamic]/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
'use client'
import Link from 'next/link'
import { useParams } from 'next/navigation'
export default function Page() {
const params = useParams()

return (
<div>
<Link href="/foo">Link</Link>
<div id="random">{Math.random()}</div>
<div id="params">{JSON.stringify(params?.dynamic)}</div>
</div>
)
}
10 changes: 10 additions & 0 deletions test/e2e/app-dir/use-params/pages/pages-dir/[dynamic]/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { useParams } from 'next/navigation'
export default function Page() {
const params = useParams()

return (
<div>
<div id="params">{JSON.stringify(params?.dynamic)}</div>
</div>
)
}
14 changes: 14 additions & 0 deletions test/e2e/app-dir/use-params/use-params.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,19 @@ createNextDescribe(
expect(await browser.elementByCss('#param-id').text()).toBe('a')
expect(await browser.elementByCss('#param-id2').text()).toBe('b')
})

it('should work on pages router', async () => {
const browser = await next.browser('/pages-dir/foobar')
expect(await browser.elementById('params').text()).toBe('"foobar"')
})

it("shouldn't rerender host component when prefetching", async () => {
const browser = await next.browser('/rerenders/foobar')
const initialRandom = await browser.elementById('random').text()
const link = await browser.elementByCss('a')
await link.hover()
const newRandom = await browser.elementById('random').text()
expect(initialRandom).toBe(newRandom)
})
}
)

0 comments on commit 7ba4a0d

Please sign in to comment.