Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add basePath in link component and add/remove it consistently #9988

Merged
merged 13 commits into from
Apr 14, 2020
7 changes: 6 additions & 1 deletion packages/next/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ const nextServerlessLoader: loader.Loader = function() {
runtimeConfig: runtimeConfig.publicRuntimeConfig || {},
previewProps: ${encodedPreviewProps},
env: process.env,
basePath: "${basePath}",
..._renderOpts
}
let _nextData = false
Expand Down
5 changes: 3 additions & 2 deletions packages/next/client/link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -161,8 +162,8 @@ class Link extends Component<LinkProps> {
// 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,
}
})

Expand Down
1 change: 1 addition & 0 deletions packages/next/client/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const urlPropertyFields = [
'asPath',
'components',
'isFallback',
'basePath',
]
const routerEvents = [
'routeChangeStart',
Expand Down
1 change: 1 addition & 0 deletions packages/next/export/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
36 changes: 24 additions & 12 deletions packages/next/next-server/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -38,6 +44,7 @@ export type BaseRouter = {
pathname: string
query: ParsedUrlQuery
asPath: string
basePath: string
}

export type NextRouter = BaseRouter &
Expand Down Expand Up @@ -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`
*/
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
// "<page>/index.html" directly for the SSR page.
if (process.env.__NEXT_EXPORT_TRAILING_SLASH) {
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
})
}
Expand All @@ -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)

Expand Down
17 changes: 10 additions & 7 deletions packages/next/next-server/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
22 changes: 16 additions & 6 deletions packages/next/next-server/server/render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -156,6 +158,7 @@ export type RenderOptsPartial = {
isDataReq?: boolean
params?: ParsedUrlQuery
previewProps: __ApiPreviewProps
basePath: string
}

export type RenderOpts = LoadComponentsReturnType & RenderOptsPartial
Expand Down Expand Up @@ -309,6 +312,7 @@ export async function renderToHTML(
isDataReq,
params,
previewProps,
basePath,
} = renderOpts

const callMiddleware = async (method: string, args: any[], props = false) => {
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 9 additions & 5 deletions test/integration/basepath/pages/hello.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import Link from 'next/link'
import { useRouter } from 'next/router'

export default () => (
<Link href="/other-page">
<a id="other-page-link">
<h1>Hello World</h1>
</a>
</Link>
<>
<Link href="/other-page">
<a id="other-page-link">
<h1>Hello World</h1>
</a>
</Link>
<div id="base-path">{useRouter().basePath}</div>
</>
)
20 changes: 20 additions & 0 deletions test/integration/basepath/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/* global jasmine */
import webdriver from 'next-webdriver'
import { join } from 'path'
import url from 'url'
import {
nextServer,
launchApp,
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
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 @@ -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
})
Expand Down