From ece907572e6ed1643e666e043c09e2d7ee8cc64e Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 27 Jul 2022 18:05:30 -0500 Subject: [PATCH 1/4] Ensure query is updated correctly with shallow and middleware --- packages/next/shared/lib/router/router.ts | 23 ++- .../e2e/middleware-rewrites/app/middleware.js | 5 + .../middleware-rewrites/test/index.test.ts | 178 +++++++++++++++++- 3 files changed, 201 insertions(+), 5 deletions(-) diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index 29294c48702b..53ecf7919925 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -1347,7 +1347,10 @@ export default class Router implements BaseRouter { if ('route' in routeInfo && isMiddlewareMatch) { pathname = routeInfo.route || route route = pathname - query = Object.assign({}, routeInfo.query || {}, query) + + if (!routeProps.shallow) { + query = Object.assign({}, routeInfo.query || {}, query) + } if (routeMatch && pathname !== parsed.pathname) { Object.keys(routeMatch).forEach((key) => { @@ -1359,8 +1362,15 @@ export default class Router implements BaseRouter { if (isDynamicRoute(pathname)) { const prefixedAs = - routeInfo.resolvedAs || - addBasePath(addLocale(as, nextState.locale), true) + !routeProps.shallow && routeInfo.resolvedAs + ? routeInfo.resolvedAs + : addBasePath( + addLocale( + new URL(as, location.href).pathname, + nextState.locale + ), + true + ) let rewriteAs = prefixedAs @@ -2346,7 +2356,7 @@ function getMiddlewareData( parseData: true, }) - const fsPathname = removeTrailingSlash(pathnameInfo.pathname) + let fsPathname = removeTrailingSlash(pathnameInfo.pathname) return Promise.all([ options.router.pageLoader.getPageList(), getClientBuildManifest(), @@ -2386,6 +2396,11 @@ function getMiddlewareData( Object.assign(parsedRewriteTarget.query, result.parsedAs.query) } } + const resolvedPathname = resolveDynamicRoute(fsPathname, pages) + + if (resolvedPathname !== fsPathname) { + fsPathname = resolvedPathname + } const resolvedHref = !pages.includes(fsPathname) ? resolveDynamicRoute( diff --git a/test/e2e/middleware-rewrites/app/middleware.js b/test/e2e/middleware-rewrites/app/middleware.js index bef33bcad188..f9734bfdf9c7 100644 --- a/test/e2e/middleware-rewrites/app/middleware.js +++ b/test/e2e/middleware-rewrites/app/middleware.js @@ -13,6 +13,11 @@ export async function middleware(request) { return NextResponse.next() } + if (url.pathname.includes('/fallback-true-blog/rewritten')) { + request.nextUrl.pathname = '/about' + return NextResponse.rewrite(request.nextUrl) + } + if (url.pathname.startsWith('/about') && url.searchParams.has('override')) { const isExternal = url.searchParams.get('override') === 'external' return NextResponse.rewrite( diff --git a/test/e2e/middleware-rewrites/test/index.test.ts b/test/e2e/middleware-rewrites/test/index.test.ts index e96e9bc64774..1d07b72fb0e5 100644 --- a/test/e2e/middleware-rewrites/test/index.test.ts +++ b/test/e2e/middleware-rewrites/test/index.test.ts @@ -4,8 +4,9 @@ import { join } from 'path' import cheerio from 'cheerio' import webdriver from 'next-webdriver' import { NextInstance } from 'test/lib/next-modes/base' -import { check, fetchViaHTTP } from 'next-test-utils' +import { check, fetchViaHTTP, waitFor } from 'next-test-utils' import { createNext, FileRef } from 'e2e-utils' +import escapeStringRegexp from 'escape-string-regexp' describe('Middleware Rewrite', () => { let next: NextInstance @@ -311,6 +312,181 @@ describe('Middleware Rewrite', () => { `/rewrite-to-afterfiles-rewrite` ) }) + + it('should handle shallow navigation correctly (non-dynamic page)', async () => { + const browser = await webdriver(next.url, '/about') + const requests = [] + + browser.on('request', (req) => { + const url = req.url() + if (url.includes('_next/data')) requests.push(url) + }) + + await browser.eval( + `next.router.push('/about?hello=world', undefined, { shallow: true })` + ) + await check(() => browser.eval(`next.router.query.hello`), 'world') + + expect(await browser.eval(`next.router.pathname`)).toBe('/about') + expect( + JSON.parse(await browser.eval(`JSON.stringify(next.router.query)`)) + ).toEqual({ hello: 'world' }) + expect(await browser.eval('location.pathname')).toBe('/about') + expect(await browser.eval('location.search')).toBe('?hello=world') + expect(requests).toEqual([]) + + await browser.eval( + `next.router.push('/about', undefined, { shallow: true })` + ) + await check( + () => browser.eval(`next.router.query.hello || 'empty'`), + 'empty' + ) + + expect(await browser.eval(`next.router.pathname`)).toBe('/about') + expect( + JSON.parse(await browser.eval(`JSON.stringify(next.router.query)`)) + ).toEqual({}) + expect(await browser.eval('location.pathname')).toBe('/about') + expect(await browser.eval('location.search')).toBe('') + expect(requests).toEqual([]) + }) + + it('should handle shallow navigation correctly (dynamic page)', async () => { + const browser = await webdriver(next.url, '/fallback-true-blog/first', { + waitHydration: false, + }) + let requests = [] + + browser.on('request', (req) => { + const url = req.url() + if (url.includes('_next/data')) requests.push(url) + }) + + // wait for initial query update request + await check(() => { + if (requests.length > 0) { + requests = [] + return 'yup' + } + }, 'yup') + + await browser.eval( + `next.router.push('/fallback-true-blog/first?hello=world', undefined, { shallow: true })` + ) + await check(() => browser.eval(`next.router.query.hello`), 'world') + + expect(await browser.eval(`next.router.pathname`)).toBe( + '/fallback-true-blog/[slug]' + ) + expect( + JSON.parse(await browser.eval(`JSON.stringify(next.router.query)`)) + ).toEqual({ hello: 'world', slug: 'first' }) + expect(await browser.eval('location.pathname')).toBe( + '/fallback-true-blog/first' + ) + expect(await browser.eval('location.search')).toBe('?hello=world') + expect(requests).toEqual([]) + + await browser.eval( + `next.router.push('/fallback-true-blog/second', undefined, { shallow: true })` + ) + await check(() => browser.eval(`next.router.query.slug`), 'second') + + expect(await browser.eval(`next.router.pathname`)).toBe( + '/fallback-true-blog/[slug]' + ) + expect( + JSON.parse(await browser.eval(`JSON.stringify(next.router.query)`)) + ).toEqual({ + slug: 'second', + }) + expect(await browser.eval('location.pathname')).toBe( + '/fallback-true-blog/second' + ) + expect(await browser.eval('location.search')).toBe('') + expect(requests).toEqual([]) + }) + + it('should resolve dynamic route after rewrite correctly', async () => { + const browser = await webdriver(next.url, '/fallback-true-blog/first', { + waitHydration: false, + }) + let requests = [] + + browser.on('request', (req) => { + const url = new URL( + req + .url() + .replace(new RegExp(escapeStringRegexp(next.buildId)), 'BUILD_ID') + ).pathname + + if (url.includes('_next/data')) requests.push(url) + }) + + // wait for initial query update request + await check(() => { + if (requests.length > 0) { + requests = [] + return 'yup' + } + }, 'yup') + + expect(await browser.eval(`next.router.pathname`)).toBe( + '/fallback-true-blog/[slug]' + ) + expect( + JSON.parse(await browser.eval(`JSON.stringify(next.router.query)`)) + ).toEqual({ + slug: 'first', + }) + expect(await browser.eval('location.pathname')).toBe( + '/fallback-true-blog/first' + ) + expect(await browser.eval('location.search')).toBe('') + expect(requests).toEqual([]) + + await browser.eval(`next.router.push('/fallback-true-blog/rewritten')`) + await check( + () => browser.eval('document.documentElement.innerHTML'), + /About Page/ + ) + + expect(await browser.eval(`next.router.pathname`)).toBe('/about') + expect( + JSON.parse(await browser.eval(`JSON.stringify(next.router.query)`)) + ).toEqual({}) + expect(await browser.eval('location.pathname')).toBe( + '/fallback-true-blog/rewritten' + ) + expect(await browser.eval('location.search')).toBe('') + expect(requests).toEqual([ + `/_next/data/BUILD_ID/en/fallback-true-blog/rewritten.json`, + ]) + + await browser.eval(`next.router.push('/fallback-true-blog/second')`) + await check( + () => browser.eval(`next.router.pathname`), + '/fallback-true-blog/[slug]' + ) + + expect(await browser.eval(`next.router.pathname`)).toBe( + '/fallback-true-blog/[slug]' + ) + expect( + JSON.parse(await browser.eval(`JSON.stringify(next.router.query)`)) + ).toEqual({ + slug: 'second', + }) + expect(await browser.eval('location.pathname')).toBe( + '/fallback-true-blog/second' + ) + expect(await browser.eval('location.search')).toBe('') + expect(requests).toEqual([ + `/_next/data/BUILD_ID/en/fallback-true-blog/rewritten.json`, + `/_next/data/BUILD_ID/en/fallback-true-blog/second.json`, + ]) + }) } function testsWithLocale(locale = '') { From 005bb8dc487b163ed021fcd51244801abcb4ba80 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 27 Jul 2022 18:12:50 -0500 Subject: [PATCH 2/4] lint-fix --- test/e2e/middleware-rewrites/test/index.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/middleware-rewrites/test/index.test.ts b/test/e2e/middleware-rewrites/test/index.test.ts index 1d07b72fb0e5..3f0f173b9238 100644 --- a/test/e2e/middleware-rewrites/test/index.test.ts +++ b/test/e2e/middleware-rewrites/test/index.test.ts @@ -4,7 +4,7 @@ import { join } from 'path' import cheerio from 'cheerio' import webdriver from 'next-webdriver' import { NextInstance } from 'test/lib/next-modes/base' -import { check, fetchViaHTTP, waitFor } from 'next-test-utils' +import { check, fetchViaHTTP } from 'next-test-utils' import { createNext, FileRef } from 'e2e-utils' import escapeStringRegexp from 'escape-string-regexp' From ff2e1277e1ae299219b2003fab9215f6574ef074 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 27 Jul 2022 18:56:08 -0500 Subject: [PATCH 3/4] update check --- packages/next/shared/lib/router/router.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index 53ecf7919925..8dc8973bec03 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -1712,6 +1712,7 @@ export default class Router implements BaseRouter { } let cachedRouteInfo = + !hasMiddleware && existingInfo && !('initial' in existingInfo) && process.env.NODE_ENV !== 'development' @@ -2395,11 +2396,12 @@ function getMiddlewareData( as = parsedRewriteTarget.pathname Object.assign(parsedRewriteTarget.query, result.parsedAs.query) } - } - const resolvedPathname = resolveDynamicRoute(fsPathname, pages) + } else if (!pages.includes(fsPathname)) { + const resolvedPathname = resolveDynamicRoute(fsPathname, pages) - if (resolvedPathname !== fsPathname) { - fsPathname = resolvedPathname + if (resolvedPathname !== fsPathname) { + fsPathname = resolvedPathname + } } const resolvedHref = !pages.includes(fsPathname) From 346e04bed6401b6d1e5e42cd64e830db9a5b3775 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 27 Jul 2022 19:29:56 -0500 Subject: [PATCH 4/4] update test case --- packages/next/shared/lib/router/router.ts | 12 ++++-------- test/e2e/middleware-rewrites/test/index.test.ts | 13 +++++++++---- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index 8dc8973bec03..c77d38c1dc28 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -1711,8 +1711,11 @@ export default class Router implements BaseRouter { return existingInfo } + if (hasMiddleware) { + existingInfo = undefined + } + let cachedRouteInfo = - !hasMiddleware && existingInfo && !('initial' in existingInfo) && process.env.NODE_ENV !== 'development' @@ -1770,13 +1773,6 @@ export default class Router implements BaseRouter { this.components[requestedRoute] = { ...existingInfo, route } return { ...existingInfo, route } } - - cachedRouteInfo = - existingInfo && - !('initial' in existingInfo) && - process.env.NODE_ENV !== 'development' - ? existingInfo - : undefined } if (route === '/api' || route.startsWith('/api/')) { diff --git a/test/e2e/middleware-rewrites/test/index.test.ts b/test/e2e/middleware-rewrites/test/index.test.ts index 3f0f173b9238..b9d61e50670d 100644 --- a/test/e2e/middleware-rewrites/test/index.test.ts +++ b/test/e2e/middleware-rewrites/test/index.test.ts @@ -482,10 +482,15 @@ describe('Middleware Rewrite', () => { '/fallback-true-blog/second' ) expect(await browser.eval('location.search')).toBe('') - expect(requests).toEqual([ - `/_next/data/BUILD_ID/en/fallback-true-blog/rewritten.json`, - `/_next/data/BUILD_ID/en/fallback-true-blog/second.json`, - ]) + expect( + requests.filter( + (req) => + ![ + `/_next/data/BUILD_ID/en/fallback-true-blog/rewritten.json`, + `/_next/data/BUILD_ID/en/fallback-true-blog/second.json`, + ].includes(req) + ) + ).toEqual([]) }) }