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 navigation applying stale data when triggered from global not found #62033

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { FlightRouterState } from '../../../server/app-render/types'
import { GLOBAL_NOT_FOUND_SEGMENT_KEY } from '../../../shared/lib/segment'

export function isNavigatingToNewRootLayout(
currentTree: FlightRouterState,
Expand All @@ -7,6 +8,10 @@ export function isNavigatingToNewRootLayout(
// Compare segments
const currentTreeSegment = currentTree[0]
const nextTreeSegment = nextTree[0]

// We currently special-case the global not found segment key, but we don't want it to be treated as a root layout change
if (currentTreeSegment === GLOBAL_NOT_FOUND_SEGMENT_KEY) return false

// If any segment is different before we find the root layout, the root layout has changed.
// E.g. /same/(group1)/layout.js -> /same/(group2)/layout.js
// First segment is 'same' for both, keep looking. (group1) changed to (group2) before the root layout was found, it must have changed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ import { handleMutable } from '../handle-mutable'
import { applyFlightData } from '../apply-flight-data'
import { prefetchQueue } from './prefetch-reducer'
import { createEmptyCacheNode } from '../../app-router'
import { DEFAULT_SEGMENT_KEY } from '../../../../shared/lib/segment'
import {
DEFAULT_SEGMENT_KEY,
GLOBAL_NOT_FOUND_SEGMENT_KEY,
} from '../../../../shared/lib/segment'
import {
listenForDynamicRequest,
updateCacheNodeOnNavigation,
Expand Down Expand Up @@ -202,7 +205,10 @@ function navigateReducer_noPPR(

if (
!applied &&
prefetchEntryCacheStatus === PrefetchCacheEntryStatus.stale
(prefetchEntryCacheStatus === PrefetchCacheEntryStatus.stale ||
// if we've navigated away from the global not found segment but didn't apply the flight data, we need to refetch
// as otherwise we'd be incorrectly using the global not found cache node for the incoming page
currentTree[0] === GLOBAL_NOT_FOUND_SEGMENT_KEY)
) {
applied = addRefetchToLeafSegments(
cache,
Expand Down Expand Up @@ -454,7 +460,10 @@ function navigateReducer_PPR(

if (
!applied &&
prefetchEntryCacheStatus === PrefetchCacheEntryStatus.stale
(prefetchEntryCacheStatus === PrefetchCacheEntryStatus.stale ||
// if we've navigated away from the global not found segment but didn't apply the flight data, we need to refetch
// as otherwise we'd be incorrectly using the global not found cache node for the incoming page
currentTree[0] === GLOBAL_NOT_FOUND_SEGMENT_KEY)
) {
applied = addRefetchToLeafSegments(
cache,
Expand Down
11 changes: 11 additions & 0 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ import {
usedDynamicAPIs,
createPostponedAbortSignal,
} from './dynamic-rendering'
import { GLOBAL_NOT_FOUND_SEGMENT_KEY } from '../../shared/lib/segment'

export type GetDynamicParamFromSegment = (
// [slug] / [[slug]] / [...slug]
Expand Down Expand Up @@ -405,6 +406,16 @@ async function ReactServerApp({ tree, ctx, asNotFound }: ReactServerAppProps) {
query
)

// If the page we're rendering is being treated as the global not-found page, we want to special-case
// the segment key so it doesn't collide with a page matching the same path.
// This is necessary because when rendering the global not-found, it will always be the root segment.
// If the not-found page prefetched a link to the root page, it would have the same data path
// (e.g., ['', { children: ['__PAGE__', {}] }]). Without this disambiguation, the router would interpret
// these pages as being able to share the same cache nodes, which is not the case as they render different things.
if (asNotFound) {
initialTree[0] = GLOBAL_NOT_FOUND_SEGMENT_KEY
}

const [MetadataTree, MetadataOutlet] = createMetadataComponents({
tree,
errorType: asNotFound ? 'not-found' : undefined,
Expand Down
1 change: 1 addition & 0 deletions packages/next/src/shared/lib/segment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ export function isGroupSegment(segment: string) {

export const PAGE_SEGMENT_KEY = '__PAGE__'
export const DEFAULT_SEGMENT_KEY = '__DEFAULT__'
export const GLOBAL_NOT_FOUND_SEGMENT_KEY = '__NOT_FOUND__'
19 changes: 19 additions & 0 deletions test/e2e/app-dir/prefetching-not-found/app/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// we want the layout to opt-out of static prefetching
export const dynamic = 'force-dynamic'

import Link from 'next/link'

export default function RootLayout({
children,
}: {
children: React.ReactNode
}) {
return (
<html lang="en">
<body>
<Link href="/">Link to `/`</Link>
<div>{children}</div>
</body>
</html>
)
}
10 changes: 10 additions & 0 deletions test/e2e/app-dir/prefetching-not-found/app/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import Link from 'next/link'

export default function Home() {
return (
<div>
<h1>Home Page</h1>
<Link href="/fake-link">Go to Invalid Page</Link>
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { nextTestSetup } from 'e2e-utils'
import { retry } from 'next-test-utils'

describe('prefetching-not-found', () => {
const { next } = nextTestSetup({
files: __dirname,
})

it('should correctly navigate to/from a global 404 page when following links with prefetch=auto', async () => {
let browser = await next.browser('/')
expect(await browser.elementByCss('h1').text()).toBe('Home Page')

await browser.elementByCss("[href='/fake-link']").click()

await retry(async () => {
expect(await browser.elementByCss('body').text()).toContain(
'This page could not be found.'
)
})

await browser.elementByCss("[href='/']").click()

await retry(async () => {
expect(await browser.elementByCss('h1').text()).toBe('Home Page')
})

// assert the same behavior, but starting at the not found page. This is to ensure that when we seed the prefetch cache,
// we don't have any cache collisions that would cause the not-found page to remain rendered when following a link to the home page
browser = await next.browser('/fake-link')
expect(await browser.elementByCss('body').text()).toContain(
'This page could not be found.'
)

await browser.elementByCss("[href='/']").click()

await retry(async () => {
expect(await browser.elementByCss('h1').text()).toBe('Home Page')
})
})
})
Loading