From 47b7660aec950d2985f1d137d574fb84b935d555 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 5 Jan 2021 19:20:04 -0600 Subject: [PATCH] Ensure path starts with / when deleting index basePath with query (#20766) This is a follow-up to https://github.com/vercel/next.js/pull/20596 and https://github.com/vercel/next.js/pull/20658 ensuring the `as` value is prefixed with the `basePath` correctly with a query. This updates the test to also ensure no errors are shown when a query is present on the index `basePath` route. Fixes: https://github.com/vercel/next.js/pull/20757#issuecomment-754338510 --- .../next/next-server/lib/router/router.ts | 39 +++++++++++++++---- test/integration/basepath/test/index.test.js | 8 ++++ .../build-output/test/index.test.js | 6 +-- .../integration/size-limit/test/index.test.js | 2 +- 4 files changed, 43 insertions(+), 12 deletions(-) diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index 00958c73dc5aa..00c9c4ae2a313 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -81,7 +81,7 @@ function addPathPrefix(path: string, prefix?: string) { return prefix && path.startsWith('/') ? path === '/' ? normalizePathTrailingSlash(prefix) - : `${prefix}${path}` + : `${prefix}${pathNoQueryHash(path) === '/' ? path.substring(1) : path}` : path } @@ -133,13 +133,18 @@ export function delLocale(path: string, locale?: string) { return path } -export function hasBasePath(path: string): boolean { +function pathNoQueryHash(path: string) { const queryIndex = path.indexOf('?') const hashIndex = path.indexOf('#') if (queryIndex > -1 || hashIndex > -1) { path = path.substring(0, queryIndex > -1 ? queryIndex : hashIndex) } + return path +} + +export function hasBasePath(path: string): boolean { + path = pathNoQueryHash(path) return path === basePath || path.startsWith(basePath + '/') } @@ -149,7 +154,9 @@ export function addBasePath(path: string): string { } export function delBasePath(path: string): string { - return path.slice(basePath.length) || '/' + path = path.slice(basePath.length) + if (!path.startsWith('/')) path = `/${path}` + return path } /** @@ -302,15 +309,31 @@ export function resolveHref( } } +function stripOrigin(url: string) { + const origin = getLocationOrigin() + + return url.startsWith(origin) ? url.substring(origin.length) : url +} + function prepareUrlAs(router: NextRouter, url: Url, as?: Url) { // If url and as provided as an object representation, // we'll format them into the string version here. - const [resolvedHref, resolvedAs] = resolveHref(router.pathname, url, true) + let [resolvedHref, resolvedAs] = resolveHref(router.pathname, url, true) + const origin = getLocationOrigin() + const hrefHadOrigin = resolvedHref.startsWith(origin) + const asHadOrigin = resolvedAs && resolvedAs.startsWith(origin) + + resolvedHref = stripOrigin(resolvedHref) + resolvedAs = resolvedAs ? stripOrigin(resolvedAs) : resolvedAs + + const preparedUrl = hrefHadOrigin ? resolvedHref : addBasePath(resolvedHref) + const preparedAs = as + ? stripOrigin(resolveHref(router.pathname, as)) + : resolvedAs || resolvedHref + return { - url: addBasePath(resolvedHref), - as: addBasePath( - as ? resolveHref(router.pathname, as) : resolvedAs || resolvedHref - ), + url: preparedUrl, + as: asHadOrigin ? preparedAs : addBasePath(preparedAs), } } diff --git a/test/integration/basepath/test/index.test.js b/test/integration/basepath/test/index.test.js index 8d0b5ae23c5d4..3c73d03e52399 100644 --- a/test/integration/basepath/test/index.test.js +++ b/test/integration/basepath/test/index.test.js @@ -1024,6 +1024,10 @@ const runTests = (dev = false) => { `${basePath}/hello` ) expect(await browser.eval('window.location.search')).toBe('?query=true') + + if (dev) { + expect(await hasRedbox(browser, false)).toBe(false) + } } finally { await browser.close() } @@ -1044,6 +1048,10 @@ const runTests = (dev = false) => { expect(pathname).toBe('/') expect(await browser.eval('window.location.pathname')).toBe(basePath) expect(await browser.eval('window.location.search')).toBe('?query=true') + + if (dev) { + expect(await hasRedbox(browser, false)).toBe(false) + } } finally { await browser.close() } diff --git a/test/integration/build-output/test/index.test.js b/test/integration/build-output/test/index.test.js index 36efddc1d58f1..aee083a85e08c 100644 --- a/test/integration/build-output/test/index.test.js +++ b/test/integration/build-output/test/index.test.js @@ -95,16 +95,16 @@ describe('Build Output', () => { expect(indexSize.endsWith('B')).toBe(true) // should be no bigger than 62.2 kb - expect(parseFloat(indexFirstLoad)).toBeCloseTo(62.3, 1) + expect(parseFloat(indexFirstLoad)).toBeCloseTo(62.4, 1) expect(indexFirstLoad.endsWith('kB')).toBe(true) expect(parseFloat(err404Size) - 3.7).toBeLessThanOrEqual(0) expect(err404Size.endsWith('kB')).toBe(true) - expect(parseFloat(err404FirstLoad)).toBeCloseTo(65.5, 1) + expect(parseFloat(err404FirstLoad)).toBeCloseTo(65.6, 1) expect(err404FirstLoad.endsWith('kB')).toBe(true) - expect(parseFloat(sharedByAll)).toBeCloseTo(62, 1) + expect(parseFloat(sharedByAll)).toBeCloseTo(62.1, 1) expect(sharedByAll.endsWith('kB')).toBe(true) if (_appSize.endsWith('kB')) { diff --git a/test/integration/size-limit/test/index.test.js b/test/integration/size-limit/test/index.test.js index a2bc467eec594..fcf4002ff8bc6 100644 --- a/test/integration/size-limit/test/index.test.js +++ b/test/integration/size-limit/test/index.test.js @@ -81,6 +81,6 @@ describe('Production response size', () => { const delta = responseSizesBytes / 1024 // Expected difference: < 0.5 - expect(delta).toBeCloseTo(282, 0) + expect(delta).toBeCloseTo(282.6, 0) }) })