From 51549d92de3069b6dc5cd1f5fcdc04d5b1cbf37a Mon Sep 17 00:00:00 2001 From: Zack Tanner Date: Thu, 4 Apr 2024 16:44:54 -0700 Subject: [PATCH] fix refreshing inactive segments that contained searchParams (#64086) When adding refresh markers for refetching segments that are "stale" (the soft navigation case where they are still in the Router State Tree but not part of the page that we're rendering), they only contained a reference to the pathname. Once the actual refresh was triggered we added the current search params to the request. However, this causes an issue: segments in the `FlightRouterState` can contain searchParams (ie, `__PAGE__?{"foo": "bar}` is what the segment becomes when adding `?foo=bar` to a page). This means that when we refresh those nodes from the server, it won't know how to find the `CacheNode` for the stale segment since we won't have them in the refetch tree. This updates to keep a reference to the searchParams that were part of the request that made it active. While fixing this, I also noticed that we were missing a spot to add the refetch marker in `applyRouterStatePatchToTree` in the case of a root refresh (caught by these tests failing in PPR) [x-ref](https://github.com/vercel/next.js/discussions/63900#discussioncomment-9002137) Closes NEXT-3007 --- .../apply-router-state-patch-to-tree.ts | 2 + .../reducers/fast-refresh-reducer.ts | 2 +- .../reducers/navigate-reducer.ts | 8 +- .../reducers/refresh-reducer.ts | 2 +- .../reducers/server-action-reducer.ts | 2 +- .../reducers/server-patch-reducer.ts | 2 +- .../refetch-inactive-parallel-segments.ts | 4 +- .../components/UpdateSearchParamsButton.tsx | 27 ++ .../[dynamic]/@modal/(.)login/page.tsx | 4 +- .../app/dynamic-refresh/[dynamic]/page.tsx | 4 +- .../app/refreshing/@modal/(.)login/page.tsx | 4 +- .../app/refreshing/page.tsx | 4 +- .../parallel-routes-revalidation.test.ts | 243 ++++++++++-------- test/turbopack-build-tests-manifest.json | 12 +- 14 files changed, 199 insertions(+), 121 deletions(-) create mode 100644 test/e2e/app-dir/parallel-routes-revalidation/app/components/UpdateSearchParamsButton.tsx diff --git a/packages/next/src/client/components/router-reducer/apply-router-state-patch-to-tree.ts b/packages/next/src/client/components/router-reducer/apply-router-state-patch-to-tree.ts index c3d3997301d18..7b51e3660215c 100644 --- a/packages/next/src/client/components/router-reducer/apply-router-state-patch-to-tree.ts +++ b/packages/next/src/client/components/router-reducer/apply-router-state-patch-to-tree.ts @@ -93,6 +93,8 @@ export function applyRouterStatePatchToTree( flightSegmentPath ) + addRefreshMarkerToActiveParallelSegments(tree, pathname) + return tree } diff --git a/packages/next/src/client/components/router-reducer/reducers/fast-refresh-reducer.ts b/packages/next/src/client/components/router-reducer/reducers/fast-refresh-reducer.ts index 9ccd1da6c9edb..b83414a2bb9ef 100644 --- a/packages/next/src/client/components/router-reducer/reducers/fast-refresh-reducer.ts +++ b/packages/next/src/client/components/router-reducer/reducers/fast-refresh-reducer.ts @@ -74,7 +74,7 @@ function fastRefreshReducerImpl( [''], currentTree, treePatch, - location.pathname + state.canonicalUrl ) if (newTree === null) { 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 d970d8c7fe7b7..3d0cf8b7ee230 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 @@ -176,7 +176,7 @@ function navigateReducer_noPPR( flightSegmentPathWithLeadingEmpty, currentTree, treePatch, - url.pathname + href ) // If the tree patch can't be applied to the current tree then we use the tree at time of prefetch @@ -187,7 +187,7 @@ function navigateReducer_noPPR( flightSegmentPathWithLeadingEmpty, treeAtTimeOfPrefetch, treePatch, - url.pathname + href ) } @@ -354,7 +354,7 @@ function navigateReducer_PPR( flightSegmentPathWithLeadingEmpty, currentTree, treePatch, - url.pathname + href ) // If the tree patch can't be applied to the current tree then we use the tree at time of prefetch @@ -365,7 +365,7 @@ function navigateReducer_PPR( flightSegmentPathWithLeadingEmpty, treeAtTimeOfPrefetch, treePatch, - url.pathname + href ) } diff --git a/packages/next/src/client/components/router-reducer/reducers/refresh-reducer.ts b/packages/next/src/client/components/router-reducer/reducers/refresh-reducer.ts index 11629c93e8595..1623275d8e1de 100644 --- a/packages/next/src/client/components/router-reducer/reducers/refresh-reducer.ts +++ b/packages/next/src/client/components/router-reducer/reducers/refresh-reducer.ts @@ -74,7 +74,7 @@ export function refreshReducer( [''], currentTree, treePatch, - location.pathname + state.canonicalUrl ) if (newTree === null) { diff --git a/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts b/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts index c9ccabe3f8b3e..7d8e314b848ad 100644 --- a/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts +++ b/packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts @@ -225,7 +225,7 @@ export function serverActionReducer( [''], currentTree, treePatch, - location.pathname + href ) if (newTree === null) { diff --git a/packages/next/src/client/components/router-reducer/reducers/server-patch-reducer.ts b/packages/next/src/client/components/router-reducer/reducers/server-patch-reducer.ts index 3d0d73e3f48a9..5fe183d7eaea7 100644 --- a/packages/next/src/client/components/router-reducer/reducers/server-patch-reducer.ts +++ b/packages/next/src/client/components/router-reducer/reducers/server-patch-reducer.ts @@ -48,7 +48,7 @@ export function serverPatchReducer( ['', ...flightSegmentPath], currentTree, treePatch, - location.pathname + state.canonicalUrl ) if (newTree === null) { diff --git a/packages/next/src/client/components/router-reducer/refetch-inactive-parallel-segments.ts b/packages/next/src/client/components/router-reducer/refetch-inactive-parallel-segments.ts index 192ec4eb215cb..95eb0ba806408 100644 --- a/packages/next/src/client/components/router-reducer/refetch-inactive-parallel-segments.ts +++ b/packages/next/src/client/components/router-reducer/refetch-inactive-parallel-segments.ts @@ -61,9 +61,7 @@ async function refreshInactiveParallelSegmentsImpl({ // Eagerly kick off the fetch for the refetch path & the parallel routes. This should be fine to do as they each operate // independently on their own cache nodes, and `applyFlightData` will copy anything it doesn't care about from the existing cache. const fetchPromise = fetchServerResponse( - // we capture the pathname of the refetch without search params, so that it can be refetched with - // the "latest" search params when it comes time to actually trigger the fetch (below) - new URL(refetchPathname + location.search, location.origin), + new URL(refetchPathname, location.origin), // refetch from the root of the updated tree, otherwise it will be scoped to the current segment // and might not contain the data we need to patch in interception route data (such as dynamic params from a previous segment) [rootTree[0], rootTree[1], rootTree[2], 'refetch'], diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/components/UpdateSearchParamsButton.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/components/UpdateSearchParamsButton.tsx new file mode 100644 index 0000000000000..c8e3724f07424 --- /dev/null +++ b/test/e2e/app-dir/parallel-routes-revalidation/app/components/UpdateSearchParamsButton.tsx @@ -0,0 +1,27 @@ +'use client' +import { useRouter } from 'next/navigation' + +export function UpdateSearchParamsButton({ + searchParams, + id, +}: { + searchParams: any + id?: string +}) { + const router = useRouter() + + return ( +
+
+ Params: {JSON.stringify(searchParams.random)} +
+ +
+ ) +} diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/@modal/(.)login/page.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/@modal/(.)login/page.tsx index 42f51e7bd7c7a..fd46b4c8e2513 100644 --- a/test/e2e/app-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/@modal/(.)login/page.tsx +++ b/test/e2e/app-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/@modal/(.)login/page.tsx @@ -1,9 +1,10 @@ import { RefreshButton } from '../../../../components/RefreshButton' import { RevalidateButton } from '../../../../components/RevalidateButton' +import { UpdateSearchParamsButton } from '../../../../components/UpdateSearchParamsButton' const getRandom = async () => Math.random() -export default async function Page({ params }) { +export default async function Page({ params, searchParams }) { const someProp = await getRandom() return ( @@ -16,6 +17,7 @@ export default async function Page({ params }) { + ) diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/page.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/page.tsx index 2634584a60bbe..c6411141bfd44 100644 --- a/test/e2e/app-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/page.tsx +++ b/test/e2e/app-dir/parallel-routes-revalidation/app/dynamic-refresh/[dynamic]/page.tsx @@ -1,6 +1,7 @@ import Link from 'next/link' +import { UpdateSearchParamsButton } from '../../components/UpdateSearchParamsButton' -export default function Home() { +export default function Home({ searchParams }) { return (
@@ -9,6 +10,7 @@ export default function Home() {
Random # from Root Page: {Math.random()}
+
) } diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/refreshing/@modal/(.)login/page.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/refreshing/@modal/(.)login/page.tsx index 30496df0e5a61..b681fe548b14c 100644 --- a/test/e2e/app-dir/parallel-routes-revalidation/app/refreshing/@modal/(.)login/page.tsx +++ b/test/e2e/app-dir/parallel-routes-revalidation/app/refreshing/@modal/(.)login/page.tsx @@ -1,9 +1,10 @@ import { RefreshButton } from '../../../components/RefreshButton' import { RevalidateButton } from '../../../components/RevalidateButton' +import { UpdateSearchParamsButton } from '../../../components/UpdateSearchParamsButton' const getRandom = async () => Math.random() -export default async function Page() { +export default async function Page({ searchParams }) { const someProp = await getRandom() return ( @@ -15,6 +16,7 @@ export default async function Page() { + ) diff --git a/test/e2e/app-dir/parallel-routes-revalidation/app/refreshing/page.tsx b/test/e2e/app-dir/parallel-routes-revalidation/app/refreshing/page.tsx index 29284fca03701..4d4e976bcdfd0 100644 --- a/test/e2e/app-dir/parallel-routes-revalidation/app/refreshing/page.tsx +++ b/test/e2e/app-dir/parallel-routes-revalidation/app/refreshing/page.tsx @@ -1,6 +1,7 @@ import Link from 'next/link' +import { UpdateSearchParamsButton } from '../components/UpdateSearchParamsButton' -export default function Home() { +export default function Home({ searchParams }) { return (
@@ -9,6 +10,7 @@ export default function Home() {
Random # from Root Page: {Math.random()}
+
) } diff --git a/test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts b/test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts index e67bcd480d547..a93659b858bf4 100644 --- a/test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts +++ b/test/e2e/app-dir/parallel-routes-revalidation/parallel-routes-revalidation.test.ts @@ -159,128 +159,167 @@ createNextDescribe( }) describe.each([ - { basePath: '/refreshing', label: 'regular' }, - { basePath: '/dynamic-refresh/foo', label: 'dynamic' }, - ])('router.refresh ($label)', ({ basePath }) => { - it('should correctly refresh data for the intercepted route and previously active page slot', async () => { - const browser = await next.browser(basePath) - let initialRandomNumber = await browser.elementById('random-number') - - await browser.elementByCss(`[href='${basePath}/login']`).click() - - // interception modal should be visible - let initialModalRandomNumber = await browser - .elementById('modal-random') - .text() - - // trigger a refresh - await browser.elementById('refresh-button').click() - - await retry(async () => { - const newRandomNumber = await browser - .elementById('random-number') - .text() - const newModalRandomNumber = await browser - .elementById('modal-random') - .text() - expect(initialRandomNumber).not.toBe(newRandomNumber) - expect(initialModalRandomNumber).not.toBe(newModalRandomNumber) - - // reset the initial values to be the new values, so that we can verify the revalidate case below. - initialRandomNumber = newRandomNumber - initialModalRandomNumber = newModalRandomNumber - }) + { basePath: '/refreshing', label: 'regular', withSearchParams: false }, + { basePath: '/refreshing', label: 'regular', withSearchParams: true }, + { + basePath: '/dynamic-refresh/foo', + label: 'dynamic', + withSearchParams: false, + }, + { + basePath: '/dynamic-refresh/foo', + label: 'dynamic', + withSearchParams: true, + }, + ])( + 'router.refresh ($label) - searchParams: $withSearchParams', + ({ basePath, withSearchParams }) => { + it('should correctly refresh data for the intercepted route and previously active page slot', async () => { + const browser = await next.browser(basePath) + let initialSearchParams: string | undefined + + if (withSearchParams) { + // add some search params prior to proceeding + await browser.elementById('update-search-params').click() + + await retry(async () => { + initialSearchParams = await browser + .elementById('search-params') + .text() + expect(initialSearchParams).toMatch(/^Params: "0\.\d+"$/) + }) + } - // trigger a revalidate - await browser.elementById('revalidate-button').click() + let initialRandomNumber = await browser.elementById('random-number') + await browser.elementByCss(`[href='${basePath}/login']`).click() - await retry(async () => { - const newRandomNumber = await browser - .elementById('random-number') - .text() - const newModalRandomNumber = await browser + // interception modal should be visible + let initialModalRandomNumber = await browser .elementById('modal-random') .text() - expect(initialRandomNumber).not.toBe(newRandomNumber) - expect(initialModalRandomNumber).not.toBe(newModalRandomNumber) - }) - // reload the page, triggering which will remove the interception route and show the full page - await browser.refresh() - - const initialLoginPageRandomNumber = await browser - .elementById('login-page-random') - .text() - - // trigger a refresh - await browser.elementById('refresh-button').click() - - await retry(async () => { - const newLoginPageRandomNumber = await browser + // trigger a refresh + await browser.elementById('refresh-button').click() + + await retry(async () => { + const newRandomNumber = await browser + .elementById('random-number') + .text() + const newModalRandomNumber = await browser + .elementById('modal-random') + .text() + expect(initialRandomNumber).not.toBe(newRandomNumber) + expect(initialModalRandomNumber).not.toBe(newModalRandomNumber) + + // reset the initial values to be the new values, so that we can verify the revalidate case below. + initialRandomNumber = newRandomNumber + initialModalRandomNumber = newModalRandomNumber + }) + + // trigger a revalidate + await browser.elementById('revalidate-button').click() + + await retry(async () => { + const newRandomNumber = await browser + .elementById('random-number') + .text() + const newModalRandomNumber = await browser + .elementById('modal-random') + .text() + expect(initialRandomNumber).not.toBe(newRandomNumber) + expect(initialModalRandomNumber).not.toBe(newModalRandomNumber) + + if (withSearchParams) { + // add additional search params in the new modal + await browser.elementById('update-search-params-modal').click() + expect( + await browser.elementById('search-params-modal').text() + ).toMatch(/^Params: "0\.\d+"$/) + + // make sure the old params are still there too + expect(await browser.elementById('search-params').text()).toBe( + initialSearchParams + ) + } + }) + + // reload the page, triggering which will remove the interception route and show the full page + await browser.refresh() + + const initialLoginPageRandomNumber = await browser .elementById('login-page-random') .text() - expect(newLoginPageRandomNumber).not.toBe( - initialLoginPageRandomNumber - ) - }) - }) - - it('should correctly refresh data for previously intercepted modal and active page slot', async () => { - const browser = await next.browser(basePath) + // trigger a refresh + await browser.elementById('refresh-button').click() - await browser.elementByCss(`[href='${basePath}/login']`).click() + await retry(async () => { + const newLoginPageRandomNumber = await browser + .elementById('login-page-random') + .text() - // interception modal should be visible - let initialModalRandomNumber = await browser - .elementById('modal-random') - .text() - - await browser.elementByCss(`[href='${basePath}/other']`).click() - // data for the /other page should be visible + expect(newLoginPageRandomNumber).not.toBe( + initialLoginPageRandomNumber + ) + }) + }) - let initialOtherPageRandomNumber = await browser - .elementById('other-page-random') - .text() + it('should correctly refresh data for previously intercepted modal and active page slot', async () => { + const browser = await next.browser(basePath) - // trigger a refresh - await browser.elementById('refresh-button').click() + await browser.elementByCss(`[href='${basePath}/login']`).click() - await retry(async () => { - const newModalRandomNumber = await browser + // interception modal should be visible + let initialModalRandomNumber = await browser .elementById('modal-random') .text() - const newOtherPageRandomNumber = await browser - .elementById('other-page-random') - .text() - expect(initialModalRandomNumber).not.toBe(newModalRandomNumber) - expect(initialOtherPageRandomNumber).not.toBe( - newOtherPageRandomNumber - ) - // reset the initial values to be the new values, so that we can verify the revalidate case below. - initialOtherPageRandomNumber = newOtherPageRandomNumber - initialModalRandomNumber = newModalRandomNumber - }) + await browser.elementByCss(`[href='${basePath}/other']`).click() + // data for the /other page should be visible - // trigger a revalidate - await browser.elementById('revalidate-button').click() - - await retry(async () => { - const newModalRandomNumber = await browser - .elementById('modal-random') - .text() - - const newOtherPageRandomNumber = await browser + let initialOtherPageRandomNumber = await browser .elementById('other-page-random') .text() - expect(initialModalRandomNumber).not.toBe(newModalRandomNumber) - expect(initialOtherPageRandomNumber).not.toBe( - newOtherPageRandomNumber - ) + + // trigger a refresh + await browser.elementById('refresh-button').click() + + await retry(async () => { + const newModalRandomNumber = await browser + .elementById('modal-random') + .text() + + const newOtherPageRandomNumber = await browser + .elementById('other-page-random') + .text() + expect(initialModalRandomNumber).not.toBe(newModalRandomNumber) + expect(initialOtherPageRandomNumber).not.toBe( + newOtherPageRandomNumber + ) + // reset the initial values to be the new values, so that we can verify the revalidate case below. + initialOtherPageRandomNumber = newOtherPageRandomNumber + initialModalRandomNumber = newModalRandomNumber + }) + + // trigger a revalidate + await browser.elementById('revalidate-button').click() + + await retry(async () => { + const newModalRandomNumber = await browser + .elementById('modal-random') + .text() + + const newOtherPageRandomNumber = await browser + .elementById('other-page-random') + .text() + expect(initialModalRandomNumber).not.toBe(newModalRandomNumber) + expect(initialOtherPageRandomNumber).not.toBe( + newOtherPageRandomNumber + ) + }) }) - }) - }) + } + ) describe('server action revalidation', () => { it('handles refreshing when multiple parallel slots are active', async () => { diff --git a/test/turbopack-build-tests-manifest.json b/test/turbopack-build-tests-manifest.json index e386424a3998d..ac3e41079f7b8 100644 --- a/test/turbopack-build-tests-manifest.json +++ b/test/turbopack-build-tests-manifest.json @@ -2272,10 +2272,14 @@ "parallel-routes-revalidation should handle a redirect action when called in a slot", "parallel-routes-revalidation should handle router.refresh() when called in a slot", "parallel-routes-revalidation should not trigger full page when calling router.refresh() on an intercepted route", - "parallel-routes-revalidation router.refresh (dynamic) should correctly refresh data for the intercepted route and previously active page slot", - "parallel-routes-revalidation router.refresh (dynamic) should correctly refresh data for previously intercepted modal and active page slot", - "parallel-routes-revalidation router.refresh (regular) should correctly refresh data for the intercepted route and previously active page slot", - "parallel-routes-revalidation router.refresh (regular) should correctly refresh data for previously intercepted modal and active page slot", + "parallel-routes-revalidation router.refresh (dynamic) - searchParams: false should correctly refresh data for the intercepted route and previously active page slot", + "parallel-routes-revalidation router.refresh (dynamic) - searchParams: false should correctly refresh data for previously intercepted modal and active page slot", + "parallel-routes-revalidation router.refresh (dynamic) - searchParams: true should correctly refresh data for the intercepted route and previously active page slot", + "parallel-routes-revalidation router.refresh (dynamic) - searchParams: true should correctly refresh data for previously intercepted modal and active page slot", + "parallel-routes-revalidation router.refresh (regular) - searchParams: false should correctly refresh data for the intercepted route and previously active page slot", + "parallel-routes-revalidation router.refresh (regular) - searchParams: false should correctly refresh data for previously intercepted modal and active page slot", + "parallel-routes-revalidation router.refresh (regular) - searchParams: true should correctly refresh data for the intercepted route and previously active page slot", + "parallel-routes-revalidation router.refresh (regular) - searchParams: true should correctly refresh data for previously intercepted modal and active page slot", "parallel-routes-revalidation server action revalidation handles refreshing when multiple parallel slots are active" ], "pending": [],