archive/rsnext
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Add searchParams to leaf cache key (#47312)

Browse source 
### What?

Makes searchParams part of the cache key for dynamic rendering
responses.

### Why?

Current the cache key only includes the pathname and not the
searchParams. This causes issues in a few cases:
- Navigation to `/dashboard` then clicking a link to
`/dashboard?sort=asc` works, but then when navigating back the cache
node for `/dashboard?sort=asc` is used instead of the content for
`/dashboard`.
- Navigation between different searchParams always had to be a hard
navigation as reusing a cache node would result in the wrong result.

### How?

Changed the leaf node's name from `''` to `'__PAGE__'` so that it can be
distinguished. Then used that `__PAGE__` marker to include the
searchParams into the cache key for that leaf node in all places it's
used.

Ideally the `__PAGE__` key becomes something that can't be addressed in
the pathname, since it still has to be serializable I'm thinking a
number would be best.

Given that the server just provides the cache key and the client only
reasons about rendering the tree the current approach of stringifying
the searchParams and making that part of the cache key could be replaced
with a hash of the stringified result instead.

fix NEXT-685 ([link](https://linear.app/vercel/issue/NEXT-685))
Fixes #45026
Fixes NEXT-688
Fixes #46503
This commit is contained in:
Tim Neutkens 2023-03-20 19:57:29 +01:00 committed by GitHub
parent afd7a50a77
commit 8a4e8059ed
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 136 additions and 64 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
packages/next/src/build/webpack/loaders/next-app-loader.ts
View file

@ -177,7 +177,7 @@ async function createTreeCodeFromPath(
if (resolvedPagePath) pages.push(resolvedPagePath)
// Use '' for segment as it's the page. There can't be a segment called '' so this is the safest way to add it.
props[parallelKey] = `['', {}, {
props[parallelKey] = `['__PAGE__', {}, {
page: [() => import(/* webpackMode: "eager" */ ${JSON.stringify(
resolvedPagePath
)}), ${JSON.stringify(resolvedPagePath)}],

2
packages/next/src/client/components/navigation.ts
View file

@ -152,7 +152,7 @@ function getSelectedLayoutSegmentPath(
if (!node) return segmentPath
const segment = node[0]
const segmentValue = Array.isArray(segment) ? segment[1] : segment
if (!segmentValue) return segmentPath
if (!segmentValue || segmentValue === '__PAGE__') return segmentPath
segmentPath.push(segmentValue)

16
packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts
View file

@ -41,13 +41,12 @@ export function navigateReducer(
const {
url,
isExternalUrl,
locationSearch,
navigateType,
cache,
mutable,
forceOptimisticNavigation,
} = action
const { pathname, search, hash } = url
const { pathname, hash } = url
const href = createHrefFromUrl(url)
const pendingPush = navigateType === 'push'
@ -110,14 +109,11 @@ export function navigateReducer(
const applied = applyFlightData(state, cache, flightDataPath)
const hardNavigate =
// TODO-APP: Revisit searchParams support
search !== locationSearch ||
shouldHardNavigate(
// TODO-APP: remove ''
['', ...flightSegmentPath],
state.tree
)
const hardNavigate = shouldHardNavigate(
// TODO-APP: remove ''
['', ...flightSegmentPath],
state.tree
)
if (hardNavigate) {
cache.status = CacheStates.READY

57
packages/next/src/server/app-render/create-flight-router-state-from-loader-tree.ts Normal file
View file

@ -0,0 +1,57 @@
import { LoaderTree } from '../lib/app-dir-module'
import { FlightRouterState, Segment } from './types'
import { GetDynamicParamFromSegment } from './index'
// TODO-APP: Move __PAGE__ to a shared constant
const PAGE_SEGMENT_KEY = '__PAGE__'
export function addSearchParamsIfPageSegment(
segment: Segment,
searchParams: any
) {
const isPageSegment = segment === PAGE_SEGMENT_KEY
if (isPageSegment) {
const stringifiedQuery = JSON.stringify(searchParams)
return stringifiedQuery !== '{}'
? segment + '?' + stringifiedQuery
: segment
}
return segment
}
export function createFlightRouterStateFromLoaderTree(
[segment, parallelRoutes, { layout }]: LoaderTree,
getDynamicParamFromSegment: GetDynamicParamFromSegment,
searchParams: any,
rootLayoutIncluded = false
): FlightRouterState {
const dynamicParam = getDynamicParamFromSegment(segment)
const treeSegment = dynamicParam ? dynamicParam.treeSegment : segment
const segmentTree: FlightRouterState = [
addSearchParamsIfPageSegment(treeSegment, searchParams),
{},
]
if (!rootLayoutIncluded && typeof layout !== 'undefined') {
rootLayoutIncluded = true
segmentTree[4] = true
}
segmentTree[1] = Object.keys(parallelRoutes).reduce(
(existingValue, currentValue) => {
existingValue[currentValue] = createFlightRouterStateFromLoaderTree(
parallelRoutes[currentValue],
getDynamicParamFromSegment,
searchParams,
rootLayoutIncluded
)
return existingValue
},
{} as FlightRouterState[1]
)
return segmentTree
}

99
packages/next/src/server/app-render/index.tsx
View file

@ -12,7 +12,6 @@ import type {
import type { StaticGenerationAsyncStorage } from '../../client/components/static-generation-async-storage'
import type { RequestAsyncStorage } from '../../client/components/request-async-storage'
import type { MetadataItems } from '../../lib/metadata/resolve-metadata'
// Import builtin react directly to avoid require cache conflicts
import React from 'next/dist/compiled/react'
import ReactDOMServer from 'next/dist/compiled/react-dom/server.browser'
@ -69,9 +68,23 @@ import { getScriptNonceFromHeader } from './get-script-nonce-from-header'
import { renderToString } from './render-to-string'
import { parseAndValidateFlightRouterState } from './parse-and-validate-flight-router-state'
import { validateURL } from './validate-url'
import {
addSearchParamsIfPageSegment,
createFlightRouterStateFromLoaderTree,
} from './create-flight-router-state-from-loader-tree'
export const isEdgeRuntime = process.env.NEXT_RUNTIME === 'edge'
export type GetDynamicParamFromSegment = (
// [slug] / [[slug]] / [...slug]
segment: string
) => {
param: string
value: string | string[] | null
treeSegment: Segment
type: DynamicParamTypesShort
} | null
export async function renderToHTMLOrFlight(
req: IncomingMessage,
res: ServerResponse,
@ -217,15 +230,10 @@ export async function renderToHTMLOrFlight(
/**
* Parse the dynamic segment and return the associated value.
*/
const getDynamicParamFromSegment = (
const getDynamicParamFromSegment: GetDynamicParamFromSegment = (
// [slug] / [[slug]] / [...slug]
segment: string
): {
param: string
value: string | string[] | null
treeSegment: Segment
type: DynamicParamTypesShort
} | null => {
) => {
const segmentParam = getSegmentParam(segment)
if (!segmentParam) {
return null
@ -326,36 +334,6 @@ export async function renderToHTMLOrFlight(
return [null, metadataItems]
}
const createFlightRouterStateFromLoaderTree = (
[segment, parallelRoutes, { layout }]: LoaderTree,
rootLayoutIncluded = false
): FlightRouterState => {
const dynamicParam = getDynamicParamFromSegment(segment)
const segmentTree: FlightRouterState = [
dynamicParam ? dynamicParam.treeSegment : segment,
{},
]
if (!rootLayoutIncluded && typeof layout !== 'undefined') {
rootLayoutIncluded = true
segmentTree[4] = true
}
segmentTree[1] = Object.keys(parallelRoutes).reduce(
(existingValue, currentValue) => {
existingValue[currentValue] = createFlightRouterStateFromLoaderTree(
parallelRoutes[currentValue],
rootLayoutIncluded
)
return existingValue
},
{} as FlightRouterState[1]
)
return segmentTree
}
let defaultRevalidate: false | undefined | number = false
// Collect all server CSS imports used by this specific entry (or entries, for parallel routes).
@ -640,9 +618,12 @@ export async function renderToHTMLOrFlight(
const childProp: ChildProp = {
// Null indicates the tree is not fully rendered
current: null,
segment: childSegmentParam
? childSegmentParam.treeSegment
: childSegment,
segment: addSearchParamsIfPageSegment(
childSegmentParam
? childSegmentParam.treeSegment
: childSegment,
query
),
}
// This is turned back into an object below.
@ -683,9 +664,12 @@ export async function renderToHTMLOrFlight(
const childProp: ChildProp = {
current: <ChildComponent />,
segment: childSegmentParam
? childSegmentParam.treeSegment
: childSegment,
segment: addSearchParamsIfPageSegment(
childSegmentParam
? childSegmentParam.treeSegment
: childSegment,
query
),
}
const segmentPath = createSegmentPath(currentSegmentPath)
@ -857,6 +841,7 @@ export async function renderToHTMLOrFlight(
rootLayoutIncluded: boolean
}): Promise<FlightDataPath> => {
const [segment, parallelRoutes, components] = loaderTreeToFilter
const parallelRoutesKeys = Object.keys(parallelRoutes)
const { layout } = components
const isLayout = typeof layout !== 'undefined'
@ -881,9 +866,10 @@ export async function renderToHTMLOrFlight(
[segmentParam.param]: segmentParam.value,
}
: parentParams
const actualSegment: Segment = segmentParam
? segmentParam.treeSegment
: segment
const actualSegment: Segment = addSearchParamsIfPageSegment(
segmentParam ? segmentParam.treeSegment : segment,
query
)
/**
* Decide if the current segment is where rendering has to start.
@ -902,7 +888,11 @@ export async function renderToHTMLOrFlight(
return [
actualSegment,
// Create router state using the slice of the loaderTree
createFlightRouterStateFromLoaderTree(loaderTreeToFilter),
createFlightRouterStateFromLoaderTree(
loaderTreeToFilter,
getDynamicParamFromSegment,
query
),
// Check if one level down from the common layout has a loading component. If it doesn't only provide the router state as part of the Flight data.
isPrefetch && !Boolean(components.loading)
? null
@ -1074,7 +1064,12 @@ export async function renderToHTMLOrFlight(
? {
validateRootLayout: {
assetPrefix: renderOpts.assetPrefix,
getTree: () => createFlightRouterStateFromLoaderTree(loaderTree),
getTree: () =>
createFlightRouterStateFromLoaderTree(
loaderTree,
getDynamicParamFromSegment,
query
),
},
}
: {}
@ -1101,7 +1096,11 @@ export async function renderToHTMLOrFlight(
asNotFound: props.asNotFound,
})
const initialTree = createFlightRouterStateFromLoaderTree(loaderTree)
const initialTree = createFlightRouterStateFromLoaderTree(
loaderTree,
getDynamicParamFromSegment,
query
)
return (
<>

6
packages/next/src/server/dev/on-demand-entry-handler.ts
View file

@ -121,9 +121,11 @@ function getEntrypointsFromTree(
? convertDynamicParamTypeToSyntax(segment[2], segment[0])
: segment
const currentPath = [...parentPath, currentSegment]
const isPageSegment = currentSegment.startsWith('__PAGE__')
if (!isFirst && currentSegment === '') {
const currentPath = [...parentPath, isPageSegment ? '' : currentSegment]
if (!isFirst && isPageSegment) {
// TODO get rid of '' at the start of tree
return [treePathToEntrypoint(currentPath.slice(1))]
}

18
test/e2e/app-dir/app/app/navigation/searchparams/page.js Normal file
View file

@ -0,0 +1,18 @@
import Link from 'next/link'
export default function Page({ searchParams }) {
return (
<>
<h1 id="result">{JSON.stringify(searchParams)}</h1>
<div>
<Link href="/navigation/searchparams?a=a">To A</Link>
</div>
<div>
<Link href="/navigation/searchparams?b=b">To B</Link>
</div>
<div>
<Link href="/navigation/searchparams?a=a&b=b">To A&B</Link>
</div>
</>
)
}