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

Preserve asPath while resolving rewrites #21410

Merged
merged 5 commits into from Jan 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions packages/next/build/webpack-config.ts
Expand Up @@ -213,7 +213,7 @@ export default async function getBaseWebpackConfig(
let plugins: PluginMetaData[] = []
let babelPresetPlugins: { dir: string; config: any }[] = []

const hasRewrites = rewrites.length > 0 || dev
const hasRewrites = rewrites.length > 0

if (config.experimental.plugins) {
plugins = await collectPlugins(dir, config.env, config.plugins)
Expand Down Expand Up @@ -344,6 +344,9 @@ export default async function getBaseWebpackConfig(
const clientResolveRewrites = require.resolve(
'next/dist/next-server/lib/router/utils/resolve-rewrites'
)
const clientResolveRewritesNoop = require.resolve(
'next/dist/next-server/lib/router/utils/resolve-rewrites-noop'
)

const resolveConfig = {
// Disable .mjs for node_modules bundling
Expand Down Expand Up @@ -385,7 +388,7 @@ export default async function getBaseWebpackConfig(
...getReactProfilingInProduction(),
[clientResolveRewrites]: hasRewrites
? clientResolveRewrites
: require.resolve('next/dist/client/dev/noop.js'),
: clientResolveRewritesNoop,
},
mainFields: isServer ? ['main', 'module'] : ['browser', 'module', 'main'],
plugins: isWebpack5
Expand Down
39 changes: 13 additions & 26 deletions packages/next/next-server/lib/router/router.ts
Expand Up @@ -932,39 +932,23 @@ export default class Router implements BaseRouter {
let resolvedAs = as

if (process.env.__NEXT_HAS_REWRITES && as.startsWith('/')) {
resolvedAs = resolveRewrites(
addBasePath(
addLocale(delBasePath(parseRelativeUrl(as).pathname), this.locale)
),
const rewritesResult = resolveRewrites(
addBasePath(addLocale(delBasePath(as), this.locale)),
pages,
rewrites,
query,
(p: string) => this._resolveHref({ pathname: p }, pages).pathname!,
this.locales
)
resolvedAs = rewritesResult.asPath

if (resolvedAs !== as) {
const potentialHref = removePathTrailingSlash(
this._resolveHref(
Object.assign({}, parsed, {
pathname: normalizeLocalePath(
hasBasePath(resolvedAs) ? delBasePath(resolvedAs) : resolvedAs,
this.locales
).pathname,
}),
pages,
false
).pathname!
)

if (rewritesResult.matchedPage && rewritesResult.resolvedHref) {
// if this directly matches a page we need to update the href to
// allow the correct page chunk to be loaded
if (pages.includes(potentialHref)) {
route = potentialHref
pathname = potentialHref
parsed.pathname = pathname
url = formatWithValidation(parsed)
}
route = rewritesResult.resolvedHref
pathname = rewritesResult.resolvedHref
parsed.pathname = pathname
url = formatWithValidation(parsed)
}
}

Expand Down Expand Up @@ -1045,7 +1029,8 @@ export default class Router implements BaseRouter {
route,
pathname,
query,
addBasePath(addLocale(resolvedAs, this.locale)),
as,
resolvedAs,
routeProps
)
let { error, props, __N_SSG, __N_SSP } = routeInfo
Expand Down Expand Up @@ -1092,6 +1077,7 @@ export default class Router implements BaseRouter {
notFoundRoute,
query,
as,
resolvedAs,
{ shallow: false }
)
}
Expand Down Expand Up @@ -1259,6 +1245,7 @@ export default class Router implements BaseRouter {
pathname: string,
query: any,
as: string,
resolvedAs: string,
routeProps: RouteProperties
): Promise<PrivateRouteInfo> {
try {
Expand Down Expand Up @@ -1298,7 +1285,7 @@ export default class Router implements BaseRouter {
if (__N_SSG || __N_SSP) {
dataHref = this.pageLoader.getDataHref(
formatWithValidation({ pathname, query }),
delBasePath(as),
resolvedAs,
__N_SSG,
this.locale
)
Expand Down
@@ -0,0 +1 @@
export default function resolveRewrites() {}
44 changes: 32 additions & 12 deletions packages/next/next-server/lib/router/utils/resolve-rewrites.ts
Expand Up @@ -4,6 +4,8 @@ import prepareDestination from './prepare-destination'
import { Rewrite } from '../../../../lib/load-custom-routes'
import { removePathTrailingSlash } from '../../../../client/normalize-trailing-slash'
import { normalizeLocalePath } from '../../i18n/normalize-locale-path'
import { parseRelativeUrl } from './parse-relative-url'
import { delBasePath } from '../router'

const customRouteMatcher = pathMatch(true)

Expand All @@ -14,11 +16,23 @@ export default function resolveRewrites(
query: ParsedUrlQuery,
resolveHref: (path: string) => string,
locales?: string[]
) {
if (!pages.includes(normalizeLocalePath(asPath, locales).pathname)) {
): {
matchedPage: boolean
parsedAs: ReturnType<typeof parseRelativeUrl>
asPath: string
resolvedHref?: string
} {
let matchedPage = false
let parsedAs = parseRelativeUrl(asPath)
let fsPathname = removePathTrailingSlash(
normalizeLocalePath(delBasePath(parsedAs.pathname), locales).pathname
)
let resolvedHref

if (!pages.includes(fsPathname)) {
for (const rewrite of rewrites) {
const matcher = customRouteMatcher(rewrite.source)
const params = matcher(asPath)
const params = matcher(parsedAs.pathname)

if (params) {
if (!rewrite.destination) {
Expand All @@ -31,30 +45,36 @@ export default function resolveRewrites(
query,
true
)
asPath = destRes.parsedDestination.pathname!
parsedAs = destRes.parsedDestination
asPath = destRes.newUrl
Object.assign(query, destRes.parsedDestination.query)

const fsPathname = normalizeLocalePath(
removePathTrailingSlash(asPath),
locales
).pathname
fsPathname = removePathTrailingSlash(
normalizeLocalePath(delBasePath(asPath), locales).pathname
)

if (pages.includes(fsPathname)) {
asPath = fsPathname
// check if we now match a page as this means we are done
// resolving the rewrites
matchedPage = true
resolvedHref = fsPathname
break
}

// check if we match a dynamic-route, if so we break the rewrites chain
const resolvedHref = resolveHref(fsPathname)
resolvedHref = resolveHref(fsPathname)

if (resolvedHref !== asPath && pages.includes(resolvedHref)) {
asPath = fsPathname
matchedPage = true
break
}
}
}
}
return asPath
return {
asPath,
parsedAs,
matchedPage,
resolvedHref,
}
}
16 changes: 14 additions & 2 deletions test/integration/custom-routes-i18n/test/index.test.js
Expand Up @@ -94,7 +94,13 @@ const runTests = () => {
await browser.elementByCss('#to-about').click()

await check(async () => {
const data = JSON.parse(await browser.elementByCss('#data').text())
const data = JSON.parse(
cheerio
.load(await browser.eval('document.documentElement.innerHTML'))(
'#data'
)
.text()
)
console.log(data)
return data.url === `${expectedIndex ? '/fr' : ''}/about`
? 'success'
Expand All @@ -108,7 +114,13 @@ const runTests = () => {
.click()

await check(async () => {
const data = JSON.parse(await browser.elementByCss('#data').text())
const data = JSON.parse(
cheerio
.load(await browser.eval('document.documentElement.innerHTML'))(
'#data'
)
.text()
)
console.log(data)
return data.url === `${expectedIndex ? '/fr' : ''}/hello`
? 'success'
Expand Down
22 changes: 22 additions & 0 deletions test/integration/production/test/index.test.js
Expand Up @@ -10,6 +10,7 @@ import {
stopApp,
waitFor,
getPageFileFromPagesManifest,
check,
} from 'next-test-utils'
import webdriver from 'next-webdriver'
import {
Expand Down Expand Up @@ -860,6 +861,27 @@ describe('Production Usage', () => {
expect(missing).toBe(false)
})

it('should preserve query when hard navigating from page 404', async () => {
const browser = await webdriver(appPort, '/')
await browser.eval(`(function() {
window.beforeNav = 1
window.next.router.push({
pathname: '/non-existent',
query: { hello: 'world' }
})
})()`)

await check(
() => browser.eval('document.documentElement.innerHTML'),
/page could not be found/
)

expect(await browser.eval('window.beforeNav')).toBe(null)
expect(await browser.eval('window.location.hash')).toBe('')
expect(await browser.eval('window.location.search')).toBe('?hello=world')
expect(await browser.eval('window.location.pathname')).toBe('/non-existent')
})

dynamicImportTests(context, (p, q) => renderViaHTTP(context.appPort, p, q))

processEnv(context)
Expand Down