Skip to content

Commit

Permalink
memoize layout router context (#64575)
Browse files Browse the repository at this point in the history
Since `AppRouterState` is promise-based (so we can `use` the values and
suspend in render), the state itself isn't stable between renders. Each
action corresponds with a new Promise object. When a link is hovered, a
prefetch action is dispatched, even if the prefetch never happens (for
example, if there's already a prefetch entry in the cache, and it
doesn't need to prefetch again). In other words, the prefetch action
will be dispatched but won't necessarily change the state.

This means that these no-op actions that don't actually change the state
values will trigger a re-render. Most of the context providers in
`AppRouter` are memoized with the exception of `LayoutRouter` context.
This PR memoizes those values so that consumers are only notified of
meaningful updates.

Fixes #63159


Closes NEXT-3127
  • Loading branch information
ztanner authored Apr 16, 2024
1 parent a9ff94f commit e9aeb9e
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 17 deletions.
40 changes: 23 additions & 17 deletions packages/next/src/client/components/app-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,27 @@ function Router({
return getSelectedParams(tree)
}, [tree])

const layoutRouterContext = useMemo(() => {
return {
childNodes: cache.parallelRoutes,
tree,
// Root node always has `url`
// Provided in AppTreeContext to ensure it can be overwritten in layout-router
url: canonicalUrl,
loading: cache.loading,
}
}, [cache.parallelRoutes, tree, canonicalUrl, cache.loading])

const globalLayoutRouterContext = useMemo(() => {
return {
buildId,
changeByServerResponse,
tree,
focusAndScrollRef,
nextUrl,
}
}, [buildId, changeByServerResponse, tree, focusAndScrollRef, nextUrl])

let head
if (matchingHead !== null) {
// The head is wrapped in an extra component so we can use
Expand Down Expand Up @@ -668,25 +689,10 @@ function Router({
<PathnameContext.Provider value={pathname}>
<SearchParamsContext.Provider value={searchParams}>
<GlobalLayoutRouterContext.Provider
value={{
buildId,
changeByServerResponse,
tree,
focusAndScrollRef,
nextUrl,
}}
value={globalLayoutRouterContext}
>
<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,
loading: cache.loading,
}}
>
<LayoutRouterContext.Provider value={layoutRouterContext}>
{content}
</LayoutRouterContext.Provider>
</AppRouterContext.Provider>
Expand Down
23 changes: 23 additions & 0 deletions test/e2e/app-dir/app-prefetch/app/with-error/error.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use client'

export default function Error({
error,
reset,
}: {
error: Error & {
digest?: string
statusCode: number
}
reset: () => void
}) {
console.log('error render')

return (
<main>
<h1>Error</h1>
<div id="random-number">Random: {Math.random()}</div>
<p>{error.message}</p>
<button onClick={reset}>Reset</button>
</main>
)
}
16 changes: 16 additions & 0 deletions test/e2e/app-dir/app-prefetch/app/with-error/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
'use client'

import { useState } from 'react'

/** Add your relevant code here for the issue to reproduce */
export default function Page() {
const [error, setError] = useState(false)
if (error) {
throw new Error('This is a test error')
}
return (
<>
<button onClick={() => setError(true)}>Throw Error</button>
</>
)
}
17 changes: 17 additions & 0 deletions test/e2e/app-dir/app-prefetch/prefetching.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,23 @@ createNextDescribe(
expect(prefetchResponse).toContain('Loading Prefetch Auto')
})

it('should not re-render error component when triggering a prefetch action', async () => {
const browser = await next.browser('/with-error')

const initialRandom = await browser
.elementByCss('button')
.click()
.waitForElementByCss('#random-number')
.text()

await browser.eval('window.next.router.prefetch("/")')

// confirm the error component was not re-rendered
expect(await browser.elementById('random-number').text()).toBe(
initialRandom
)
})

describe('dynamic rendering', () => {
describe.each(['/force-dynamic', '/revalidate-0'])('%s', (basePath) => {
it('should not re-render layout when navigating between sub-pages', async () => {
Expand Down

1 comment on commit e9aeb9e

@ijjk
Copy link
Member

@ijjk ijjk commented on e9aeb9e Apr 16, 2024

Choose a reason for hiding this comment

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

Stats from current release

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary v14.2.1 vercel/next.js canary Change
buildDuration 13.8s 25.4s ⚠️ +11.5s
buildDurationCached 7.5s 6.7s N/A
nodeModulesSize 199 MB 199 MB ⚠️ +79.1 kB
nextStartRea..uration (ms) 401ms 411ms N/A
Client Bundles (main, webpack)
vercel/next.js canary v14.2.1 vercel/next.js canary Change
2453-HASH.js gzip 31.4 kB 31.4 kB N/A
3304.HASH.js gzip 181 B 181 B
3f784ff6-HASH.js gzip 53.7 kB 53.7 kB
8299-HASH.js gzip 5.1 kB 5.1 kB N/A
framework-HASH.js gzip 45.2 kB 45.2 kB
main-app-HASH.js gzip 241 B 242 B N/A
main-HASH.js gzip 32.2 kB 29.7 kB N/A
webpack-HASH.js gzip 1.68 kB 1.68 kB N/A
Overall change 99 kB 99 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary v14.2.1 vercel/next.js canary Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary v14.2.1 vercel/next.js canary Change
_app-HASH.js gzip 196 B 197 B N/A
_error-HASH.js gzip 184 B 184 B
amp-HASH.js gzip 505 B 505 B
css-HASH.js gzip 324 B 325 B N/A
dynamic-HASH.js gzip 2.5 kB 2.5 kB N/A
edge-ssr-HASH.js gzip 258 B 258 B
head-HASH.js gzip 352 B 352 B
hooks-HASH.js gzip 370 B 371 B N/A
image-HASH.js gzip 4.27 kB 4.27 kB
index-HASH.js gzip 259 B 259 B
link-HASH.js gzip 2.67 kB 2.67 kB N/A
routerDirect..HASH.js gzip 314 B 312 B N/A
script-HASH.js gzip 386 B 386 B
withRouter-HASH.js gzip 309 B 309 B
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 6.63 kB 6.63 kB
Client Build Manifests
vercel/next.js canary v14.2.1 vercel/next.js canary Change
_buildManifest.js gzip 483 B 485 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary v14.2.1 vercel/next.js canary Change
index.html gzip 530 B 527 B N/A
link.html gzip 542 B 540 B N/A
withRouter.html gzip 525 B 522 B N/A
Overall change 0 B 0 B
Edge SSR bundle Size
vercel/next.js canary v14.2.1 vercel/next.js canary Change
edge-ssr.js gzip 95.6 kB 94.4 kB N/A
page.js gzip 3.06 kB 3.05 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary v14.2.1 vercel/next.js canary Change
middleware-b..fest.js gzip 623 B 625 B N/A
middleware-r..fest.js gzip 155 B 156 B N/A
middleware.js gzip 25.5 kB 25.5 kB N/A
edge-runtime..pack.js gzip 839 B 839 B
Overall change 839 B 839 B
Next Runtimes Overall increase ⚠️
vercel/next.js canary v14.2.1 vercel/next.js canary Change
app-page-exp...dev.js gzip 171 kB 171 kB N/A
app-page-exp..prod.js gzip 97.4 kB 97.5 kB N/A
app-page-tur..prod.js gzip 99.2 kB 99.2 kB N/A
app-page-tur..prod.js gzip 93.4 kB 93.5 kB N/A
app-page.run...dev.js gzip 144 kB 145 kB N/A
app-page.run..prod.js gzip 91.9 kB 92 kB N/A
app-route-ex...dev.js gzip 21.5 kB 21.5 kB N/A
app-route-ex..prod.js gzip 15.2 kB 15.2 kB N/A
app-route-tu..prod.js gzip 15.2 kB 15.2 kB N/A
app-route-tu..prod.js gzip 14.9 kB 14.9 kB N/A
app-route.ru...dev.js gzip 21.1 kB 21.1 kB N/A
app-route.ru..prod.js gzip 14.9 kB 14.9 kB N/A
pages-api-tu..prod.js gzip 9.55 kB 9.55 kB
pages-api.ru...dev.js gzip 9.82 kB 9.82 kB
pages-api.ru..prod.js gzip 9.55 kB 9.55 kB
pages-turbo...prod.js gzip 22.5 kB 21.4 kB N/A
pages.runtim...dev.js gzip 23.1 kB 22.1 kB N/A
pages.runtim..prod.js gzip 22.5 kB 21.4 kB N/A
server.runti..prod.js gzip 51.3 kB 51.5 kB ⚠️ +253 B
Overall change 80.2 kB 80.5 kB ⚠️ +253 B
build cache Overall increase ⚠️
vercel/next.js canary v14.2.1 vercel/next.js canary Change
0.pack gzip 1.58 MB 1.59 MB ⚠️ +11.6 kB
index.pack gzip 107 kB 107 kB N/A
Overall change 1.58 MB 1.59 MB ⚠️ +11.6 kB
Diff details
Diff for page.js

Diff too large to display

Diff for middleware.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Diff for 2453-HASH.js

Diff too large to display

Diff for main-HASH.js

Diff too large to display

Diff for app-page-exp..ntime.dev.js
failed to diff
Diff for app-page-exp..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page.runtime.dev.js
failed to diff
Diff for app-page.runtime.prod.js

Diff too large to display

Diff for app-route-ex..ntime.dev.js

Diff too large to display

Diff for app-route-ex..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route.runtime.dev.js

Diff too large to display

Diff for app-route.ru..time.prod.js

Diff too large to display

Diff for pages-turbo...time.prod.js

Diff too large to display

Diff for pages.runtime.dev.js

Diff too large to display

Diff for pages.runtime.prod.js

Diff too large to display

Diff for server.runtime.prod.js

Diff too large to display

Please sign in to comment.