Skip to content

Commit d959d28

Browse files
ijjkijjk
authored
Ensure client cache keys match between prefetch and transition (#38089)
* Ensure client cache keys match between prefetch and transition * handle some middleware cases * lint-fix * fix lint rename
1 parent a72e136 commit d959d28

File tree

2 files changed

+148
-25
lines changed

2 files changed

+148
-25
lines changed

‎packages/next/shared/lib/router/router.ts‎

Lines changed: 55 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1199,7 +1199,7 @@ export default class Router implements BaseRouter {
11991199
router: this,
12001200
}))
12011201

1202-
if (!isMiddlewareMatch && shouldResolveHref && pathname !== '/_error') {
1202+
if (shouldResolveHref && pathname !== '/_error') {
12031203
;(options as any)._shouldResolveHref = true
12041204

12051205
if (process.env.__NEXT_HAS_REWRITES && as.startsWith('/')) {
@@ -1216,22 +1216,30 @@ export default class Router implements BaseRouter {
12161216
handleHardNavigation({ url: as, router: this })
12171217
return true
12181218
}
1219-
resolvedAs = rewritesResult.asPath
1219+
if (!isMiddlewareMatch) {
1220+
resolvedAs = rewritesResult.asPath
1221+
}
12201222

12211223
if (rewritesResult.matchedPage && rewritesResult.resolvedHref) {
12221224
// if this directly matches a page we need to update the href to
12231225
// allow the correct page chunk to be loaded
12241226
pathname = rewritesResult.resolvedHref
12251227
parsed.pathname = addBasePath(pathname)
1226-
url = formatWithValidation(parsed)
1228+
1229+
if (!isMiddlewareMatch) {
1230+
url = formatWithValidation(parsed)
1231+
}
12271232
}
12281233
} else {
12291234
parsed.pathname = resolveDynamicRoute(pathname, pages)
12301235

12311236
if (parsed.pathname !== pathname) {
12321237
pathname = parsed.pathname
12331238
parsed.pathname = addBasePath(pathname)
1234-
url = formatWithValidation(parsed)
1239+
1240+
if (!isMiddlewareMatch) {
1241+
url = formatWithValidation(parsed)
1242+
}
12351243
}
12361244
}
12371245
}
@@ -1250,13 +1258,14 @@ export default class Router implements BaseRouter {
12501258
resolvedAs = removeLocale(removeBasePath(resolvedAs), nextState.locale)
12511259

12521260
let route = removeTrailingSlash(pathname)
1261+
let routeMatch: { [paramName: string]: string | string[] } | false = false
12531262

1254-
if (!isMiddlewareMatch && isDynamicRoute(route)) {
1263+
if (isDynamicRoute(route)) {
12551264
const parsedAs = parseRelativeUrl(resolvedAs)
12561265
const asPathname = parsedAs.pathname
12571266

12581267
const routeRegex = getRouteRegex(route)
1259-
const routeMatch = getRouteMatcher(routeRegex)(asPathname)
1268+
routeMatch = getRouteMatcher(routeRegex)(asPathname)
12601269
const shouldInterpolate = route === asPathname
12611270
const interpolatedAs = shouldInterpolate
12621271
? interpolateAs(route, asPathname, query)
@@ -1267,7 +1276,7 @@ export default class Router implements BaseRouter {
12671276
(param) => !query[param]
12681277
)
12691278

1270-
if (missingParams.length > 0) {
1279+
if (missingParams.length > 0 && !isMiddlewareMatch) {
12711280
if (process.env.NODE_ENV !== 'production') {
12721281
console.warn(
12731282
`${
@@ -1329,6 +1338,14 @@ export default class Router implements BaseRouter {
13291338
route = pathname
13301339
query = Object.assign({}, routeInfo.query || {}, query)
13311340

1341+
if (routeMatch && pathname !== parsed.pathname) {
1342+
Object.keys(routeMatch).forEach((key) => {
1343+
if (routeMatch && query[key] === routeMatch[key]) {
1344+
delete query[key]
1345+
}
1346+
})
1347+
}
1348+
13321349
if (isDynamicRoute(pathname)) {
13331350
const prefixedAs =
13341351
routeInfo.resolvedAs ||
@@ -1346,10 +1363,10 @@ export default class Router implements BaseRouter {
13461363
rewriteAs = localeResult.pathname
13471364
}
13481365
const routeRegex = getRouteRegex(pathname)
1349-
const routeMatch = getRouteMatcher(routeRegex)(rewriteAs)
1366+
const curRouteMatch = getRouteMatcher(routeRegex)(rewriteAs)
13501367

1351-
if (routeMatch) {
1352-
Object.assign(query, routeMatch)
1368+
if (curRouteMatch) {
1369+
Object.assign(query, curRouteMatch)
13531370
}
13541371
}
13551372
}
@@ -1994,6 +2011,17 @@ export default class Router implements BaseRouter {
19942011
const pages = await this.pageLoader.getPageList()
19952012
let resolvedAs = asPath
19962013

2014+
const locale =
2015+
typeof options.locale !== 'undefined'
2016+
? options.locale || undefined
2017+
: this.locale
2018+
2019+
const isMiddlewareMatch = await matchesMiddleware({
2020+
asPath: asPath,
2021+
locale: locale,
2022+
router: this,
2023+
})
2024+
19972025
if (process.env.__NEXT_HAS_REWRITES && asPath.startsWith('/')) {
19982026
let rewrites: any
19992027
;({ __rewrites: rewrites } = await getClientBuildManifest())
@@ -2020,18 +2048,25 @@ export default class Router implements BaseRouter {
20202048
// allow the correct page chunk to be loaded
20212049
pathname = rewritesResult.resolvedHref
20222050
parsed.pathname = pathname
2023-
url = formatWithValidation(parsed)
2051+
2052+
if (!isMiddlewareMatch) {
2053+
url = formatWithValidation(parsed)
2054+
}
20242055
}
2025-
} else {
2026-
parsed.pathname = resolveDynamicRoute(parsed.pathname, pages)
2056+
}
2057+
parsed.pathname = resolveDynamicRoute(parsed.pathname, pages)
20272058

2028-
if (parsed.pathname !== pathname) {
2029-
pathname = parsed.pathname
2030-
parsed.pathname = pathname
2031-
Object.assign(
2032-
query,
2033-
getRouteMatcher(getRouteRegex(parsed.pathname))(asPath) || {}
2034-
)
2059+
if (isDynamicRoute(parsed.pathname)) {
2060+
pathname = parsed.pathname
2061+
parsed.pathname = pathname
2062+
Object.assign(
2063+
query,
2064+
getRouteMatcher(getRouteRegex(parsed.pathname))(
2065+
parsePath(asPath).pathname
2066+
) || {}
2067+
)
2068+
2069+
if (!isMiddlewareMatch) {
20352070
url = formatWithValidation(parsed)
20362071
}
20372072
}
@@ -2041,11 +2076,6 @@ export default class Router implements BaseRouter {
20412076
return
20422077
}
20432078

2044-
const locale =
2045-
typeof options.locale !== 'undefined'
2046-
? options.locale || undefined
2047-
: this.locale
2048-
20492079
// TODO: if the route middleware's data request
20502080
// resolves to is not an SSG route we should bust the cache
20512081
// but we shouldn't allow prefetch to keep triggering

‎test/integration/dynamic-routing/test/index.test.js‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,99 @@ const appDir = join(__dirname, '../')
2828
const buildIdPath = join(appDir, '.next/BUILD_ID')
2929

3030
function runTests({ dev, serverless }) {
31+
if (!dev) {
32+
it('should have correct cache entries on prefetch', async () => {
33+
const browser = await webdriver(appPort, '/')
34+
await browser.waitForCondition('!!window.next.router.isReady')
35+
36+
const getCacheKeys = async () => {
37+
return (await browser.eval('Object.keys(window.next.router.sdc)'))
38+
.map((key) => {
39+
// strip http://localhost:PORT
40+
// and then strip buildId prefix
41+
return key
42+
.substring(key.indexOf('/_next'))
43+
.replace(/\/_next\/data\/(.*?)\//, '/_next/data/BUILD_ID/')
44+
})
45+
.sort()
46+
}
47+
48+
const cacheKeys = await getCacheKeys()
49+
expect(cacheKeys).toEqual(
50+
process.env.__MIDDLEWARE_TEST
51+
? [
52+
'/_next/data/BUILD_ID/[name].json?another=value&name=%5Bname%5D',
53+
'/_next/data/BUILD_ID/added-later/first.json?name=added-later&comment=first',
54+
'/_next/data/BUILD_ID/blog/321/comment/123.json?name=321&id=123',
55+
'/_next/data/BUILD_ID/d/dynamic-1.json?id=dynamic-1',
56+
'/_next/data/BUILD_ID/index.json',
57+
'/_next/data/BUILD_ID/on-mount/test-w-hash.json?post=test-w-hash',
58+
'/_next/data/BUILD_ID/p1/p2/all-ssg/hello.json?rest=hello',
59+
'/_next/data/BUILD_ID/p1/p2/all-ssg/hello1/hello2.json?rest=hello1&rest=hello2',
60+
'/_next/data/BUILD_ID/p1/p2/all-ssr/:42.json?rest=%3A42',
61+
'/_next/data/BUILD_ID/p1/p2/all-ssr/hello.json?rest=hello',
62+
'/_next/data/BUILD_ID/p1/p2/all-ssr/hello1%2F/he%2Fllo2.json?rest=hello1%2F&rest=he%2Fllo2',
63+
'/_next/data/BUILD_ID/p1/p2/all-ssr/hello1/hello2.json?rest=hello1&rest=hello2',
64+
'/_next/data/BUILD_ID/p1/p2/nested-all-ssg/hello.json?rest=hello',
65+
'/_next/data/BUILD_ID/p1/p2/nested-all-ssg/hello1/hello2.json?rest=hello1&rest=hello2',
66+
'/_next/data/BUILD_ID/post-1.json?fromHome=true&name=post-1',
67+
'/_next/data/BUILD_ID/post-1.json?hidden=value&name=post-1',
68+
'/_next/data/BUILD_ID/post-1.json?name=post-1',
69+
'/_next/data/BUILD_ID/post-1.json?name=post-1&another=value',
70+
'/_next/data/BUILD_ID/post-1/comment-1.json?name=post-1&comment=comment-1',
71+
'/_next/data/BUILD_ID/post-1/comments.json?name=post-1',
72+
]
73+
: [
74+
'/_next/data/BUILD_ID/p1/p2/all-ssg/hello.json?rest=hello',
75+
'/_next/data/BUILD_ID/p1/p2/all-ssg/hello1/hello2.json?rest=hello1&rest=hello2',
76+
'/_next/data/BUILD_ID/p1/p2/nested-all-ssg/hello.json?rest=hello',
77+
'/_next/data/BUILD_ID/p1/p2/nested-all-ssg/hello1/hello2.json?rest=hello1&rest=hello2',
78+
]
79+
)
80+
81+
// ensure no new cache entries after navigation
82+
const links = [
83+
{
84+
linkSelector: '#ssg-catch-all-single',
85+
waitForSelector: '#all-ssg-content',
86+
},
87+
{
88+
linkSelector: '#ssg-catch-all-single-interpolated',
89+
waitForSelector: '#all-ssg-content',
90+
},
91+
{
92+
linkSelector: '#ssg-catch-all-multi',
93+
waitForSelector: '#all-ssg-content',
94+
},
95+
{
96+
linkSelector: '#ssg-catch-all-multi-no-as',
97+
waitForSelector: '#all-ssg-content',
98+
},
99+
{
100+
linkSelector: '#ssg-catch-all-multi',
101+
waitForSelector: '#all-ssg-content',
102+
},
103+
{
104+
linkSelector: '#nested-ssg-catch-all-single',
105+
waitForSelector: '#nested-all-ssg-content',
106+
},
107+
{
108+
linkSelector: '#nested-ssg-catch-all-multi',
109+
waitForSelector: '#nested-all-ssg-content',
110+
},
111+
]
112+
113+
for (const { linkSelector, waitForSelector } of links) {
114+
await browser.elementByCss(linkSelector).click()
115+
await browser.waitForElementByCss(waitForSelector)
116+
await browser.back()
117+
await browser.waitForElementByCss(linkSelector)
118+
}
119+
const newCacheKeys = await getCacheKeys()
120+
expect(newCacheKeys).toEqual(cacheKeys)
121+
})
122+
}
123+
31124
if (dev) {
32125
it('should not have error after pinging WebSocket', async () => {
33126
const browser = await webdriver(appPort, '/')

0 commit comments

Comments
 (0)