From c3071c5c7cb865762ca93338bcd3e9d165be446a Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 21 Apr 2022 16:54:43 -0500 Subject: [PATCH] Fix failing E2E deployment test cases --- .../next-serverless-loader/api-handler.ts | 3 +- .../next-serverless-loader/page-handler.ts | 2 +- .../loaders/next-serverless-loader/utils.ts | 12 ++++-- packages/next/server/base-server.ts | 41 ++++++++++++++---- .../lib/router/utils/prepare-destination.ts | 1 + test/e2e/prerender.test.ts | 7 +++- test/production/required-server-files.test.ts | 42 +++++++++++++++++++ .../pages/dynamic/[slug].js | 4 +- 8 files changed, 97 insertions(+), 15 deletions(-) diff --git a/packages/next/build/webpack/loaders/next-serverless-loader/api-handler.ts b/packages/next/build/webpack/loaders/next-serverless-loader/api-handler.ts index 0e699a74d5cd4dc..7c842f4f7bc4de5 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader/api-handler.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader/api-handler.ts @@ -30,7 +30,8 @@ export function getApiHandler(ctx: ServerlessHandlerCtx) { // We need to trust the dynamic route params from the proxy // to ensure we are using the correct values const trustQuery = req.headers[vercelHeader] - const parsedUrl = handleRewrites(req, parseUrl(req.url!, true)) + const parsedUrl = parseUrl(req.url!, true) + handleRewrites(req, parsedUrl) if (parsedUrl.query.nextInternalLocale) { delete parsedUrl.query.nextInternalLocale diff --git a/packages/next/build/webpack/loaders/next-serverless-loader/page-handler.ts b/packages/next/build/webpack/loaders/next-serverless-loader/page-handler.ts index 0ac97927d3a109e..f15b47b4d3e10fd 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader/page-handler.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader/page-handler.ts @@ -139,7 +139,7 @@ export function getPageHandler(ctx: ServerlessHandlerCtx) { } const origQuery = Object.assign({}, parsedUrl.query) - parsedUrl = handleRewrites(req, parsedUrl) + handleRewrites(req, parsedUrl) handleBasePath(req, parsedUrl) // remove ?amp=1 from request URL if rendering for export diff --git a/packages/next/build/webpack/loaders/next-serverless-loader/utils.ts b/packages/next/build/webpack/loaders/next-serverless-loader/utils.ts index 13afb72bfa34dca..338e111e0573d2e 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader/utils.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader/utils.ts @@ -91,6 +91,8 @@ export function getUtils({ req: BaseNextRequest | IncomingMessage, parsedUrl: UrlWithParsedQuery ) { + const rewriteParams = {} + for (const rewrite of rewrites) { const matcher = getCustomRouteMatcher(rewrite.source) let params = matcher(parsedUrl.pathname) @@ -106,13 +108,14 @@ export function getUtils({ } if (params) { - const { parsedDestination } = prepareDestination({ + const { parsedDestination, destQuery } = prepareDestination({ appendParamsToQuery: true, destination: rewrite.destination, params: params, query: parsedUrl.query, }) + Object.assign(rewriteParams, destQuery, params) Object.assign(parsedUrl.query, parsedDestination.query) delete (parsedDestination as any).query @@ -152,7 +155,7 @@ export function getUtils({ } } - return parsedUrl + return rewriteParams } function handleBasePath( @@ -285,7 +288,8 @@ export function getUtils({ function normalizeVercelUrl( req: BaseNextRequest | IncomingMessage, - trustQuery: boolean + trustQuery: boolean, + paramKeys?: string[] ) { // make sure to normalize req.url on Vercel to strip dynamic params // from the query which are added during routing @@ -293,7 +297,7 @@ export function getUtils({ const _parsedUrl = parseUrl(req.url!, true) delete (_parsedUrl as any).search - for (const param of Object.keys(defaultRouteRegex.groups)) { + for (const param of paramKeys || Object.keys(defaultRouteRegex.groups)) { delete _parsedUrl.query[param] } req.url = formatUrl(_parsedUrl) diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts index 2e1f835e2706da7..4df12587419f03f 100644 --- a/packages/next/server/base-server.ts +++ b/packages/next/server/base-server.ts @@ -440,21 +440,26 @@ export default abstract class Server { typeof req.headers['x-matched-path'] === 'string' ) { const reqUrlIsDataUrl = req.url?.includes('/_next/data') + const parsedMatchedPath = parseUrl(req.headers['x-matched-path'] || '') const matchedPathIsDataUrl = - req.headers['x-matched-path']?.includes('/_next/data') + parsedMatchedPath.pathname?.includes('/_next/data') const isDataUrl = reqUrlIsDataUrl || matchedPathIsDataUrl let parsedPath = parseUrl( isDataUrl ? req.url! : (req.headers['x-matched-path'] as string), true ) - let matchedPathname = parsedPath.pathname! let matchedPathnameNoExt = isDataUrl ? matchedPathname.replace(/\.json$/, '') : matchedPathname + let srcPathname = isDataUrl + ? parsedMatchedPath.pathname?.replace(/\.json$/, '') || + matchedPathnameNoExt + : matchedPathnameNoExt + if (this.nextConfig.i18n) { const localePathResult = normalizeLocalePath( matchedPathname || '/', @@ -469,9 +474,10 @@ export default abstract class Server { if (isDataUrl) { matchedPathname = denormalizePagePath(matchedPathname) matchedPathnameNoExt = denormalizePagePath(matchedPathnameNoExt) + srcPathname = denormalizePagePath(srcPathname) } - const pageIsDynamic = isDynamicRoute(matchedPathnameNoExt) + const pageIsDynamic = isDynamicRoute(srcPathname) const combinedRewrites: Rewrite[] = [] combinedRewrites.push(...this.customRoutes.rewrites.beforeFiles) @@ -480,7 +486,7 @@ export default abstract class Server { const utils = getUtils({ pageIsDynamic, - page: matchedPathnameNoExt, + page: srcPathname, i18n: this.nextConfig.i18n, basePath: this.nextConfig.basePath, rewrites: combinedRewrites, @@ -492,7 +498,15 @@ export default abstract class Server { if (this.nextConfig.i18n && !url.locale?.path.detectedLocale) { parsedUrl.pathname = `/${url.locale?.locale}${parsedUrl.pathname}` } - utils.handleRewrites(req, parsedUrl) + const pathnameBeforeRewrite = parsedUrl.pathname + const rewriteParams = utils.handleRewrites(req, parsedUrl) + const rewriteParamKeys = Object.keys(rewriteParams) + const didRewrite = pathnameBeforeRewrite !== parsedUrl.pathname + + if (didRewrite) { + addRequestMeta(req, '_nextRewroteUrl', parsedUrl.pathname!) + addRequestMeta(req, '_nextDidRewrite', true) + } // interpolate dynamic params and normalize URL if needed if (pageIsDynamic) { @@ -538,9 +552,14 @@ export default abstract class Server { pathname: matchedPathname, }) } - Object.assign(parsedUrl.query, params) - utils.normalizeVercelUrl(req, true) + } + + if (pageIsDynamic || didRewrite) { + utils.normalizeVercelUrl(req, true, [ + ...rewriteParamKeys, + ...Object.keys(utils.defaultRouteRegex?.groups || {}), + ]) } } catch (err) { if (err instanceof DecodeError) { @@ -1368,6 +1387,14 @@ export default abstract class Server { isRedirect = renderResult.renderOpts.isRedirect } else { const origQuery = parseUrl(req.url || '', true).query + + // clear any dynamic route params so they aren't in + // the resolvedUrl + if (opts.params) { + Object.keys(opts.params).forEach((key) => { + delete origQuery[key] + }) + } const hadTrailingSlash = urlPathname !== '/' && this.nextConfig.trailingSlash diff --git a/packages/next/shared/lib/router/utils/prepare-destination.ts b/packages/next/shared/lib/router/utils/prepare-destination.ts index 5653948c4fd9684..52a2bbe689cc5c0 100644 --- a/packages/next/shared/lib/router/utils/prepare-destination.ts +++ b/packages/next/shared/lib/router/utils/prepare-destination.ts @@ -217,6 +217,7 @@ export function prepareDestination(args: { return { newUrl, + destQuery, parsedDestination, } } diff --git a/test/e2e/prerender.test.ts b/test/e2e/prerender.test.ts index 82c5f215539c44e..8ac991b0699dc35 100644 --- a/test/e2e/prerender.test.ts +++ b/test/e2e/prerender.test.ts @@ -844,7 +844,12 @@ describe('Prerender', () => { it('should 404 for an invalid data url', async () => { const res = await fetchViaHTTP(next.url, `/_next/data/${next.buildId}`) - expect(res.status).toBe(404) + + // when deployed this will match due to `index.json` matching the + // directory itself + if (!isDeploy) { + expect(res.status).toBe(404) + } }) it('should allow rewriting to SSG page with fallback: false', async () => { diff --git a/test/production/required-server-files.test.ts b/test/production/required-server-files.test.ts index dfbb46f0b0f69e3..b1bdbba48aa5464 100644 --- a/test/production/required-server-files.test.ts +++ b/test/production/required-server-files.test.ts @@ -54,6 +54,10 @@ describe('should set-up next', () => { source: '/some-catch-all/:path*', destination: '/', }, + { + source: '/to-dynamic/post-2', + destination: '/dynamic/post-2?hello=world', + }, { source: '/to-dynamic/:path', destination: '/dynamic/:path', @@ -667,6 +671,44 @@ describe('should set-up next', () => { expect(await res.text()).toContain('Bad Request') }) + it('should have correct resolvedUrl from rewrite', async () => { + const res = await fetchViaHTTP(appPort, '/to-dynamic/post-1', undefined, { + headers: { + 'x-matched-path': '/dynamic/[slug]', + }, + }) + expect(res.status).toBe(200) + const $ = cheerio.load(await res.text()) + expect($('#resolved-url').text()).toBe('/dynamic/post-1') + }) + + it('should have correct resolvedUrl from rewrite with added query', async () => { + const res = await fetchViaHTTP(appPort, '/to-dynamic/post-2', undefined, { + headers: { + 'x-matched-path': '/dynamic/[slug]', + }, + }) + expect(res.status).toBe(200) + const $ = cheerio.load(await res.text()) + expect($('#resolved-url').text()).toBe('/dynamic/post-2') + }) + + it('should have correct resolvedUrl from dynamic route', async () => { + const res = await fetchViaHTTP( + appPort, + `/_next/data/${next.buildId}/dynamic/post-2.json`, + { slug: 'post-2' }, + { + headers: { + 'x-matched-path': '/dynamic/[slug]', + }, + } + ) + expect(res.status).toBe(200) + const json = await res.json() + expect(json.pageProps.resolvedUrl).toBe('/dynamic/post-2') + }) + it('should bubble error correctly for gip page', async () => { errors = [] const res = await fetchViaHTTP(appPort, '/errors/gip', { crash: '1' }) diff --git a/test/production/required-server-files/pages/dynamic/[slug].js b/test/production/required-server-files/pages/dynamic/[slug].js index b5a19a314ad080d..6f4aa1145642afc 100644 --- a/test/production/required-server-files/pages/dynamic/[slug].js +++ b/test/production/required-server-files/pages/dynamic/[slug].js @@ -1,8 +1,9 @@ import { useRouter } from 'next/router' -export const getServerSideProps = ({ params }) => { +export const getServerSideProps = ({ params, resolvedUrl }) => { return { props: { + resolvedUrl, hello: 'world', slug: params.slug, random: Math.random(), @@ -16,6 +17,7 @@ export default function Page(props) { <>

dynamic page

{props.slug}

+

{props.resolvedUrl}

{JSON.stringify(router)}

{JSON.stringify(props)}