From d57e0e0a748a6f6316f07b963e62719806bd60bd Mon Sep 17 00:00:00 2001 From: Zack Tanner Date: Thu, 15 Feb 2024 05:30:45 -0800 Subject: [PATCH] make ACTION_RESTORE resilient to a missing tree --- .../next/src/client/components/app-router.tsx | 14 +++---- .../reducers/restore-reducer.ts | 13 ++++-- .../router-reducer/router-reducer-types.ts | 8 ++-- .../shallow-routing/app/(shallow)/layout.tsx | 7 +++- .../shallow-routing/shallow-routing.test.ts | 42 +++++++++++++++++++ 5 files changed, 68 insertions(+), 16 deletions(-) diff --git a/packages/next/src/client/components/app-router.tsx b/packages/next/src/client/components/app-router.tsx index 5e31ef1ce9e36..061630f8f6abf 100644 --- a/packages/next/src/client/components/app-router.tsx +++ b/packages/next/src/client/components/app-router.tsx @@ -511,19 +511,14 @@ function Router({ url: string | URL | null | undefined ) => { const href = window.location.href - const urlToRestore = new URL(url ?? href, href) - if (!window.history.state?.__PRIVATE_NEXTJS_INTERNALS_TREE) { - // we cannot safely recover from a missing tree -- we need trigger an MPA navigation - // to restore the router history to the correct state. - window.location.href = urlToRestore.pathname - return - } + const tree: FlightRouterState | undefined = + window.history.state?.__PRIVATE_NEXTJS_INTERNALS_TREE startTransition(() => { dispatch({ type: ACTION_RESTORE, - url: urlToRestore, - tree: window.history.state.__PRIVATE_NEXTJS_INTERNALS_TREE, + url: new URL(url ?? href, href), + tree, }) }) } @@ -542,6 +537,7 @@ function Router({ if (data?.__NA || data?._N) { return originalPushState(data, _unused, url) } + data = copyNextJsInternalHistoryState(data) if (url) { diff --git a/packages/next/src/client/components/router-reducer/reducers/restore-reducer.ts b/packages/next/src/client/components/router-reducer/reducers/restore-reducer.ts index 77a67a4d81792..6387d4e15c507 100644 --- a/packages/next/src/client/components/router-reducer/reducers/restore-reducer.ts +++ b/packages/next/src/client/components/router-reducer/reducers/restore-reducer.ts @@ -13,6 +13,13 @@ export function restoreReducer( ): ReducerState { const { url, tree } = action const href = createHrefFromUrl(url) + // This action is used to restore the router state from the history state. + // However, it's possible that the history state no longer contains the `FlightRouterState`. + // We will copy over the internal state on pushState/replaceState events, but if a history entry + // occurred before hydration, or if the user navigated to a hash using a regular anchor link, + // the history state will not contain the `FlightRouterState`. + // In this case, we'll continue to use the existing tree so the router doesn't get into an invalid state. + const treeToRestore = tree || state.tree const oldCache = state.cache const newCache = process.env.__NEXT_PPR @@ -20,7 +27,7 @@ export function restoreReducer( // data for any segment whose dynamic data was already received. This // prevents an unnecessary flash back to PPR state during a // back/forward navigation. - updateCacheNodeOnPopstateRestoration(oldCache, tree) + updateCacheNodeOnPopstateRestoration(oldCache, treeToRestore) : oldCache return { @@ -37,7 +44,7 @@ export function restoreReducer( cache: newCache, prefetchCache: state.prefetchCache, // Restore provided tree - tree: tree, - nextUrl: extractPathFromFlightRouterState(tree) ?? url.pathname, + tree: treeToRestore, + nextUrl: extractPathFromFlightRouterState(treeToRestore) ?? url.pathname, } } 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 9268aca1fa0f6..1b2e604e4faf3 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 @@ -114,15 +114,17 @@ export interface NavigateAction { /** * Restore applies the provided router state. - * - Only used for `popstate` (back/forward navigation) where a known router state has to be applied. - * - Router state is applied as-is from the history state. + * - Used for `popstate` (back/forward navigation) where a known router state has to be applied. + * - Also used when syncing the router state with `pushState`/`replaceState` calls. + * - Router state is applied as-is from the history state, if available. + * - If the history state does not contain the router state, the existing router state is used. * - If any cache node is missing it will be fetched in layout-router during rendering and the server-patch case. * - If existing cache nodes match these are used. */ export interface RestoreAction { type: typeof ACTION_RESTORE url: URL - tree: FlightRouterState + tree: FlightRouterState | undefined } /** diff --git a/test/e2e/app-dir/shallow-routing/app/(shallow)/layout.tsx b/test/e2e/app-dir/shallow-routing/app/(shallow)/layout.tsx index 2329aaf5f96b3..fb79d54a9ea45 100644 --- a/test/e2e/app-dir/shallow-routing/app/(shallow)/layout.tsx +++ b/test/e2e/app-dir/shallow-routing/app/(shallow)/layout.tsx @@ -5,6 +5,11 @@ export default function ShallowLayout({ children }) { <>

Shallow Routing

+
+ + Hash Navigation (non-Link) + +
To A @@ -85,7 +90,7 @@ export default function ShallowLayout({ children }) {
- {children} +
{children}
) } diff --git a/test/e2e/app-dir/shallow-routing/shallow-routing.test.ts b/test/e2e/app-dir/shallow-routing/shallow-routing.test.ts index ffa488f899e07..6d3e8847dc97c 100644 --- a/test/e2e/app-dir/shallow-routing/shallow-routing.test.ts +++ b/test/e2e/app-dir/shallow-routing/shallow-routing.test.ts @@ -447,6 +447,48 @@ createNextDescribe( 'Page A' ) }) + + it('should support hash navigations while continuing to work for pushState/replaceState APIs', async () => { + const browser = await next.browser('/a') + expect( + await browser + .elementByCss('#to-pushstate-string-url') + .click() + .waitForElementByCss('#pushstate-string-url') + .text() + ).toBe('PushState String Url') + + await browser.elementByCss('#hash-navigation').click() + + // Check current url contains the hash + expect(await browser.url()).toBe( + `${next.url}/pushstate-string-url#content` + ) + + await browser.elementByCss('#push-string-url').click() + + // Check useSearchParams value is the new searchparam + await check(() => browser.elementByCss('#my-data').text(), 'foo') + + // Check current url is the new searchparams + expect(await browser.url()).toBe( + `${next.url}/pushstate-string-url?query=foo` + ) + + // Same cycle a second time + await browser.elementByCss('#push-string-url').click() + + // Check useSearchParams value is the new searchparam + await check( + () => browser.elementByCss('#my-data').text(), + 'foo-added' + ) + + // Check current url is the new searchparams + expect(await browser.url()).toBe( + `${next.url}/pushstate-string-url?query=foo-added` + ) + }) }) }) }