Skip to content

Commit

Permalink
fix edge case with basepath 404
Browse files Browse the repository at this point in the history
  • Loading branch information
Janpot committed Jul 31, 2020
1 parent acaedb3 commit 80f3ada
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 7 deletions.
4 changes: 2 additions & 2 deletions packages/next/client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { RouterContext } from '../next-server/lib/router-context'
import { isDynamicRoute } from '../next-server/lib/router/utils/is-dynamic'
import * as envConfig from '../next-server/lib/runtime-config'
import { getURL, loadGetInitialProps, ST } from '../next-server/lib/utils'
import { delBasePath, basePath } from '../next-server/lib/router/router'
import { delBasePath, hasBasePath } from '../next-server/lib/router/router'
import initHeadManager from './head-manager'
import PageLoader from './page-loader'
import measureWebVitals from './performance-relayer'
Expand Down Expand Up @@ -52,7 +52,7 @@ envConfig.setConfig({
let asPath = getURL()

// make sure not to attempt stripping basePath for 404s
if (asPath.startsWith(basePath)) {
if (hasBasePath(asPath)) {
asPath = delBasePath(asPath)
}

Expand Down
15 changes: 11 additions & 4 deletions packages/next/next-server/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,18 @@ import {
normalizePathTrailingSlash,
} from '../../../client/normalize-trailing-slash'

export const basePath = (process.env.__NEXT_ROUTER_BASEPATH as string) || ''
const basePath = (process.env.__NEXT_ROUTER_BASEPATH as string) || ''

function buildCancellationError() {
return Object.assign(new Error('Route Cancelled'), {
cancelled: true,
})
}

export function hasBasePath(path: string): boolean {
return path === basePath || path.startsWith(basePath + '/')
}

export function addBasePath(path: string): string {
return basePath
? path === '/'
Expand Down Expand Up @@ -268,7 +272,7 @@ export default class Router implements BaseRouter {
const browserUrl = getURL()
// make sure "as" doesn't start with double slashes or else it can
// throw an error as it's considered invalid
if (as.substr(0, 2) !== '//' && browserUrl.startsWith(basePath)) {
if (as.substr(0, 2) !== '//' && hasBasePath(browserUrl)) {
// in order for `e.state` to work on the `onpopstate` event
// we have to register the initial route upon initialization
// if it doesn't start with the basePath, it's to be treated as an external url
Expand Down Expand Up @@ -335,14 +339,17 @@ export default class Router implements BaseRouter {
// Actually, for (1) we don't need to nothing. But it's hard to detect that event.
// So, doing the following for (1) does no harm.
const browserUrl = getURL()
if (browserUrl.startsWith(basePath)) {
// if it doesn't start with the basePath, it's to be treated as an external url
console.log(document.location.href, e)
if (hasBasePath(browserUrl)) {
const { pathname, query } = this
this.changeState(
'replaceState',
formatWithValidation({ pathname, query }),
delBasePath(browserUrl)
)
} else {
// if it doesn't start with the basePath, it's to be treated as an external url
window.location.reload()
}
return
}
Expand Down
21 changes: 21 additions & 0 deletions test/integration/basepath/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,27 @@ const runTests = (context, dev = false) => {
expect(pathname).toBe('/missing')
})

it('should handle 404 urls that start with basePath', async () => {
const browser = await webdriver(context.appPort, '/docshello')
expect(await browser.eval(() => window.next.router.asPath)).toBe(
'/docshello'
)
expect(await browser.eval(() => window.location.pathname)).toBe(
'/docshello'
)
})

it('should navigating back to a non-basepath 404 that starts with basepath', async () => {
const browser = await webdriver(context.appPort, '/docshello')
await browser.eval(() => window.next.router.push('/hello'))
await browser.waitForElementByCss('#pathname')
await browser.back()
check(() => browser.eval(() => window.location.pathname), '/docshello')
expect(await browser.eval(() => window.next.router.asPath)).toBe(
'/docshello'
)
})

it('should update dynamic params after mount correctly', async () => {
const browser = await webdriver(context.appPort, '/docs/hello-dynamic')
const text = await browser.elementByCss('#slug').text()
Expand Down
2 changes: 1 addition & 1 deletion test/integration/build-output/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ describe('Build Output', () => {
expect(parseFloat(err404FirstLoad) - 63).toBeLessThanOrEqual(0)
expect(err404FirstLoad.endsWith('kB')).toBe(true)

expect(parseFloat(sharedByAll) - 59.3).toBeLessThanOrEqual(0)
expect(parseFloat(sharedByAll) - 59.4).toBeLessThanOrEqual(0)
expect(sharedByAll.endsWith('kB')).toBe(true)

if (_appSize.endsWith('kB')) {
Expand Down

0 comments on commit 80f3ada

Please sign in to comment.