Skip to content

Commit

Permalink
Ensure path starts with / when deleting index basePath with query (#2…
Browse files Browse the repository at this point in the history
…0766)

This is a follow-up to #20596 and #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: #20757 (comment)
  • Loading branch information
ijjk authored Jan 6, 2021
1 parent 51f2a53 commit 47b7660
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 12 deletions.
39 changes: 31 additions & 8 deletions packages/next/next-server/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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 + '/')
}

Expand All @@ -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
}

/**
Expand Down Expand Up @@ -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),
}
}

Expand Down
8 changes: 8 additions & 0 deletions test/integration/basepath/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand All @@ -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()
}
Expand Down
6 changes: 3 additions & 3 deletions test/integration/build-output/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')) {
Expand Down
2 changes: 1 addition & 1 deletion test/integration/size-limit/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})

0 comments on commit 47b7660

Please sign in to comment.