From 3ccca88f659ea976a47a2eea82d3b999a2052419 Mon Sep 17 00:00:00 2001 From: Zack Tanner Date: Thu, 4 Apr 2024 14:41:11 -0700 Subject: [PATCH] fix refreshing inactive segments that contained searchParams --- .../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 c3d3997301d18c..7b51e3660215c3 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 9ccd1da6c9edb4..b83414a2bb9ef5 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 d970d8c7fe7b7f..3d0cf8b7ee230d 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 11629c93e85955..1623275d8e1de1 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 c9ccabe3f8b3ee..7d8e314b848ad1 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 3d0d73e3f48a96..5fe183d7eaea7f 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 192ec4eb215cba..95eb0ba806408e 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 00000000000000..df06cc919446d9 --- /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 42f51e7bd7c7aa..fd46b4c8e25134 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 2634584a60bbe9..c6411141bfd441 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 30496df0e5a610..b681fe548b14cb 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 29284fca03701e..4d4e976bcdfd02 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 e67bcd480d547e..a93659b858bf40 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 e386424a3998d1..ac3e41079f7b8f 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": [],