make router restore action resilient to a missing tree (#62098)
### What Following an anchor link to a hash param, and then attempting to use `history.pushState` or `history.replaceState`, would result in an MPA navigation to the targeted URL. ### Why In #61822, a guard was added to prevent calling `ACTION_RESTORE` with a missing tree, to match other call-sites where we do the same. This was to prevent the app from crashing in the case where app router internals weren't available in the history state. The original assumption was that this is a rare / unlikely edge case. However the above scenario is a very probable case where this can happen, and triggering an MPA navigation isn't ideal. ### How This updates `ACTION_RESTORE` to be resilient to an undefined router state tree. When this happens, we'll still trigger the restore action to sync params, but use the existing flight router state. Closes NEXT-2502
This commit is contained in:
parent
079fe5432f
commit
5309c30c7d
5 changed files with 68 additions and 16 deletions
|
@ -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) {
|
||||
|
|
|
@ -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,
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -5,6 +5,11 @@ export default function ShallowLayout({ children }) {
|
|||
<>
|
||||
<h1>Shallow Routing</h1>
|
||||
<div>
|
||||
<div>
|
||||
<a href="#content" id="hash-navigation">
|
||||
Hash Navigation (non-Link)
|
||||
</a>
|
||||
</div>
|
||||
<div>
|
||||
<Link href="/a" id="to-a">
|
||||
To A
|
||||
|
@ -85,7 +90,7 @@ export default function ShallowLayout({ children }) {
|
|||
</Link>
|
||||
</div>
|
||||
</div>
|
||||
{children}
|
||||
<div id="content">{children}</div>
|
||||
</>
|
||||
)
|
||||
}
|
||||
|
|
|
@ -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`
|
||||
)
|
||||
})
|
||||
})
|
||||
})
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue