Skip to content

Commit

Permalink
Avoid adding basePath when it's not needed (#14535)
Browse files Browse the repository at this point in the history
* Avoid adding basePath when it's not needed

When using the `basePath` setting, on pages with params it will fire a router change. This will pass the url pathname in the `as` param using the `getUrl()` function. This means the `as` path will be sent through already including the `basePath`, leading to `/basePath/basePath/path` which will cause the router to throw an error.

* lint

* Add test case and ensure removal

* Make sure to re-add before changeState

Co-authored-by: JJ Kasper <[email protected]>
  • Loading branch information
anthonyshort and ijjk authored Jun 25, 2020
1 parent fe529c4 commit 61c4cdb
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 3 deletions.
3 changes: 2 additions & 1 deletion packages/next/client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +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 } 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 @@ -48,7 +49,7 @@ envConfig.setConfig({
publicRuntimeConfig: runtimeConfig || {},
})

const asPath = getURL()
const asPath = delBasePath(getURL())

const pageLoader = new PageLoader(buildId, prefix, page)
const register = ([r, f]) => pageLoader.registerPage(r, f)
Expand Down
2 changes: 1 addition & 1 deletion packages/next/next-server/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ export default class Router implements BaseRouter {
this.changeState(
'replaceState',
formatWithValidation({ pathname: addBasePath(pathname), query }),
as
addBasePath(as)
)
}

Expand Down
2 changes: 1 addition & 1 deletion test/integration/basepath/pages/[slug].js
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import { useRouter } from 'next/router'

export default () => <p>slug: {useRouter().query.slug}</p>
export default () => <p id="slug">slug: {useRouter().query.slug}</p>
6 changes: 6 additions & 0 deletions test/integration/basepath/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,12 @@ const runTests = (context, dev = false) => {
})
}

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()
expect(text).toContain('slug: hello-dynamic')
})

it('should navigate to index page with getStaticProps', async () => {
const browser = await webdriver(context.appPort, '/docs/hello')
await browser.eval('window.beforeNavigate = "hi"')
Expand Down

0 comments on commit 61c4cdb

Please sign in to comment.