From d3e308a79fa3b218d77cbc5735a802f0faa22a6d Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 14 Apr 2020 02:50:39 -0500 Subject: [PATCH] Add basePath in link component and add/remove it consistently (#9988) * Add basePath in link component and add/remove it consistently * Update to not use regex for delBasePath * Expose addBasePath as router method * Revert "Expose addBasePath as router method" This reverts commit 40fed596195c6affabf837e42d472452768e13a3. * Expose basePath as router field * Apply suggestion * Expose basePath as router field * remove un-used vars * Update externals * Apply lint fix * Update size-limit test * Update prefetch --- packages/next/build/webpack-config.ts | 7 +++- .../webpack/loaders/next-serverless-loader.ts | 1 + packages/next/client/link.tsx | 5 +-- packages/next/client/router.ts | 1 + packages/next/export/index.ts | 1 + .../next/next-server/lib/router/router.ts | 36 ++++++++++++------- .../next/next-server/server/next-server.ts | 17 +++++---- packages/next/next-server/server/render.tsx | 22 ++++++++---- test/integration/basepath/pages/hello.js | 14 +++++--- test/integration/basepath/test/index.test.js | 20 +++++++++++ .../integration/size-limit/test/index.test.js | 2 +- 11 files changed, 92 insertions(+), 34 deletions(-) diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts index 3910972ad21af..74f647d1c7f4e 100644 --- a/packages/next/build/webpack-config.ts +++ b/packages/next/build/webpack-config.ts @@ -540,7 +540,12 @@ export default async function getBaseWebpackConfig( let isNextExternal: boolean = false if (isLocal) { - isNextExternal = /next[/\\]dist[/\\]next-server[/\\]/.test(res) + // we need to process next-server/lib/router/router so that + // the DefinePlugin can inject process.env values + isNextExternal = /next[/\\]dist[/\\]next-server[/\\](?!lib[/\\]router[/\\]router)/.test( + res + ) + if (!isNextExternal) { return callback() } diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts index f306df06eac30..a496f6a850419 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts @@ -266,6 +266,7 @@ const nextServerlessLoader: loader.Loader = function() { runtimeConfig: runtimeConfig.publicRuntimeConfig || {}, previewProps: ${encodedPreviewProps}, env: process.env, + basePath: "${basePath}", ..._renderOpts } let _nextData = false diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index c13fd1bc2bfac..1da480e15dee5 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -9,6 +9,7 @@ import { getLocationOrigin, } from '../next-server/lib/utils' import Router from './router' +import { addBasePath } from '../next-server/lib/router/router' function isLocal(href: string) { const url = parse(href, false, true) @@ -161,8 +162,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/client/router.ts b/packages/next/client/router.ts index 6de604eada93a..acee032740b9d 100644 --- a/packages/next/client/router.ts +++ b/packages/next/client/router.ts @@ -36,6 +36,7 @@ const urlPropertyFields = [ 'asPath', 'components', 'isFallback', + 'basePath', ] const routerEvents = [ 'routeChangeStart', diff --git a/packages/next/export/index.ts b/packages/next/export/index.ts index c202c2d50eff7..4e78f6bd9f446 100644 --- a/packages/next/export/index.ts +++ b/packages/next/export/index.ts @@ -238,6 +238,7 @@ export default async function( dev: false, staticMarkup: false, hotReloader: null, + basePath: nextConfig.experimental.basePath, canonicalBase: nextConfig.amp?.canonicalBase || '', isModern: nextConfig.experimental.modern, ampValidatorPath: nextConfig.experimental.amp?.validator || undefined, diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index efb02db76feaa..b7084450359bd 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -16,10 +16,16 @@ import { isDynamicRoute } from './utils/is-dynamic' import { getRouteMatcher } from './utils/route-matcher' import { getRouteRegex } from './utils/route-regex' -function addBasePath(path: string): string { - // variable is always a string - const p = process.env.__NEXT_ROUTER_BASEPATH as string - return path.indexOf(p) !== 0 ? p + path : path +const basePath = process.env.__NEXT_ROUTER_BASEPATH as string + +export function addBasePath(path: string): string { + return path.indexOf(basePath) !== 0 ? basePath + path : path +} + +function delBasePath(path: string): string { + return path.indexOf(basePath) === 0 + ? path.substr(basePath.length) || '/' + : path } function toRoute(path: string): string { @@ -38,6 +44,7 @@ export type BaseRouter = { pathname: string query: ParsedUrlQuery asPath: string + basePath: string } export type NextRouter = BaseRouter & @@ -133,6 +140,8 @@ export default class Router implements BaseRouter { pathname: string query: ParsedUrlQuery asPath: string + basePath: string + /** * Map of all components loaded in `Router` */ @@ -206,6 +215,7 @@ export default class Router implements BaseRouter { this.asPath = // @ts-ignore this is temporarily global (attached to window) isDynamicRoute(pathname) && __NEXT_DATA__.autoExport ? pathname : as + this.basePath = basePath this.sub = subscription this.clc = null this._wrapApp = wrapApp @@ -361,9 +371,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) { @@ -386,7 +399,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), options) + this.changeState(method, url, as, options) this.scrollToHash(as) Router.events.emit('hashChangeComplete', as) return resolve(true) @@ -458,7 +471,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) if (process.env.NODE_ENV !== 'production') { const appComp: any = this.components['/_app'].Component @@ -737,12 +750,10 @@ export default class Router implements BaseRouter { if (process.env.NODE_ENV !== 'production') { return } - + const route = delBasePath(toRoute(pathname)) Promise.all([ - this.pageLoader.prefetchData(url, asPath), - this.pageLoader[options.priority ? 'loadPage' : 'prefetch']( - toRoute(pathname) - ), + this.pageLoader.prefetchData(url, delBasePath(asPath)), + this.pageLoader[options.priority ? 'loadPage' : 'prefetch'](route), ]).then(() => resolve(), reject) }) } @@ -752,6 +763,7 @@ export default class Router implements BaseRouter { const cancel = (this.clc = () => { cancelled = true }) + route = delBasePath(route) const componentResult = 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 6d988fc5b58c2..96ef3408a17f6 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -123,6 +123,7 @@ export default class Server { previewProps: __ApiPreviewProps customServer?: boolean ampOptimizerConfig?: { [key: string]: any } + basePath: string } private compression?: Middleware private onErrorMiddleware?: ({ err }: { err: Error }) => Promise @@ -179,6 +180,7 @@ export default class Server { previewProps: this.getPreviewProps(), customServer: customServer === true ? true : undefined, ampOptimizerConfig: this.nextConfig.experimental.amp?.optimizer, + basePath: this.nextConfig.experimental.basePath, } // Only the `publicRuntimeConfig` key is exposed to the client side @@ -265,14 +267,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/packages/next/next-server/server/render.tsx b/packages/next/next-server/server/render.tsx index f3dd4c3f5736a..e30f5423c2933 100644 --- a/packages/next/next-server/server/render.tsx +++ b/packages/next/next-server/server/render.tsx @@ -54,6 +54,7 @@ class ServerRouter implements NextRouter { pathname: string query: ParsedUrlQuery asPath: string + basePath: string events: any isFallback: boolean // TODO: Remove in the next major version, as this would mean the user is adding event listeners in server-side `render` method @@ -63,14 +64,15 @@ class ServerRouter implements NextRouter { pathname: string, query: ParsedUrlQuery, as: string, - { isFallback }: { isFallback: boolean } + { isFallback }: { isFallback: boolean }, + basePath: string ) { this.route = pathname.replace(/\/$/, '') || '/' this.pathname = pathname this.query = query this.asPath = as - this.isFallback = isFallback + this.basePath = basePath } push(): any { noRouter() @@ -156,6 +158,7 @@ export type RenderOptsPartial = { isDataReq?: boolean params?: ParsedUrlQuery previewProps: __ApiPreviewProps + basePath: string } export type RenderOpts = LoadComponentsReturnType & RenderOptsPartial @@ -309,6 +312,7 @@ export async function renderToHTML( isDataReq, params, previewProps, + basePath, } = renderOpts const callMiddleware = async (method: string, args: any[], props = false) => { @@ -452,10 +456,16 @@ export async function renderToHTML( await Loadable.preloadAll() // Make sure all dynamic imports are loaded // url will always be set - const asPath = req.url as string - const router = new ServerRouter(pathname, query, asPath, { - isFallback: isFallback, - }) + const asPath: string = req.url as string + const router = new ServerRouter( + pathname, + query, + asPath, + { + isFallback: isFallback, + }, + basePath + ) const ctx = { err, req: isAutoExport ? undefined : req, diff --git a/test/integration/basepath/pages/hello.js b/test/integration/basepath/pages/hello.js index b72f0beaf5b77..c5064613a454d 100644 --- a/test/integration/basepath/pages/hello.js +++ b/test/integration/basepath/pages/hello.js @@ -1,9 +1,13 @@ import Link from 'next/link' +import { useRouter } from 'next/router' export default () => ( - - -

Hello World

-
- + <> + + +

Hello World

+
+ +
{useRouter().basePath}
+ ) diff --git a/test/integration/basepath/test/index.test.js b/test/integration/basepath/test/index.test.js index e3b19b33ad0a9..f94d6838d96c4 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 { @@ -59,6 +73,12 @@ const runTests = (context, dev = false) => { } }) + it('should have basePath field on Router', async () => { + const html = await renderViaHTTP(context.appPort, '/docs/hello') + const $ = cheerio.load(html) + expect($('#base-path').text()).toBe('/docs') + }) + it('should navigate to the page without refresh', async () => { const browser = await webdriver(context.appPort, '/docs/hello') try { diff --git a/test/integration/size-limit/test/index.test.js b/test/integration/size-limit/test/index.test.js index 59d77021a016c..066b6264c12a2 100644 --- a/test/integration/size-limit/test/index.test.js +++ b/test/integration/size-limit/test/index.test.js @@ -100,7 +100,7 @@ describe('Production response size', () => { ) // These numbers are without gzip compression! - const delta = responseSizesBytes - 165 * 1024 + const delta = responseSizesBytes - 166 * 1024 expect(delta).toBeLessThanOrEqual(1024) // don't increase size more than 1kb expect(delta).toBeGreaterThanOrEqual(-1024) // don't decrease size more than 1kb without updating target })