From 7fd4081ea4197e067005d447962d3f87eab629ad Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 27 Jun 2022 22:11:56 -0500 Subject: [PATCH 1/4] Ensure client cache keys match between prefetch and transition --- packages/next/shared/lib/router/router.ts | 37 ++++---- .../dynamic-routing/test/index.test.js | 93 +++++++++++++++++++ 2 files changed, 112 insertions(+), 18 deletions(-) diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index 6848d416ff5a..41f9e2d6437f 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -1199,7 +1199,7 @@ export default class Router implements BaseRouter { router: this, })) - if (!isMiddlewareMatch && shouldResolveHref && pathname !== '/_error') { + if (shouldResolveHref && pathname !== '/_error') { ;(options as any)._shouldResolveHref = true if (process.env.__NEXT_HAS_REWRITES && as.startsWith('/')) { @@ -1251,7 +1251,7 @@ export default class Router implements BaseRouter { let route = removeTrailingSlash(pathname) - if (!isMiddlewareMatch && isDynamicRoute(route)) { + if (isDynamicRoute(route)) { const parsedAs = parseRelativeUrl(resolvedAs) const asPathname = parsedAs.pathname @@ -1267,7 +1267,7 @@ export default class Router implements BaseRouter { (param) => !query[param] ) - if (missingParams.length > 0) { + if (missingParams.length > 0 && !isMiddlewareMatch) { if (process.env.NODE_ENV !== 'production') { console.warn( `${ @@ -2022,23 +2022,19 @@ export default class Router implements BaseRouter { parsed.pathname = pathname url = formatWithValidation(parsed) } - } else { - parsed.pathname = resolveDynamicRoute(parsed.pathname, pages) - - if (parsed.pathname !== pathname) { - pathname = parsed.pathname - parsed.pathname = pathname - Object.assign( - query, - getRouteMatcher(getRouteRegex(parsed.pathname))(asPath) || {} - ) - url = formatWithValidation(parsed) - } } + parsed.pathname = resolveDynamicRoute(parsed.pathname, pages) - // Prefetch is not supported in development mode because it would trigger on-demand-entries - if (process.env.NODE_ENV !== 'production') { - return + if (isDynamicRoute(parsed.pathname)) { + pathname = parsed.pathname + parsed.pathname = pathname + Object.assign( + query, + getRouteMatcher(getRouteRegex(parsed.pathname))( + parsePath(asPath).pathname + ) || {} + ) + url = formatWithValidation(parsed) } const locale = @@ -2046,6 +2042,11 @@ export default class Router implements BaseRouter { ? options.locale || undefined : this.locale + // Prefetch is not supported in development mode because it would trigger on-demand-entries + if (process.env.NODE_ENV !== 'production') { + return + } + // TODO: if the route middleware's data request // resolves to is not an SSG route we should bust the cache // but we shouldn't allow prefetch to keep triggering diff --git a/test/integration/dynamic-routing/test/index.test.js b/test/integration/dynamic-routing/test/index.test.js index 1b5bf31a715c..75f8beddd0b3 100644 --- a/test/integration/dynamic-routing/test/index.test.js +++ b/test/integration/dynamic-routing/test/index.test.js @@ -28,6 +28,99 @@ const appDir = join(__dirname, '../') const buildIdPath = join(appDir, '.next/BUILD_ID') function runTests({ dev, serverless }) { + if (!dev) { + it('should have correct cache entries on prefetch', async () => { + const browser = await webdriver(appPort, '/') + await browser.waitForCondition('!!window.next.router.isReady') + + const getCacheKeys = async () => { + return (await browser.eval('Object.keys(window.next.router.sdc)')) + .map((key) => { + // strip http://localhost:PORT + // and then strip buildId prefix + return key + .substring(key.indexOf('/_next')) + .replace(/\/_next\/data\/(.*?)\//, '/_next/data/BUILD_ID/') + }) + .sort() + } + + const cacheKeys = await getCacheKeys() + expect(cacheKeys).toEqual( + process.env.__MIDDLEWARE_TEST + ? [ + '/_next/data/BUILD_ID/[name].json?another=value&name=%5Bname%5D', + '/_next/data/BUILD_ID/added-later/first.json?name=added-later&comment=first', + '/_next/data/BUILD_ID/blog/321/comment/123.json?name=321&id=123', + '/_next/data/BUILD_ID/d/dynamic-1.json?id=dynamic-1', + '/_next/data/BUILD_ID/index.json', + '/_next/data/BUILD_ID/on-mount/test-w-hash.json?post=test-w-hash', + '/_next/data/BUILD_ID/p1/p2/all-ssg/hello.json?rest=hello', + '/_next/data/BUILD_ID/p1/p2/all-ssg/hello1/hello2.json?rest=hello1&rest=hello2', + '/_next/data/BUILD_ID/p1/p2/all-ssr/:42.json?rest=%3A42', + '/_next/data/BUILD_ID/p1/p2/all-ssr/hello.json?rest=hello', + '/_next/data/BUILD_ID/p1/p2/all-ssr/hello1%2F/he%2Fllo2.json?rest=hello1%2F&rest=he%2Fllo2', + '/_next/data/BUILD_ID/p1/p2/all-ssr/hello1/hello2.json?rest=hello1&rest=hello2', + '/_next/data/BUILD_ID/p1/p2/nested-all-ssg/hello.json?rest=hello', + '/_next/data/BUILD_ID/p1/p2/nested-all-ssg/hello1/hello2.json?rest=hello1&rest=hello2', + '/_next/data/BUILD_ID/post-1.json?fromHome=true&name=post-1', + '/_next/data/BUILD_ID/post-1.json?hidden=value&name=post-1', + '/_next/data/BUILD_ID/post-1.json?name=post-1', + '/_next/data/BUILD_ID/post-1.json?name=post-1&another=value', + '/_next/data/BUILD_ID/post-1/comment-1.json?name=post-1&comment=comment-1', + '/_next/data/BUILD_ID/post-1/comments.json?name=post-1', + ] + : [ + '/_next/data/BUILD_ID/p1/p2/all-ssg/hello.json?rest=hello', + '/_next/data/BUILD_ID/p1/p2/all-ssg/hello1/hello2.json?rest=hello1&rest=hello2', + '/_next/data/BUILD_ID/p1/p2/nested-all-ssg/hello.json?rest=hello', + '/_next/data/BUILD_ID/p1/p2/nested-all-ssg/hello1/hello2.json?rest=hello1&rest=hello2', + ] + ) + + // ensure no new cache entries after navigation + const links = [ + { + linkSelector: '#ssg-catch-all-single', + waitForSelector: '#all-ssg-content', + }, + { + linkSelector: '#ssg-catch-all-single-interpolated', + waitForSelector: '#all-ssg-content', + }, + { + linkSelector: '#ssg-catch-all-multi', + waitForSelector: '#all-ssg-content', + }, + { + linkSelector: '#ssg-catch-all-multi-no-as', + waitForSelector: '#all-ssg-content', + }, + { + linkSelector: '#ssg-catch-all-multi', + waitForSelector: '#all-ssg-content', + }, + { + linkSelector: '#nested-ssg-catch-all-single', + waitForSelector: '#nested-all-ssg-content', + }, + { + linkSelector: '#nested-ssg-catch-all-multi', + waitForSelector: '#nested-all-ssg-content', + }, + ] + + for (const { linkSelector, waitForSelector } of links) { + await browser.elementByCss(linkSelector).click() + await browser.waitForElementByCss(waitForSelector) + await browser.back() + await browser.waitForElementByCss(linkSelector) + } + const newCacheKeys = await getCacheKeys() + expect(newCacheKeys).toEqual(cacheKeys) + }) + } + if (dev) { it('should not have error after pinging WebSocket', async () => { const browser = await webdriver(appPort, '/') From f80b9bcc8637e623e4200aaf19ae88a2f49d1445 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 27 Jun 2022 22:51:14 -0500 Subject: [PATCH 2/4] handle some middleware cases --- packages/next/shared/lib/router/router.ts | 51 ++++++++++++++++++----- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index 41f9e2d6437f..2ee6a6ceb1c2 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -1216,14 +1216,19 @@ export default class Router implements BaseRouter { handleHardNavigation({ url: as, router: this }) return true } - resolvedAs = rewritesResult.asPath + if (!isMiddlewareMatch) { + resolvedAs = rewritesResult.asPath + } if (rewritesResult.matchedPage && rewritesResult.resolvedHref) { // if this directly matches a page we need to update the href to // allow the correct page chunk to be loaded pathname = rewritesResult.resolvedHref parsed.pathname = addBasePath(pathname) - url = formatWithValidation(parsed) + + if (!isMiddlewareMatch) { + url = formatWithValidation(parsed) + } } } else { parsed.pathname = resolveDynamicRoute(pathname, pages) @@ -1231,7 +1236,10 @@ export default class Router implements BaseRouter { if (parsed.pathname !== pathname) { pathname = parsed.pathname parsed.pathname = addBasePath(pathname) - url = formatWithValidation(parsed) + + if (!isMiddlewareMatch) { + url = formatWithValidation(parsed) + } } } } @@ -1250,13 +1258,14 @@ export default class Router implements BaseRouter { resolvedAs = removeLocale(removeBasePath(resolvedAs), nextState.locale) let route = removeTrailingSlash(pathname) + let routeMatch: { [paramName: string]: string | string[] } | false = false if (isDynamicRoute(route)) { const parsedAs = parseRelativeUrl(resolvedAs) const asPathname = parsedAs.pathname const routeRegex = getRouteRegex(route) - const routeMatch = getRouteMatcher(routeRegex)(asPathname) + routeMatch = getRouteMatcher(routeRegex)(asPathname) const shouldInterpolate = route === asPathname const interpolatedAs = shouldInterpolate ? interpolateAs(route, asPathname, query) @@ -1329,6 +1338,14 @@ export default class Router implements BaseRouter { route = pathname query = Object.assign({}, routeInfo.query || {}, query) + if (routeMatch && pathname !== parsed.pathname) { + Object.keys(routeMatch).forEach((key) => { + if (routeMatch && query[key] === routeMatch[key]) { + delete query[key] + } + }) + } + if (isDynamicRoute(pathname)) { const prefixedAs = routeInfo.resolvedAs || @@ -1994,6 +2011,17 @@ export default class Router implements BaseRouter { const pages = await this.pageLoader.getPageList() let resolvedAs = asPath + const locale = + typeof options.locale !== 'undefined' + ? options.locale || undefined + : this.locale + + const isMiddlewareMatch = await matchesMiddleware({ + asPath: asPath, + locale: locale, + router: this, + }) + if (process.env.__NEXT_HAS_REWRITES && asPath.startsWith('/')) { let rewrites: any ;({ __rewrites: rewrites } = await getClientBuildManifest()) @@ -2020,7 +2048,10 @@ export default class Router implements BaseRouter { // allow the correct page chunk to be loaded pathname = rewritesResult.resolvedHref parsed.pathname = pathname - url = formatWithValidation(parsed) + + if (!isMiddlewareMatch) { + url = formatWithValidation(parsed) + } } } parsed.pathname = resolveDynamicRoute(parsed.pathname, pages) @@ -2034,13 +2065,11 @@ export default class Router implements BaseRouter { parsePath(asPath).pathname ) || {} ) - url = formatWithValidation(parsed) - } - const locale = - typeof options.locale !== 'undefined' - ? options.locale || undefined - : this.locale + if (!isMiddlewareMatch) { + url = formatWithValidation(parsed) + } + } // Prefetch is not supported in development mode because it would trigger on-demand-entries if (process.env.NODE_ENV !== 'production') { From 1314d1acf5f9063482b3295aff53d5d9be638466 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 27 Jun 2022 22:58:28 -0500 Subject: [PATCH 3/4] lint-fix --- packages/next/shared/lib/router/router.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index 2ee6a6ceb1c2..835024729ceb 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -1363,9 +1363,9 @@ export default class Router implements BaseRouter { rewriteAs = localeResult.pathname } const routeRegex = getRouteRegex(pathname) - const routeMatch = getRouteMatcher(routeRegex)(rewriteAs) + const curRouteMatch = getRouteMatcher(routeRegex)(rewriteAs) - if (routeMatch) { + if (curRouteMatch) { Object.assign(query, routeMatch) } } From 71a249b6d1212d884f027ad02b69776f24707cf0 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 27 Jun 2022 23:13:12 -0500 Subject: [PATCH 4/4] fix lint rename --- packages/next/shared/lib/router/router.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index 835024729ceb..09106e75ca8e 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -1366,7 +1366,7 @@ export default class Router implements BaseRouter { const curRouteMatch = getRouteMatcher(routeRegex)(rewriteAs) if (curRouteMatch) { - Object.assign(query, routeMatch) + Object.assign(query, curRouteMatch) } } }