diff --git a/packages/next/src/client/components/router-reducer/create-initial-router-state.test.tsx b/packages/next/src/client/components/router-reducer/create-initial-router-state.test.tsx index dbf73ee5eb2d8..020d4e8c6319e 100644 --- a/packages/next/src/client/components/router-reducer/create-initial-router-state.test.tsx +++ b/packages/next/src/client/components/router-reducer/create-initial-router-state.test.tsx @@ -115,6 +115,7 @@ describe('createInitialRouterState', () => { lastUsedTime: expect.any(Number), treeAtTimeOfPrefetch: initialTree, status: PrefetchCacheEntryStatus.fresh, + pathname: '/linking', }, ], ]), diff --git a/packages/next/src/client/components/router-reducer/create-initial-router-state.ts b/packages/next/src/client/components/router-reducer/create-initial-router-state.ts index 7765b4a8c0514..b176f90f0452f 100644 --- a/packages/next/src/client/components/router-reducer/create-initial-router-state.ts +++ b/packages/next/src/client/components/router-reducer/create-initial-router-state.ts @@ -5,7 +5,7 @@ import { createHrefFromUrl } from './create-href-from-url' import { fillLazyItemsTillLeafWithHead } from './fill-lazy-items-till-leaf-with-head' import { extractPathFromFlightRouterState } from './compute-changed-path' import { createPrefetchCacheEntryForInitialLoad } from './prefetch-cache-utils' -import { PrefetchKind, type PrefetchCacheEntry } from './router-reducer-types' +import type { PrefetchCacheEntry } from './router-reducer-types' import { addRefreshMarkerToActiveParallelSegments } from './refetch-inactive-parallel-segments' export interface InitialRouterStateParameters { @@ -101,7 +101,6 @@ export function createInitialRouterState({ createPrefetchCacheEntryForInitialLoad({ url, - kind: PrefetchKind.AUTO, data: { f: initialFlightData, c: undefined, diff --git a/packages/next/src/client/components/router-reducer/fill-cache-with-new-subtree-data.ts b/packages/next/src/client/components/router-reducer/fill-cache-with-new-subtree-data.ts index 0ad45401304bc..19a5dde12318c 100644 --- a/packages/next/src/client/components/router-reducer/fill-cache-with-new-subtree-data.ts +++ b/packages/next/src/client/components/router-reducer/fill-cache-with-new-subtree-data.ts @@ -1,21 +1,19 @@ import type { CacheNode } from '../../../shared/lib/app-router-context.shared-runtime' -import type { - FlightDataPath, - CacheNodeSeedData, -} from '../../../server/app-render/types' +import type { FlightDataPath } from '../../../server/app-render/types' import { invalidateCacheByRouterState } from './invalidate-cache-by-router-state' import { fillLazyItemsTillLeafWithHead } from './fill-lazy-items-till-leaf-with-head' import { createRouterCacheKey } from './create-router-cache-key' import type { PrefetchCacheEntry } from './router-reducer-types' /** - * Fill cache with rsc based on flightDataPath + * Common logic for filling cache with new sub tree data. */ -export function fillCacheWithNewSubTreeData( +function fillCacheHelper( newCache: CacheNode, existingCache: CacheNode, flightDataPath: FlightDataPath, - prefetchEntry?: PrefetchCacheEntry + prefetchEntry: PrefetchCacheEntry | undefined, + fillLazyItems: boolean ): void { const isLastEntry = flightDataPath.length <= 5 const [parallelRouteKey, segment] = flightDataPath @@ -39,16 +37,17 @@ export function fillCacheWithNewSubTreeData( const existingChildCacheNode = existingChildSegmentMap.get(cacheKey) let childCacheNode = childSegmentMap.get(cacheKey) + const cacheNodeSeedData = flightDataPath[3] if (isLastEntry) { if ( - !childCacheNode || - !childCacheNode.lazyData || - childCacheNode === existingChildCacheNode + cacheNodeSeedData && + (!childCacheNode || + !childCacheNode.lazyData || + childCacheNode === existingChildCacheNode) ) { - const seedData: CacheNodeSeedData = flightDataPath[3] - const rsc = seedData[2] - const loading = seedData[3] + const rsc = cacheNodeSeedData[2] + const loading = cacheNodeSeedData[3] childCacheNode = { lazyData: null, rsc, @@ -56,28 +55,29 @@ export function fillCacheWithNewSubTreeData( head: null, prefetchHead: null, loading, - // Ensure segments other than the one we got data for are preserved. - parallelRoutes: existingChildCacheNode - ? new Map(existingChildCacheNode.parallelRoutes) - : new Map(), + parallelRoutes: + fillLazyItems && existingChildCacheNode + ? new Map(existingChildCacheNode.parallelRoutes) + : new Map(), } - if (existingChildCacheNode) { + if (existingChildCacheNode && fillLazyItems) { invalidateCacheByRouterState( childCacheNode, existingChildCacheNode, flightDataPath[2] ) } - - fillLazyItemsTillLeafWithHead( - childCacheNode, - existingChildCacheNode, - flightDataPath[2], - seedData, - flightDataPath[4], - prefetchEntry - ) + if (fillLazyItems) { + fillLazyItemsTillLeafWithHead( + childCacheNode, + existingChildCacheNode, + flightDataPath[2], + cacheNodeSeedData, + flightDataPath[4], + prefetchEntry + ) + } childSegmentMap.set(cacheKey, childCacheNode) } @@ -103,10 +103,32 @@ export function fillCacheWithNewSubTreeData( childSegmentMap.set(cacheKey, childCacheNode) } - fillCacheWithNewSubTreeData( + fillCacheHelper( childCacheNode, existingChildCacheNode, flightDataPath.slice(2), - prefetchEntry + prefetchEntry, + fillLazyItems ) } + +/** + * Fill cache with rsc based on flightDataPath + */ +export function fillCacheWithNewSubTreeData( + newCache: CacheNode, + existingCache: CacheNode, + flightDataPath: FlightDataPath, + prefetchEntry?: PrefetchCacheEntry +): void { + fillCacheHelper(newCache, existingCache, flightDataPath, prefetchEntry, true) +} + +export function fillCacheWithNewSubTreeDataButOnlyLoading( + newCache: CacheNode, + existingCache: CacheNode, + flightDataPath: FlightDataPath, + prefetchEntry?: PrefetchCacheEntry +): void { + fillCacheHelper(newCache, existingCache, flightDataPath, prefetchEntry, false) +} diff --git a/packages/next/src/client/components/router-reducer/prefetch-cache-utils.ts b/packages/next/src/client/components/router-reducer/prefetch-cache-utils.ts index 3c36c85be4488..1960fbd32dd70 100644 --- a/packages/next/src/client/components/router-reducer/prefetch-cache-utils.ts +++ b/packages/next/src/client/components/router-reducer/prefetch-cache-utils.ts @@ -1,4 +1,3 @@ -import { createHrefFromUrl } from './create-href-from-url' import { fetchServerResponse } from './fetch-server-response' import { PrefetchCacheEntryStatus, @@ -9,6 +8,16 @@ import { import { prefetchQueue } from './reducers/prefetch-reducer' import type { FetchServerResponseResult } from '../../../server/app-render/types' +const INTERCEPTION_CACHE_KEY_MARKER = '%' + +export type AliasedPrefetchCacheEntry = PrefetchCacheEntry & { + /** This is a special property that indicates a prefetch entry associated with a different URL + * was returned rather than the requested URL. This signals to the router that it should only + * apply the part that doesn't depend on searchParams (specifically the loading state). + */ + aliased?: boolean +} + /** * Creates a cache key for the router prefetch cache * @@ -16,21 +25,111 @@ import type { FetchServerResponseResult } from '../../../server/app-render/types * @param nextUrl - an internal URL, primarily used for handling rewrites. Defaults to '/'. * @return The generated prefetch cache key. */ -function createPrefetchCacheKey(url: URL, nextUrl?: string | null) { - const pathnameFromUrl = createHrefFromUrl( - url, - // Ensures the hash is not part of the cache key as it does not impact the server fetch - false - ) +function createPrefetchCacheKeyImpl( + url: URL, + includeSearchParams: boolean, + prefix?: string | null +) { + // Initially we only use the pathname as the cache key. We don't want to include + // search params so that multiple URLs with the same search parameter can re-use + // loading states. + let pathnameFromUrl = url.pathname + + // RSC responses can differ based on search params, specifically in the case where we aren't + // returning a partial response (ie with `PrefetchKind.AUTO`). + // In the auto case, since loading.js & layout.js won't have access to search params, + // we can safely re-use that cache entry. But for full prefetches, we should not + // re-use the cache entry as the response may differ. + if (includeSearchParams) { + // if we have a full prefetch, we can include the search param in the key, + // as we'll be getting back a full response. The server might have read the search + // params when generating the full response. + pathnameFromUrl += url.search + } - // nextUrl is used as a cache key delimiter since entries can vary based on the Next-URL header - if (nextUrl) { - return `${nextUrl}%${pathnameFromUrl}` + if (prefix) { + return `${prefix}${INTERCEPTION_CACHE_KEY_MARKER}${pathnameFromUrl}` } return pathnameFromUrl } +function createPrefetchCacheKey( + url: URL, + kind: PrefetchKind | undefined, + nextUrl?: string | null +) { + return createPrefetchCacheKeyImpl(url, kind === PrefetchKind.FULL, nextUrl) +} + +function getExistingCacheEntry( + url: URL, + kind: PrefetchKind = PrefetchKind.TEMPORARY, + nextUrl: string | null, + prefetchCache: Map +): AliasedPrefetchCacheEntry | undefined { + // We first check if there's a more specific interception route prefetch entry + // This is because when we detect a prefetch that corresponds with an interception route, we prefix it with nextUrl (see `createPrefetchCacheKey`) + // to avoid conflicts with other pages that may have the same URL but render different things depending on the `Next-URL` header. + for (const maybeNextUrl of [nextUrl, null]) { + const cacheKeyWithParams = createPrefetchCacheKeyImpl( + url, + true, + maybeNextUrl + ) + const cacheKeyWithoutParams = createPrefetchCacheKeyImpl( + url, + false, + maybeNextUrl + ) + + // First, we check if we have a cache entry that exactly matches the URL + const cacheKeyToUse = url.search + ? cacheKeyWithParams + : cacheKeyWithoutParams + + if (prefetchCache.has(cacheKeyToUse)) { + return prefetchCache.get(cacheKeyToUse) + } + + // If the request contains search params, and we're not doing a full prefetch, we can return the + // param-less entry if it exists. + // This is technically covered by the check at the bottom of this function, which iterates over cache entries, + // but lets us arrive there quicker in the param-full case. + const entryWithoutParams = prefetchCache.get(cacheKeyWithoutParams) + if ( + url.search && + kind !== PrefetchKind.FULL && + entryWithoutParams && + // We shouldn't return the aliased entry if it was relocated to a new cache key. + // Since it's rewritten, it could respond with a completely different loading state. + !entryWithoutParams.key.includes(INTERCEPTION_CACHE_KEY_MARKER) + ) { + return { ...entryWithoutParams, aliased: true } + } + } + + // If we've gotten to this point, we didn't find a specific cache entry that matched + // the request URL. + // We attempt a partial match by checking if there's a cache entry with the same pathname. + // Regardless of what we find, since it doesn't correspond with the requested URL, we'll mark it "aliased". + // This will signal to the router that it should only apply the loading state on the prefetched data. + if (kind !== PrefetchKind.FULL) { + for (const cacheEntry of prefetchCache.values()) { + if ( + cacheEntry.pathname === url.pathname && + // We shouldn't return the aliased entry if it was relocated to a new cache key. + // Since it's rewritten, it could respond with a completely different loading state. + !cacheEntry.key.includes(INTERCEPTION_CACHE_KEY_MARKER) + ) { + return { ...cacheEntry, aliased: true } + } + } + } + + return undefined +} + /** * Returns a prefetch cache entry if one exists. Otherwise creates a new one and enqueues a fetch request * to retrieve the prefetch data from the server. @@ -48,24 +147,13 @@ export function getOrCreatePrefetchCacheEntry({ > & { url: URL kind?: PrefetchKind -}): PrefetchCacheEntry { - let existingCacheEntry: PrefetchCacheEntry | undefined = undefined - // We first check if there's a more specific interception route prefetch entry - // This is because when we detect a prefetch that corresponds with an interception route, we prefix it with nextUrl (see `createPrefetchCacheKey`) - // to avoid conflicts with other pages that may have the same URL but render different things depending on the `Next-URL` header. - const interceptionCacheKey = createPrefetchCacheKey(url, nextUrl) - const interceptionData = prefetchCache.get(interceptionCacheKey) - - if (interceptionData) { - existingCacheEntry = interceptionData - } else { - // If we dont find a more specific interception route prefetch entry, we check for a regular prefetch entry - const prefetchCacheKey = createPrefetchCacheKey(url) - const prefetchData = prefetchCache.get(prefetchCacheKey) - if (prefetchData) { - existingCacheEntry = prefetchData - } - } +}): AliasedPrefetchCacheEntry { + const existingCacheEntry = getExistingCacheEntry( + url, + kind, + nextUrl, + prefetchCache + ) if (existingCacheEntry) { // Grab the latest status of the cache entry and update it @@ -120,18 +208,23 @@ function prefixExistingPrefetchCacheEntry({ url, nextUrl, prefetchCache, + existingCacheKey, }: Pick & { url: URL + existingCacheKey: string }) { - const existingCacheKey = createPrefetchCacheKey(url) const existingCacheEntry = prefetchCache.get(existingCacheKey) if (!existingCacheEntry) { // no-op -- there wasn't an entry to move return } - const newCacheKey = createPrefetchCacheKey(url, nextUrl) - prefetchCache.set(newCacheKey, existingCacheEntry) + const newCacheKey = createPrefetchCacheKey( + url, + existingCacheEntry.kind, + nextUrl + ) + prefetchCache.set(newCacheKey, { ...existingCacheEntry, key: newCacheKey }) prefetchCache.delete(existingCacheKey) return newCacheKey @@ -145,17 +238,18 @@ export function createPrefetchCacheEntryForInitialLoad({ tree, prefetchCache, url, - kind, data, }: Pick & { url: URL - kind: PrefetchKind data: FetchServerResponseResult }) { + // The initial cache entry technically includes full data, but it isn't explicitly prefetched -- we just seed the + // prefetch cache so that we can skip an extra prefetch request later, since we already have the data. + const kind = PrefetchKind.AUTO // if the prefetch corresponds with an interception route, we use the nextUrl to prefix the cache key const prefetchCacheKey = data.i - ? createPrefetchCacheKey(url, nextUrl) - : createPrefetchCacheKey(url) + ? createPrefetchCacheKey(url, kind, nextUrl) + : createPrefetchCacheKey(url, kind) const prefetchEntry = { treeAtTimeOfPrefetch: tree, @@ -165,7 +259,8 @@ export function createPrefetchCacheEntryForInitialLoad({ lastUsedTime: Date.now(), key: prefetchCacheKey, status: PrefetchCacheEntryStatus.fresh, - } + pathname: url.pathname, + } satisfies PrefetchCacheEntry prefetchCache.set(prefetchCacheKey, prefetchEntry) @@ -189,7 +284,7 @@ function createLazyPrefetchEntry({ url: URL kind: PrefetchKind }): PrefetchCacheEntry { - const prefetchCacheKey = createPrefetchCacheKey(url) + const prefetchCacheKey = createPrefetchCacheKey(url, kind) // initiates the fetch request for the prefetch and attaches a listener // to the promise to update the prefetch cache entry when the promise resolves (if necessary) @@ -209,6 +304,7 @@ function createLazyPrefetchEntry({ // Determine if we need to prefix the cache key with the nextUrl newCacheKey = prefixExistingPrefetchCacheEntry({ url, + existingCacheKey: prefetchCacheKey, nextUrl, prefetchCache, }) @@ -239,6 +335,7 @@ function createLazyPrefetchEntry({ lastUsedTime: null, key: prefetchCacheKey, status: PrefetchCacheEntryStatus.fresh, + pathname: url.pathname, } prefetchCache.set(prefetchCacheKey, prefetchEntry) @@ -282,14 +379,14 @@ function getPrefetchEntryCacheStatus({ // For "auto" prefetching, we'll re-use only the loading boundary for up to `static` staletime window. // A stale entry will only re-use the `loading` boundary, not the full data. // This will trigger a "lazy fetch" for the full data. - if (kind === 'auto') { + if (kind === PrefetchKind.AUTO) { if (Date.now() < prefetchTime + STATIC_STALETIME_MS) { return PrefetchCacheEntryStatus.stale } } // for "full" prefetching, we'll re-use the cache entry data for up to `static` staletime window. - if (kind === 'full') { + if (kind === PrefetchKind.FULL) { if (Date.now() < prefetchTime + STATIC_STALETIME_MS) { return PrefetchCacheEntryStatus.reusable } diff --git a/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts b/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts index c27b2ba3c2d84..d28638e4eef45 100644 --- a/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts +++ b/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts @@ -30,6 +30,7 @@ import { prunePrefetchCache, } from '../prefetch-cache-utils' import { clearCacheNodeDataForSegmentPath } from '../clear-cache-node-data-for-segment-path' +import { fillCacheWithNewSubTreeDataButOnlyLoading } from '../fill-cache-with-new-subtree-data' export function handleExternalUrl( state: ReadonlyReducerState, @@ -209,7 +210,35 @@ function navigateReducer_noPPR( const cache: CacheNode = createEmptyCacheNode() let applied = false - if ( + // The prefetch cache entry was aliased -- this signals that we only fill in the cache with the + // loading state and not the actual parallel route seed data. + if (prefetchValues.aliased) { + // Root render + if (flightDataPath.length === 3) { + // Fill in the cache with the new loading / rsc data + const cacheNodeSeedData = flightDataPath[1] + const rsc = cacheNodeSeedData[2] + const loading = cacheNodeSeedData[3] + cache.loading = loading + cache.rsc = rsc + } else { + // Copy rsc for the root node of the cache. + cache.rsc = currentCache.rsc + cache.prefetchRsc = currentCache.prefetchRsc + cache.loading = currentCache.loading + cache.parallelRoutes = new Map(currentCache.parallelRoutes) + + // recursively fill in `rsc` and `loading` but skip everything else + fillCacheWithNewSubTreeDataButOnlyLoading( + cache, + currentCache, + flightDataPath, + prefetchValues + ) + } + + applied = true + } else if ( prefetchValues.status === PrefetchCacheEntryStatus.stale && !mutable.onlyHashChange && !isFirstRead @@ -406,7 +435,8 @@ function navigateReducer_PPR( // TODO: We should get rid of the else branch and do all navigations // via updateCacheNodeOnNavigation. The current structure is just // an incremental step. - flightDataPath.length === 3 + flightDataPath.length === 3 && + !prefetchValues.aliased ) { const prefetchedTree: FlightRouterState = flightDataPath[0] const seedData = flightDataPath[1] @@ -479,7 +509,35 @@ function navigateReducer_PPR( const cache: CacheNode = createEmptyCacheNode() let applied = false - if ( + // The prefetch cache entry was aliased -- this signals that we only fill in the cache with the + // loading state and not the actual parallel route seed data. + if (prefetchValues.aliased) { + // Root render + if (flightDataPath.length === 3) { + // Fill in the cache with the new loading / rsc data + const cacheNodeSeedData = flightDataPath[1] + const rsc = cacheNodeSeedData[2] + const loading = cacheNodeSeedData[3] + cache.loading = loading + cache.rsc = rsc + } else { + // Copy rsc for the root node of the cache. + cache.rsc = currentCache.rsc + cache.prefetchRsc = currentCache.prefetchRsc + cache.loading = currentCache.loading + cache.parallelRoutes = new Map(currentCache.parallelRoutes) + + // recursively fill in `rsc` and `loading` but skip everything else + fillCacheWithNewSubTreeDataButOnlyLoading( + cache, + currentCache, + flightDataPath, + prefetchValues + ) + } + + applied = true + } else if ( prefetchValues.status === PrefetchCacheEntryStatus.stale && !mutable.onlyHashChange && !isFirstRead diff --git a/packages/next/src/client/components/router-reducer/router-reducer-types.ts b/packages/next/src/client/components/router-reducer/router-reducer-types.ts index dd9a04624f1a3..909b7afbc3b52 100644 --- a/packages/next/src/client/components/router-reducer/router-reducer-types.ts +++ b/packages/next/src/client/components/router-reducer/router-reducer-types.ts @@ -207,6 +207,7 @@ export type PrefetchCacheEntry = { lastUsedTime: number | null key: string status: PrefetchCacheEntryStatus + pathname: string } export enum PrefetchCacheEntryStatus { diff --git a/test/e2e/app-dir/navigation/navigation.test.ts b/test/e2e/app-dir/navigation/navigation.test.ts index 15d7d54e81163..43167f765d548 100644 --- a/test/e2e/app-dir/navigation/navigation.test.ts +++ b/test/e2e/app-dir/navigation/navigation.test.ts @@ -178,8 +178,6 @@ describe('app dir - navigation', () => { if (isNextStart || isNextDeploy) { await browser.waitForIdleNetwork() - // there should be an RSC call for the prefetch - expect(hasRscRequest).toBe(true) } // Wait for all network requests to finish, and then initialize the flag diff --git a/test/e2e/app-dir/searchparams-reuse-loading/app/layout.tsx b/test/e2e/app-dir/searchparams-reuse-loading/app/layout.tsx new file mode 100644 index 0000000000000..00098bb74ebdc --- /dev/null +++ b/test/e2e/app-dir/searchparams-reuse-loading/app/layout.tsx @@ -0,0 +1,10 @@ +import { ReactNode, Suspense } from 'react' +export default function Root({ children }: { children: ReactNode }) { + return ( + + + {children} + + + ) +} diff --git a/test/e2e/app-dir/searchparams-reuse-loading/app/onclick-navs/version-1/page.tsx b/test/e2e/app-dir/searchparams-reuse-loading/app/onclick-navs/version-1/page.tsx new file mode 100644 index 0000000000000..3e97ed3ab2ace --- /dev/null +++ b/test/e2e/app-dir/searchparams-reuse-loading/app/onclick-navs/version-1/page.tsx @@ -0,0 +1,23 @@ +'use client' +import Link from 'next/link' +import { useRouter } from 'next/navigation' + +export default function Page() { + const router = useRouter() + return ( + <> +
+ + /search-params?id=1 (prefetch: true) + +
+ + + ) +} diff --git a/test/e2e/app-dir/searchparams-reuse-loading/app/onclick-navs/version-2/page.tsx b/test/e2e/app-dir/searchparams-reuse-loading/app/onclick-navs/version-2/page.tsx new file mode 100644 index 0000000000000..a792c3f5fca29 --- /dev/null +++ b/test/e2e/app-dir/searchparams-reuse-loading/app/onclick-navs/version-2/page.tsx @@ -0,0 +1,23 @@ +'use client' +import Link from 'next/link' +import { useRouter } from 'next/navigation' + +export default function Page() { + const router = useRouter() + return ( + <> +
+ + /search-params (prefetch: true) + +
+ + + ) +} diff --git a/test/e2e/app-dir/searchparams-reuse-loading/app/onclick-navs/version-3/page.tsx b/test/e2e/app-dir/searchparams-reuse-loading/app/onclick-navs/version-3/page.tsx new file mode 100644 index 0000000000000..1e6be2b68a1d3 --- /dev/null +++ b/test/e2e/app-dir/searchparams-reuse-loading/app/onclick-navs/version-3/page.tsx @@ -0,0 +1,23 @@ +'use client' +import Link from 'next/link' +import { useRouter } from 'next/navigation' + +export default function Page() { + const router = useRouter() + return ( + <> +
+ + /search-params?id=1 (prefetch: true) + +
+ + + ) +} diff --git a/test/e2e/app-dir/searchparams-reuse-loading/app/page.tsx b/test/e2e/app-dir/searchparams-reuse-loading/app/page.tsx new file mode 100644 index 0000000000000..4a4ee37361bab --- /dev/null +++ b/test/e2e/app-dir/searchparams-reuse-loading/app/page.tsx @@ -0,0 +1,29 @@ +import Link from 'next/link' + +export default function Page() { + return ( + <> + Go to search +
+ +
    +
  • + /search-params?id=1 +
  • +
  • + /search-params?id=2 +
  • +
  • + + /search-params?id=3 (prefetch: true) + +
  • +
  • + + /search-params (prefetch: true) + +
  • +
+ + ) +} diff --git a/test/e2e/app-dir/searchparams-reuse-loading/app/search-params/loading.tsx b/test/e2e/app-dir/searchparams-reuse-loading/app/search-params/loading.tsx new file mode 100644 index 0000000000000..1d7bf4a78bfc6 --- /dev/null +++ b/test/e2e/app-dir/searchparams-reuse-loading/app/search-params/loading.tsx @@ -0,0 +1,3 @@ +export default function Loading() { + return

Loading...

+} diff --git a/test/e2e/app-dir/searchparams-reuse-loading/app/search-params/page.tsx b/test/e2e/app-dir/searchparams-reuse-loading/app/search-params/page.tsx new file mode 100644 index 0000000000000..2de15ac420d9e --- /dev/null +++ b/test/e2e/app-dir/searchparams-reuse-loading/app/search-params/page.tsx @@ -0,0 +1,16 @@ +import Link from 'next/link' + +export default async function Page({ + searchParams, +}: { + searchParams: Record +}) { + // sleep for 500ms + await new Promise((resolve) => setTimeout(resolve, 500)) + return ( + <> +

{JSON.stringify(searchParams)}

+ Back + + ) +} diff --git a/test/e2e/app-dir/searchparams-reuse-loading/app/search/layout.tsx b/test/e2e/app-dir/searchparams-reuse-loading/app/search/layout.tsx new file mode 100644 index 0000000000000..17302534856ed --- /dev/null +++ b/test/e2e/app-dir/searchparams-reuse-loading/app/search/layout.tsx @@ -0,0 +1,13 @@ +'use client' + +import { Fragment } from 'react' +import { useSearchParams } from 'next/navigation' + +export default function SearchLayout({ + children, +}: { + children: React.ReactNode +}) { + let searchParams = useSearchParams() + return {children} +} diff --git a/test/e2e/app-dir/searchparams-reuse-loading/app/search/loading.tsx b/test/e2e/app-dir/searchparams-reuse-loading/app/search/loading.tsx new file mode 100644 index 0000000000000..c39332b81daa0 --- /dev/null +++ b/test/e2e/app-dir/searchparams-reuse-loading/app/search/loading.tsx @@ -0,0 +1,3 @@ +export default function Loading() { + return

Loading...

+} diff --git a/test/e2e/app-dir/searchparams-reuse-loading/app/search/page.tsx b/test/e2e/app-dir/searchparams-reuse-loading/app/search/page.tsx new file mode 100644 index 0000000000000..fc52f12ec9e37 --- /dev/null +++ b/test/e2e/app-dir/searchparams-reuse-loading/app/search/page.tsx @@ -0,0 +1,12 @@ +import { Search } from './search' + +export default async function Page({ searchParams }: { searchParams: any }) { + await new Promise((resolve) => setTimeout(resolve, 3000)) + + return ( +
+ +

Search Value: {searchParams.q ?? 'None'}

+
+ ) +} diff --git a/test/e2e/app-dir/searchparams-reuse-loading/app/search/search.tsx b/test/e2e/app-dir/searchparams-reuse-loading/app/search/search.tsx new file mode 100644 index 0000000000000..c50fae8db3feb --- /dev/null +++ b/test/e2e/app-dir/searchparams-reuse-loading/app/search/search.tsx @@ -0,0 +1,22 @@ +'use client' + +import { useRouter } from 'next/navigation' + +export function Search() { + let router = useRouter() + + function search(event: React.FormEvent) { + event.preventDefault() + + let input = event.currentTarget.q.value + let params = new URLSearchParams([['q', input]]) + router.push(`/search?${params}`) + } + + return ( +
+ + +
+ ) +} diff --git a/test/e2e/app-dir/searchparams-reuse-loading/next.config.js b/test/e2e/app-dir/searchparams-reuse-loading/next.config.js new file mode 100644 index 0000000000000..807126e4cf0bf --- /dev/null +++ b/test/e2e/app-dir/searchparams-reuse-loading/next.config.js @@ -0,0 +1,6 @@ +/** + * @type {import('next').NextConfig} + */ +const nextConfig = {} + +module.exports = nextConfig diff --git a/test/e2e/app-dir/searchparams-reuse-loading/searchparams-reuse-loading.test.ts b/test/e2e/app-dir/searchparams-reuse-loading/searchparams-reuse-loading.test.ts new file mode 100644 index 0000000000000..6a6dbbf764200 --- /dev/null +++ b/test/e2e/app-dir/searchparams-reuse-loading/searchparams-reuse-loading.test.ts @@ -0,0 +1,390 @@ +import { nextTestSetup } from 'e2e-utils' +import type { Route, Page } from 'playwright' + +describe('searchparams-reuse-loading', () => { + const { next, isNextDev } = nextTestSetup({ + files: __dirname, + }) + + it('should re-use the prefetched loading state when navigating to a new searchParam value', async () => { + const browser = await next.browser('/search') + await browser.waitForElementByCss('#page-content') + + // trigger a transition by submitting a new search + await browser.elementByCss('input').type('test') + await browser.elementByCss('button').click() + + const loading = await browser.waitForElementByCss('#loading') + expect(await loading.text()).toBe('Loading...') + + const searchValue = await browser.waitForElementByCss('#search-value') + expect(await searchValue.text()).toBe('Search Value: test') + + // One more time! + await browser.elementByCss('input').type('another') + await browser.elementByCss('button').click() + + const newLoading = await browser.waitForElementByCss('#loading') + expect(await newLoading.text()).toBe('Loading...') + + const newSearchValue = await browser.waitForElementByCss('#search-value') + expect(await newSearchValue.text()).toBe('Search Value: another') + }) + + // Dev doesn't perform prefetching, so this test is skipped, as it relies on intercepting + // prefetch network requests. + if (!isNextDev) { + it('should correctly return different RSC data for full prefetches with different searchParam values', async () => { + const rscRequestPromise = new Map< + string, + { resolve: () => Promise } + >() + + let interceptRequests = false + const browser = await next.browser('/', { + beforePageLoad(page: Page) { + page.route('**/search-params*', async (route: Route) => { + if (!interceptRequests) { + return route.continue() + } + + const request = route.request() + const headers = await request.allHeaders() + const url = new URL(request.url()) + const promiseKey = + url.pathname + '?id=' + url.searchParams.get('id') + + if (headers['rsc'] === '1' && !headers['next-router-prefetch']) { + // Create a promise that will be resolved by the later test code + let resolvePromise: () => void + const promise = new Promise((res) => { + resolvePromise = res + }) + + if (rscRequestPromise.has(promiseKey)) { + throw new Error('Duplicate request') + } + + rscRequestPromise.set(promiseKey, { + resolve: async () => { + await route.continue() + // wait a moment to ensure the response is received + await new Promise((res) => setTimeout(res, 500)) + resolvePromise() + }, + }) + + // Await the promise to effectively stall the request + await promise + } else { + await route.continue() + } + }) + }, + }) + + await browser.waitForIdleNetwork() + interceptRequests = true + // The first link we click is "auto" prefetched. + await browser.elementByCss('[href="/search-params?id=1"]').click() + + // We expect to click it and immediately see a loading state + expect(await browser.elementById('loading').text()).toBe('Loading...') + // We only resolve the dynamic request after we've confirmed loading exists, + // to avoid a race where the dynamic request handles the loading state instead. + let dynamicRequest = rscRequestPromise.get('/search-params?id=1') + expect(dynamicRequest).toBeDefined() + + // resolve the promise + await dynamicRequest.resolve() + dynamicRequest = undefined + + // Confirm the params are correct + const params = await browser.waitForElementByCss('#params').text() + expect(params).toBe('{"id":"1"}') + + await browser.elementByCss("[href='/']").click() + + // Do the exact same thing again, for another prefetch auto link, to ensure + // loading works as expected and we get different search params + await browser.elementByCss('[href="/search-params?id=2"]').click() + expect(await browser.elementById('loading').text()).toBe('Loading...') + dynamicRequest = rscRequestPromise.get('/search-params?id=2') + expect(dynamicRequest).toBeDefined() + + // resolve the promise + await dynamicRequest.resolve() + dynamicRequest = undefined + + const params2 = await browser.waitForElementByCss('#params').text() + expect(params2).toBe('{"id":"2"}') + + // Dev mode doesn't perform full prefetches, so this test is conditional + await browser.elementByCss("[href='/']").click() + + await browser.elementByCss('[href="/search-params?id=3"]').click() + expect(rscRequestPromise.has('/search-params?id=3')).toBe(false) + // no need to resolve any dynamic requests, as this is a full prefetch + const params3 = await browser.waitForElementByCss('#params').text() + expect(params3).toBe('{"id":"3"}') + }) + + // /search-params (full) to /search-params?id=1 (missing) + // navigation will use loading from the full prefetch + it('should re-use loading from "full" prefetch for param-full URL when navigating to param-less route', async () => { + const rscRequestPromise = new Map< + string, + { resolve: () => Promise } + >() + + let interceptRequests = false + const browser = await next.browser('/onclick-navs/version-1', { + beforePageLoad(page: Page) { + page.route('**/search-params*', async (route: Route) => { + if (!interceptRequests) { + return route.continue() + } + + const request = route.request() + const headers = await request.allHeaders() + const url = new URL(request.url()) + const promiseKey = + url.pathname + + (url.searchParams.has('id') + ? `?id=${url.searchParams.get('id')}` + : '') + + if (headers['rsc'] === '1' && !headers['next-router-prefetch']) { + // Create a promise that will be resolved by the later test code + let resolvePromise: () => void + const promise = new Promise((res) => { + resolvePromise = res + }) + + if (rscRequestPromise.has(promiseKey)) { + throw new Error('Duplicate request') + } + + rscRequestPromise.set(promiseKey, { + resolve: async () => { + await route.continue() + // wait a moment to ensure the response is received + await new Promise((res) => setTimeout(res, 500)) + resolvePromise() + }, + }) + + // Await the promise to effectively stall the request + await promise + } else { + await route.continue() + } + }) + }, + }) + + await browser.waitForIdleNetwork() + interceptRequests = true + + // The button will trigger a router.push to the search-params route + // we use a button to ensure there was no automatic prefetching of this URL + await browser.elementByCss('button').click() + + // We expect to click it and immediately see a loading state + expect(await browser.elementById('loading').text()).toBe('Loading...') + + // We only resolve the dynamic request after we've confirmed loading exists, + // to avoid a race where the dynamic request handles the loading state instead. + let dynamicRequest = rscRequestPromise.get('/search-params') + expect(dynamicRequest).toBeDefined() + + // resolve the promise + await dynamicRequest.resolve() + dynamicRequest = undefined + + // Confirm the params are correct - we navigated to a page without params so we expect an empty object + const params = await browser.waitForElementByCss('#params').text() + expect(params).toBe('{}') + + await browser.back() + + // Navigating to the prefetch: true page should not trigger a new request and should immediately render the content + await browser.elementByCss("[href='/search-params?id=1']").click() + expect(rscRequestPromise.has('/search-params?id=1')).toBe(false) + const params1 = await browser.waitForElementByCss('#params').text() + expect(params1).toBe('{"id":"1"}') + }) + + // /search-params?id=1 (full) to /search-params (missing) + // navigation will use loading from the full prefetch + it('should re-use loading from "full" prefetch for param-less URL when navigating to param-full route', async () => { + const rscRequestPromise = new Map< + string, + { resolve: () => Promise } + >() + + let interceptRequests = false + const browser = await next.browser('/onclick-navs/version-2', { + beforePageLoad(page: Page) { + page.route('**/search-params*', async (route: Route) => { + if (!interceptRequests) { + return route.continue() + } + + const request = route.request() + const headers = await request.allHeaders() + const url = new URL(request.url()) + const promiseKey = + url.pathname + + (url.searchParams.has('id') + ? `?id=${url.searchParams.get('id')}` + : '') + + if (headers['rsc'] === '1' && !headers['next-router-prefetch']) { + // Create a promise that will be resolved by the later test code + let resolvePromise: () => void + const promise = new Promise((res) => { + resolvePromise = res + }) + + if (rscRequestPromise.has(promiseKey)) { + throw new Error('Duplicate request') + } + + rscRequestPromise.set(promiseKey, { + resolve: async () => { + await route.continue() + // wait a moment to ensure the response is received + await new Promise((res) => setTimeout(res, 500)) + resolvePromise() + }, + }) + + // Await the promise to effectively stall the request + await promise + } else { + await route.continue() + } + }) + }, + }) + + await browser.waitForIdleNetwork() + interceptRequests = true + + // The button will trigger a router.push to the search-params?id=1 route + // we use a button to ensure there was no automatic prefetching of this URL + await browser.elementByCss('button').click() + + // We expect to click it and immediately see a loading state + expect(await browser.elementById('loading').text()).toBe('Loading...') + + // We only resolve the dynamic request after we've confirmed loading exists, + // to avoid a race where the dynamic request handles the loading state instead. + let dynamicRequest = rscRequestPromise.get('/search-params?id=1') + expect(dynamicRequest).toBeDefined() + + // resolve the promise + await dynamicRequest.resolve() + dynamicRequest = undefined + + // Confirm the params are correct + const params = await browser.waitForElementByCss('#params').text() + expect(params).toBe('{"id":"1"}') + + await browser.back() + + // Navigating to the prefetch: true page should not trigger a new request and should immediately render the content + await browser.elementByCss("[href='/search-params']").click() + expect(rscRequestPromise.has('/search-params')).toBe(false) + const params1 = await browser.waitForElementByCss('#params').text() + expect(params1).toBe('{}') + }) + + // /search-params?id=1 (full) to /search-params?id=2 (missing) + // navigation will use loading from the full prefetch + it('should re-use loading from "full" prefetch for param-full URL when navigating to param-full route', async () => { + const rscRequestPromise = new Map< + string, + { resolve: () => Promise } + >() + + let interceptRequests = false + const browser = await next.browser('/onclick-navs/version-3', { + beforePageLoad(page: Page) { + page.route('**/search-params*', async (route: Route) => { + if (!interceptRequests) { + return route.continue() + } + + const request = route.request() + const headers = await request.allHeaders() + const url = new URL(request.url()) + const promiseKey = + url.pathname + + (url.searchParams.has('id') + ? `?id=${url.searchParams.get('id')}` + : '') + + if (headers['rsc'] === '1' && !headers['next-router-prefetch']) { + // Create a promise that will be resolved by the later test code + let resolvePromise: () => void + const promise = new Promise((res) => { + resolvePromise = res + }) + + if (rscRequestPromise.has(promiseKey)) { + throw new Error('Duplicate request') + } + + rscRequestPromise.set(promiseKey, { + resolve: async () => { + await route.continue() + // wait a moment to ensure the response is received + await new Promise((res) => setTimeout(res, 500)) + resolvePromise() + }, + }) + + // Await the promise to effectively stall the request + await promise + } else { + await route.continue() + } + }) + }, + }) + + await browser.waitForIdleNetwork() + interceptRequests = true + + // The button will trigger a router.push to the search-params?id=2 route + // we use a button to ensure there was no automatic prefetching of this URL + await browser.elementByCss('button').click() + + // We expect to click it and immediately see a loading state + expect(await browser.elementById('loading').text()).toBe('Loading...') + + // We only resolve the dynamic request after we've confirmed loading exists, + // to avoid a race where the dynamic request handles the loading state instead. + let dynamicRequest = rscRequestPromise.get('/search-params?id=2') + expect(dynamicRequest).toBeDefined() + + // resolve the promise + await dynamicRequest.resolve() + dynamicRequest = undefined + + // Confirm the params are correct + const params = await browser.waitForElementByCss('#params').text() + expect(params).toBe('{"id":"2"}') + + await browser.back() + + // Navigating to the prefetch: true page should not trigger a new request and should immediately render the content + await browser.elementByCss("[href='/search-params?id=1']").click() + expect(rscRequestPromise.has('/search-params?id=1')).toBe(false) + const params1 = await browser.waitForElementByCss('#params').text() + expect(params1).toBe('{"id":"1"}') + }) + } +})