fix incorrect refresh request when basePath is set (#64589)
`initialCanonicalUrl` differs from the `canonicalUrl` that gets set on the client, such as when there's a basePath set. This hoists the `canonicalUrl` construction up so we can re-use it when adding refetch markers to the tree. This also renames `pathname` -> `path` since it also includes search params. I added a test to confirm no extra erroneous fetches happened in both cases. Fixes #64584 Closes NEXT-3130
This commit is contained in:
parent
e9aeb9e1b9
commit
ddf2af59c6
5 changed files with 66 additions and 21 deletions
|
@ -80,7 +80,7 @@ export function applyRouterStatePatchToTree(
|
|||
flightSegmentPath: FlightSegmentPath,
|
||||
flightRouterState: FlightRouterState,
|
||||
treePatch: FlightRouterState,
|
||||
pathname: string
|
||||
path: string
|
||||
): FlightRouterState | null {
|
||||
const [segment, parallelRoutes, url, refetch, isRootLayout] =
|
||||
flightRouterState
|
||||
|
@ -93,7 +93,7 @@ export function applyRouterStatePatchToTree(
|
|||
flightSegmentPath
|
||||
)
|
||||
|
||||
addRefreshMarkerToActiveParallelSegments(tree, pathname)
|
||||
addRefreshMarkerToActiveParallelSegments(tree, path)
|
||||
|
||||
return tree
|
||||
}
|
||||
|
@ -119,7 +119,7 @@ export function applyRouterStatePatchToTree(
|
|||
flightSegmentPath.slice(2),
|
||||
parallelRoutes[parallelRouteKey],
|
||||
treePatch,
|
||||
pathname
|
||||
path
|
||||
)
|
||||
|
||||
if (parallelRoutePatch === null) {
|
||||
|
@ -142,7 +142,7 @@ export function applyRouterStatePatchToTree(
|
|||
tree[4] = true
|
||||
}
|
||||
|
||||
addRefreshMarkerToActiveParallelSegments(tree, pathname)
|
||||
addRefreshMarkerToActiveParallelSegments(tree, path)
|
||||
|
||||
return tree
|
||||
}
|
||||
|
|
|
@ -49,7 +49,15 @@ export function createInitialRouterState({
|
|||
loading: initialSeedData[3],
|
||||
}
|
||||
|
||||
addRefreshMarkerToActiveParallelSegments(initialTree, initialCanonicalUrl)
|
||||
const canonicalUrl =
|
||||
// location.href is read as the initial value for canonicalUrl in the browser
|
||||
// This is safe to do as canonicalUrl can't be rendered, it's only used to control the history updates in the useEffect further down in this file.
|
||||
location
|
||||
? // window.location does not have the same type as URL but has all the fields createHrefFromUrl needs.
|
||||
createHrefFromUrl(location)
|
||||
: initialCanonicalUrl
|
||||
|
||||
addRefreshMarkerToActiveParallelSegments(initialTree, canonicalUrl)
|
||||
|
||||
const prefetchCache = new Map<string, PrefetchCacheEntry>()
|
||||
|
||||
|
@ -82,13 +90,7 @@ export function createInitialRouterState({
|
|||
hashFragment: null,
|
||||
segmentPaths: [],
|
||||
},
|
||||
canonicalUrl:
|
||||
// location.href is read as the initial value for canonicalUrl in the browser
|
||||
// This is safe to do as canonicalUrl can't be rendered, it's only used to control the history updates in the useEffect further down in this file.
|
||||
location
|
||||
? // window.location does not have the same type as URL but has all the fields createHrefFromUrl needs.
|
||||
createHrefFromUrl(location)
|
||||
: initialCanonicalUrl,
|
||||
canonicalUrl,
|
||||
nextUrl:
|
||||
// the || operator is intentional, the pathname can be an empty string
|
||||
(extractPathFromFlightRouterState(initialTree) || location?.pathname) ??
|
||||
|
|
|
@ -45,23 +45,23 @@ async function refreshInactiveParallelSegmentsImpl({
|
|||
fetchedSegments: Set<string>
|
||||
rootTree: FlightRouterState
|
||||
}) {
|
||||
const [, parallelRoutes, refetchPathname, refetchMarker] = updatedTree
|
||||
const [, parallelRoutes, refetchPath, refetchMarker] = updatedTree
|
||||
const fetchPromises = []
|
||||
|
||||
if (
|
||||
refetchPathname &&
|
||||
refetchPathname !== location.pathname &&
|
||||
refetchPath &&
|
||||
refetchPath !== location.pathname + location.search &&
|
||||
refetchMarker === 'refresh' &&
|
||||
// it's possible for the tree to contain multiple segments that contain data at the same URL
|
||||
// we keep track of them so we can dedupe the requests
|
||||
!fetchedSegments.has(refetchPathname)
|
||||
!fetchedSegments.has(refetchPath)
|
||||
) {
|
||||
fetchedSegments.add(refetchPathname) // Mark this URL as fetched
|
||||
fetchedSegments.add(refetchPath) // Mark this URL as fetched
|
||||
|
||||
// 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(
|
||||
new URL(refetchPathname, location.origin),
|
||||
new URL(refetchPath, 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'],
|
||||
|
@ -110,16 +110,16 @@ async function refreshInactiveParallelSegmentsImpl({
|
|||
*/
|
||||
export function addRefreshMarkerToActiveParallelSegments(
|
||||
tree: FlightRouterState,
|
||||
pathname: string
|
||||
path: string
|
||||
) {
|
||||
const [segment, parallelRoutes, , refetchMarker] = tree
|
||||
// a page segment might also contain concatenated search params, so we do a partial match on the key
|
||||
if (segment.includes(PAGE_SEGMENT_KEY) && refetchMarker !== 'refresh') {
|
||||
tree[2] = pathname
|
||||
tree[2] = path
|
||||
tree[3] = 'refresh'
|
||||
}
|
||||
|
||||
for (const key in parallelRoutes) {
|
||||
addRefreshMarkerToActiveParallelSegments(parallelRoutes[key], pathname)
|
||||
addRefreshMarkerToActiveParallelSegments(parallelRoutes[key], path)
|
||||
}
|
||||
}
|
||||
|
|
16
test/e2e/app-dir/app-basepath/app/refresh/page.js
Normal file
16
test/e2e/app-dir/app-basepath/app/refresh/page.js
Normal file
|
@ -0,0 +1,16 @@
|
|||
'use client'
|
||||
|
||||
import { useRouter } from 'next/navigation'
|
||||
|
||||
export default function Page() {
|
||||
const router = useRouter()
|
||||
return (
|
||||
<button
|
||||
onClick={() => {
|
||||
router.refresh()
|
||||
}}
|
||||
>
|
||||
Refresh
|
||||
</button>
|
||||
)
|
||||
}
|
|
@ -64,5 +64,32 @@ createNextDescribe(
|
|||
expect(await browser.url()).toBe(`${next.url}/base/dynamic/dest`)
|
||||
})
|
||||
})
|
||||
|
||||
it.each(['/base/refresh', '/base/refresh?foo=bar'])(
|
||||
`should only make a single RSC call to the current page (%s)`,
|
||||
async (path) => {
|
||||
let rscRequests = []
|
||||
const browser = await next.browser(path, {
|
||||
beforePageLoad(page) {
|
||||
page.on('request', (request) => {
|
||||
return request.allHeaders().then((headers) => {
|
||||
if (
|
||||
headers['RSC'.toLowerCase()] === '1' &&
|
||||
// Prefetches also include `RSC`
|
||||
headers['Next-Router-Prefetch'.toLowerCase()] !== '1'
|
||||
) {
|
||||
rscRequests.push(request.url())
|
||||
}
|
||||
})
|
||||
})
|
||||
},
|
||||
})
|
||||
await browser.elementByCss('button').click()
|
||||
await retry(async () => {
|
||||
expect(rscRequests.length).toBe(1)
|
||||
expect(rscRequests[0]).toContain(`${next.url}${path}`)
|
||||
})
|
||||
}
|
||||
)
|
||||
}
|
||||
)
|
||||
|
|
Loading…
Reference in a new issue