From 2481c2cad1b34958a8666793eb0d92d42cd8311f Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 21 Jan 2021 01:45:42 -0600 Subject: [PATCH 1/4] Preserve asPath while resolving rewrites --- packages/next/build/webpack-config.ts | 2 +- .../next/next-server/lib/router/router.ts | 39 +++++++------------ .../lib/router/utils/resolve-rewrites.ts | 38 ++++++++++++------ .../custom-routes-i18n/test/index.test.js | 16 +++++++- .../integration/production/test/index.test.js | 22 +++++++++++ 5 files changed, 77 insertions(+), 40 deletions(-) diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts index 2ea65df28332875..e5e569aad84574a 100644 --- a/packages/next/build/webpack-config.ts +++ b/packages/next/build/webpack-config.ts @@ -213,7 +213,7 @@ export default async function getBaseWebpackConfig( let plugins: PluginMetaData[] = [] let babelPresetPlugins: { dir: string; config: any }[] = [] - const hasRewrites = rewrites.length > 0 || dev + const hasRewrites = rewrites.length > 0 if (config.experimental.plugins) { plugins = await collectPlugins(dir, config.env, config.plugins) diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index 44cc9486e395912..c5f464b15f08ff2 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -932,39 +932,23 @@ export default class Router implements BaseRouter { let resolvedAs = as if (process.env.__NEXT_HAS_REWRITES && as.startsWith('/')) { - resolvedAs = resolveRewrites( - addBasePath( - addLocale(delBasePath(parseRelativeUrl(as).pathname), this.locale) - ), + const rewritesResult = resolveRewrites( + addBasePath(addLocale(delBasePath(as), this.locale)), pages, rewrites, query, (p: string) => this._resolveHref({ pathname: p }, pages).pathname!, this.locales ) + resolvedAs = rewritesResult.asPath - if (resolvedAs !== as) { - const potentialHref = removePathTrailingSlash( - this._resolveHref( - Object.assign({}, parsed, { - pathname: normalizeLocalePath( - hasBasePath(resolvedAs) ? delBasePath(resolvedAs) : resolvedAs, - this.locales - ).pathname, - }), - pages, - false - ).pathname! - ) - + 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 - if (pages.includes(potentialHref)) { - route = potentialHref - pathname = potentialHref - parsed.pathname = pathname - url = formatWithValidation(parsed) - } + route = rewritesResult.resolvedHref + pathname = rewritesResult.resolvedHref + parsed.pathname = pathname + url = formatWithValidation(parsed) } } @@ -1045,7 +1029,8 @@ export default class Router implements BaseRouter { route, pathname, query, - addBasePath(addLocale(resolvedAs, this.locale)), + as, + resolvedAs, routeProps ) let { error, props, __N_SSG, __N_SSP } = routeInfo @@ -1092,6 +1077,7 @@ export default class Router implements BaseRouter { notFoundRoute, query, as, + resolvedAs, { shallow: false } ) } @@ -1259,6 +1245,7 @@ export default class Router implements BaseRouter { pathname: string, query: any, as: string, + resolvedAs: string, routeProps: RouteProperties ): Promise { try { @@ -1298,7 +1285,7 @@ export default class Router implements BaseRouter { if (__N_SSG || __N_SSP) { dataHref = this.pageLoader.getDataHref( formatWithValidation({ pathname, query }), - delBasePath(as), + resolvedAs, __N_SSG, this.locale ) diff --git a/packages/next/next-server/lib/router/utils/resolve-rewrites.ts b/packages/next/next-server/lib/router/utils/resolve-rewrites.ts index f5909d513b6da60..8e20fb5dfa95a3f 100644 --- a/packages/next/next-server/lib/router/utils/resolve-rewrites.ts +++ b/packages/next/next-server/lib/router/utils/resolve-rewrites.ts @@ -4,6 +4,8 @@ import prepareDestination from './prepare-destination' import { Rewrite } from '../../../../lib/load-custom-routes' import { removePathTrailingSlash } from '../../../../client/normalize-trailing-slash' import { normalizeLocalePath } from '../../i18n/normalize-locale-path' +import { parseRelativeUrl } from './parse-relative-url' +import { delBasePath } from '../router' const customRouteMatcher = pathMatch(true) @@ -15,10 +17,17 @@ export default function resolveRewrites( resolveHref: (path: string) => string, locales?: string[] ) { - if (!pages.includes(normalizeLocalePath(asPath, locales).pathname)) { + let matchedPage = false + let parsedAs = parseRelativeUrl(asPath) + let fsPathname = removePathTrailingSlash( + normalizeLocalePath(delBasePath(parsedAs.pathname), locales).pathname + ) + let resolvedHref + + if (!pages.includes(fsPathname)) { for (const rewrite of rewrites) { const matcher = customRouteMatcher(rewrite.source) - const params = matcher(asPath) + const params = matcher(parsedAs.pathname) if (params) { if (!rewrite.destination) { @@ -31,30 +40,37 @@ export default function resolveRewrites( query, true ) - asPath = destRes.parsedDestination.pathname! + parsedAs = destRes.parsedDestination + asPath = destRes.newUrl Object.assign(query, destRes.parsedDestination.query) - const fsPathname = normalizeLocalePath( - removePathTrailingSlash(asPath), - locales - ).pathname + fsPathname = removePathTrailingSlash( + normalizeLocalePath(delBasePath(asPath), locales).pathname + ) if (pages.includes(fsPathname)) { - asPath = fsPathname // check if we now match a page as this means we are done // resolving the rewrites + matchedPage = true + resolvedHref = fsPathname break } // check if we match a dynamic-route, if so we break the rewrites chain - const resolvedHref = resolveHref(fsPathname) + resolvedHref = resolveHref(fsPathname) if (resolvedHref !== asPath && pages.includes(resolvedHref)) { - asPath = fsPathname + matchedPage = true break } } } } - return asPath + return { + asPath, + parsedAs, + fsPathname, + matchedPage, + resolvedHref, + } } diff --git a/test/integration/custom-routes-i18n/test/index.test.js b/test/integration/custom-routes-i18n/test/index.test.js index 01fed18c05c2957..3baa5858ac01b29 100644 --- a/test/integration/custom-routes-i18n/test/index.test.js +++ b/test/integration/custom-routes-i18n/test/index.test.js @@ -94,7 +94,13 @@ const runTests = () => { await browser.elementByCss('#to-about').click() await check(async () => { - const data = JSON.parse(await browser.elementByCss('#data').text()) + const data = JSON.parse( + cheerio + .load(await browser.eval('document.documentElement.innerHTML'))( + '#data' + ) + .text() + ) console.log(data) return data.url === `${expectedIndex ? '/fr' : ''}/about` ? 'success' @@ -108,7 +114,13 @@ const runTests = () => { .click() await check(async () => { - const data = JSON.parse(await browser.elementByCss('#data').text()) + const data = JSON.parse( + cheerio + .load(await browser.eval('document.documentElement.innerHTML'))( + '#data' + ) + .text() + ) console.log(data) return data.url === `${expectedIndex ? '/fr' : ''}/hello` ? 'success' diff --git a/test/integration/production/test/index.test.js b/test/integration/production/test/index.test.js index 8b1920043ab00cd..7acfcb58908c2a5 100644 --- a/test/integration/production/test/index.test.js +++ b/test/integration/production/test/index.test.js @@ -10,6 +10,7 @@ import { stopApp, waitFor, getPageFileFromPagesManifest, + check, } from 'next-test-utils' import webdriver from 'next-webdriver' import { @@ -860,6 +861,27 @@ describe('Production Usage', () => { expect(missing).toBe(false) }) + it('should preserve query when hard navigating from page 404', async () => { + const browser = await webdriver(appPort, '/') + await browser.eval(`(function() { + window.beforeNav = 1 + window.next.router.push({ + pathname: '/non-existent', + query: { hello: 'world' } + }) + })()`) + + await check( + () => browser.eval('document.documentElement.innerHTML'), + /page could not be found/ + ) + + expect(await browser.eval('window.beforeNav')).toBe(null) + expect(await browser.eval('window.location.hash')).toBe('') + expect(await browser.eval('window.location.search')).toBe('?hello=world') + expect(await browser.eval('window.location.pathname')).toBe('/non-existent') + }) + dynamicImportTests(context, (p, q) => renderViaHTTP(context.appPort, p, q)) processEnv(context) From e95e7f8d06d0361d1d281900ca5e7e95d1481d27 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 21 Jan 2021 13:42:27 -0600 Subject: [PATCH 2/4] remove un-needed alias --- packages/next/build/webpack-config.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts index e5e569aad84574a..d75f5d6ad3ba159 100644 --- a/packages/next/build/webpack-config.ts +++ b/packages/next/build/webpack-config.ts @@ -341,10 +341,6 @@ export default async function getBaseWebpackConfig( } } - const clientResolveRewrites = require.resolve( - 'next/dist/next-server/lib/router/utils/resolve-rewrites' - ) - const resolveConfig = { // Disable .mjs for node_modules bundling extensions: isServer @@ -383,9 +379,6 @@ export default async function getBaseWebpackConfig( [DOT_NEXT_ALIAS]: distDir, ...getOptimizedAliases(isServer), ...getReactProfilingInProduction(), - [clientResolveRewrites]: hasRewrites - ? clientResolveRewrites - : require.resolve('next/dist/client/dev/noop.js'), }, mainFields: isServer ? ['main', 'module'] : ['browser', 'module', 'main'], plugins: isWebpack5 From 0c8f5094016487241d6ba4012736cd835e2372a7 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 21 Jan 2021 14:24:52 -0600 Subject: [PATCH 3/4] Add new alias --- packages/next/build/webpack-config.ts | 10 ++++++++++ .../lib/router/utils/resolve-rewrites-noop.ts | 1 + 2 files changed, 11 insertions(+) create mode 100644 packages/next/next-server/lib/router/utils/resolve-rewrites-noop.ts diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts index d75f5d6ad3ba159..c36610e0a04bc54 100644 --- a/packages/next/build/webpack-config.ts +++ b/packages/next/build/webpack-config.ts @@ -341,6 +341,13 @@ export default async function getBaseWebpackConfig( } } + const clientResolveRewrites = require.resolve( + 'next/dist/next-server/lib/router/utils/resolve-rewrites' + ) + const clientResolveRewritesNoop = require.resolve( + 'next/dist/next-server/lib/router/utils/resolve-rewrites-noop' + ) + const resolveConfig = { // Disable .mjs for node_modules bundling extensions: isServer @@ -379,6 +386,9 @@ export default async function getBaseWebpackConfig( [DOT_NEXT_ALIAS]: distDir, ...getOptimizedAliases(isServer), ...getReactProfilingInProduction(), + [clientResolveRewrites]: hasRewrites + ? clientResolveRewrites + : clientResolveRewritesNoop, }, mainFields: isServer ? ['main', 'module'] : ['browser', 'module', 'main'], plugins: isWebpack5 diff --git a/packages/next/next-server/lib/router/utils/resolve-rewrites-noop.ts b/packages/next/next-server/lib/router/utils/resolve-rewrites-noop.ts new file mode 100644 index 000000000000000..7104c3881eb4018 --- /dev/null +++ b/packages/next/next-server/lib/router/utils/resolve-rewrites-noop.ts @@ -0,0 +1 @@ +export default function resolveRewrites() {} From e6846b8a838d833615bb82868ea0556c389112e6 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 21 Jan 2021 17:01:24 -0600 Subject: [PATCH 4/4] Add return type for resolve rewrites --- .../next/next-server/lib/router/utils/resolve-rewrites.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/next/next-server/lib/router/utils/resolve-rewrites.ts b/packages/next/next-server/lib/router/utils/resolve-rewrites.ts index 8e20fb5dfa95a3f..4ed3987999ab92b 100644 --- a/packages/next/next-server/lib/router/utils/resolve-rewrites.ts +++ b/packages/next/next-server/lib/router/utils/resolve-rewrites.ts @@ -16,7 +16,12 @@ export default function resolveRewrites( query: ParsedUrlQuery, resolveHref: (path: string) => string, locales?: string[] -) { +): { + matchedPage: boolean + parsedAs: ReturnType + asPath: string + resolvedHref?: string +} { let matchedPage = false let parsedAs = parseRelativeUrl(asPath) let fsPathname = removePathTrailingSlash( @@ -69,7 +74,6 @@ export default function resolveRewrites( return { asPath, parsedAs, - fsPathname, matchedPage, resolvedHref, }