From 32c26be29c75041e5532425062c783d43b09fc98 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 7 Jan 2020 19:20:36 -0600 Subject: [PATCH] Add basePath in link component and add/remove it consistently --- packages/next/client/link.tsx | 5 ++-- .../next/next-server/lib/router/router.ts | 26 ++++++++++++------- .../next/next-server/server/next-server.ts | 15 ++++++----- test/integration/basepath/test/index.test.js | 14 ++++++++++ 4 files changed, 42 insertions(+), 18 deletions(-) diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index df83a94c54733..b30291c2fb2b7 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -8,6 +8,7 @@ import { formatWithValidation, getLocationOrigin, } from '../next-server/lib/utils' +import { addBasePath } from '../next-server/lib/router/router' function isLocal(href: string) { const url = parse(href, false, true) @@ -150,8 +151,8 @@ class Link extends Component { // as per https://reactjs.org/blog/2018/06/07/you-probably-dont-need-derived-state.html formatUrls = memoizedFormatUrl((href, asHref) => { return { - href: formatUrl(href), - as: asHref ? formatUrl(asHref) : asHref, + href: addBasePath(formatUrl(href)), + as: asHref ? addBasePath(formatUrl(asHref)) : asHref, } }) diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index bc221ecbe2a7e..f29ec6fd84f92 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -17,10 +17,15 @@ import { isDynamicRoute } from './utils/is-dynamic' import { getRouteMatcher } from './utils/route-matcher' import { getRouteRegex } from './utils/route-regex' -function addBasePath(path: string): string { - // @ts-ignore variable is always a string - const p: string = process.env.__NEXT_ROUTER_BASEPATH - return path.indexOf(p) !== 0 ? p + path : path +// @ts-ignore variable is always a string +const basePath: string = process.env.__NEXT_ROUTER_BASEPATH + +export function addBasePath(path: string): string { + return path.indexOf(basePath) !== 0 ? basePath + path : path +} + +function delBasePath(path: string): string { + return path.replace(new RegExp(`^${basePath}`), '') } function toRoute(path: string): string { @@ -274,9 +279,12 @@ export default class Router implements BaseRouter { // If url and as provided as an object representation, // we'll format them into the string version here. - const url = typeof _url === 'object' ? formatWithValidation(_url) : _url + let url = typeof _url === 'object' ? formatWithValidation(_url) : _url let as = typeof _as === 'object' ? formatWithValidation(_as) : _as + url = addBasePath(url) + as = addBasePath(as) + // Add the ending slash to the paths. So, we can serve the // "/index.html" directly for the SSR page. if (process.env.__NEXT_EXPORT_TRAILING_SLASH) { @@ -299,7 +307,7 @@ export default class Router implements BaseRouter { if (!options._h && this.onlyAHashChange(as)) { this.asPath = as Router.events.emit('hashChangeStart', as) - this.changeState(method, url, addBasePath(as)) + this.changeState(method, url, as) this.scrollToHash(as) Router.events.emit('hashChangeComplete', as) return resolve(true) @@ -361,7 +369,7 @@ export default class Router implements BaseRouter { } Router.events.emit('beforeHistoryChange', as) - this.changeState(method, url, addBasePath(as), options) + this.changeState(method, url, as, options) const hash = window.location.hash.substring(1) if (process.env.NODE_ENV !== 'production') { @@ -614,8 +622,7 @@ export default class Router implements BaseRouter { return } - // @ts-ignore pathname is always defined - const route = toRoute(pathname) + const route = delBasePath(toRoute(pathname)) this.pageLoader.prefetch(route).then(resolve, reject) }) } @@ -625,6 +632,7 @@ export default class Router implements BaseRouter { const cancel = (this.clc = () => { cancelled = true }) + route = delBasePath(route) const Component = await this.pageLoader.loadPage(route) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 444173b53ea00..c06f0d3aa451d 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -236,14 +236,15 @@ export default class Server { parsedUrl.query = parseQs(parsedUrl.query) } - if (parsedUrl.pathname!.startsWith(this.nextConfig.experimental.basePath)) { + const { basePath } = this.nextConfig.experimental + + // if basePath is set require it be present + if (basePath && !req.url!.startsWith(basePath)) { + return this.render404(req, res, parsedUrl) + } else { // If replace ends up replacing the full url it'll be `undefined`, meaning we have to default it to `/` - parsedUrl.pathname = - parsedUrl.pathname!.replace( - this.nextConfig.experimental.basePath, - '' - ) || '/' - req.url = req.url!.replace(this.nextConfig.experimental.basePath, '') + parsedUrl.pathname = parsedUrl.pathname!.replace(basePath, '') || '/' + req.url = req.url!.replace(basePath, '') } res.statusCode = 200 diff --git a/test/integration/basepath/test/index.test.js b/test/integration/basepath/test/index.test.js index e3b19b33ad0a9..950f9f8788e58 100644 --- a/test/integration/basepath/test/index.test.js +++ b/test/integration/basepath/test/index.test.js @@ -2,6 +2,7 @@ /* global jasmine */ import webdriver from 'next-webdriver' import { join } from 'path' +import url from 'url' import { nextServer, launchApp, @@ -49,6 +50,19 @@ const runTests = (context, dev = false) => { } }) + it('should have correct href for a link', async () => { + const browser = await webdriver(context.appPort, '/docs/hello') + const href = await browser.elementByCss('a').getAttribute('href') + const { pathname } = url.parse(href) + expect(pathname).toBe('/docs/other-page') + }) + + it('should show 404 for page not under the /docs prefix', async () => { + const text = await renderViaHTTP(context.appPort, '/hello') + expect(text).not.toContain('Hello World') + expect(text).toContain('This page could not be found') + }) + it('should show the other-page page under the /docs prefix', async () => { const browser = await webdriver(context.appPort, '/docs/other-page') try {