From c7c84ff5407968d3da8674ef9dfaaff300624923 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 25 Aug 2022 14:25:44 +0100 Subject: [PATCH 01/20] fix: mask flight parameters from middleware --- packages/next/server/utils.ts | 31 ++++++++++---- packages/next/server/web/adapter.ts | 15 ++++--- packages/next/server/web/next-url.ts | 42 +++++++++++++++++++ .../app/app/internal/failure/page.server.js | 3 ++ .../app-dir/app/app/internal/page.server.js | 18 ++++++++ .../app/app/internal/success/page.server.js | 3 ++ test/e2e/app-dir/app/middleware.js | 19 ++++++++- test/e2e/app-dir/index.test.ts | 20 +++++++++ 8 files changed, 135 insertions(+), 16 deletions(-) create mode 100644 test/e2e/app-dir/app/app/internal/failure/page.server.js create mode 100644 test/e2e/app-dir/app/app/internal/page.server.js create mode 100644 test/e2e/app-dir/app/app/internal/success/page.server.js diff --git a/packages/next/server/utils.ts b/packages/next/server/utils.ts index 856f5f7032629f7..dd378fa9e70dd24 100644 --- a/packages/next/server/utils.ts +++ b/packages/next/server/utils.ts @@ -29,17 +29,30 @@ export function isTargetLikeServerless(target: string) { return isServerless || isServerlessTrace } +const INTERNAL_QUERY_NAMES = [ + '__nextFallback', + '__nextLocale', + '__nextDefaultLocale', + '__nextIsNotFound', + // RSC + '__flight__', + '__props__', + // Routing + '__flight_router_state_tree__', +] as const + export function stripInternalQueries(query: NextParsedUrlQuery) { - delete query.__nextFallback - delete query.__nextLocale - delete query.__nextDefaultLocale - delete query.__nextIsNotFound + for (const name of INTERNAL_QUERY_NAMES) { + delete query[name] + } +} - // RSC - delete query.__flight__ - delete query.__props__ - // routing - delete query.__flight_router_state_tree__ +export function stripInternalSearchParams(searchParams: URLSearchParams) { + for (const name of INTERNAL_QUERY_NAMES) { + searchParams.delete(name) + } + + return searchParams } // When react version is >= 18 opt-in using reactRoot diff --git a/packages/next/server/web/adapter.ts b/packages/next/server/web/adapter.ts index 9c40ca31c056236..b94bfff6bae66f6 100644 --- a/packages/next/server/web/adapter.ts +++ b/packages/next/server/web/adapter.ts @@ -8,6 +8,7 @@ import { NextResponse } from './spec-extension/response' import { relativizeURL } from '../../shared/lib/router/utils/relativize-url' import { waitUntilSymbol } from './spec-extension/fetch-event' import { NextURL } from './next-url' +import { stripInternalSearchParams } from '../utils' class NextRequestHint extends NextRequest { sourcePage: string @@ -54,12 +55,12 @@ export async function adapter(params: { requestUrl.pathname = '/' } - // clean-up any internal query params - for (const key of [...requestUrl.searchParams.keys()]) { - if (key.startsWith('__next')) { - requestUrl.searchParams.delete(key) - } - } + // Preserve flight data. + const flightData = requestUrl.flightData + requestUrl.flightData = undefined + + // Strip internal query parameters off the request. + stripInternalSearchParams(requestUrl.searchParams) const request = new NextRequestHint({ page: params.page, @@ -105,6 +106,7 @@ export async function adapter(params: { if (rewriteUrl.host === request.nextUrl.host) { rewriteUrl.buildId = buildId || rewriteUrl.buildId + rewriteUrl.flightData = flightData || rewriteUrl.flightData response.headers.set('x-middleware-rewrite', String(rewriteUrl)) } @@ -142,6 +144,7 @@ export async function adapter(params: { if (redirectURL.host === request.nextUrl.host) { redirectURL.buildId = buildId || redirectURL.buildId + redirectURL.flightData = flightData || redirectURL.flightData response.headers.set('Location', String(redirectURL)) } diff --git a/packages/next/server/web/next-url.ts b/packages/next/server/web/next-url.ts index 118c2820107cda1..c688b4d0fe3de50 100644 --- a/packages/next/server/web/next-url.ts +++ b/packages/next/server/web/next-url.ts @@ -15,6 +15,12 @@ interface Options { } } +const FLIGHT_PARAMETERS = [ + '__flight__', + '__props__', + '__flight_router_state_tree__', +] as const + const REGEX_LOCALHOST_HOSTNAME = /(?!^https?:\/\/)(127(?:\.(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)){3}|::1|localhost)/ @@ -31,6 +37,7 @@ export class NextURL { [Internal]: { basePath: string buildId?: string + flightData?: Record defaultLocale?: string domainLocale?: DomainLocale locale?: string @@ -89,6 +96,21 @@ export class NextURL { this[Internal].buildId = pathnameInfo.buildId this[Internal].locale = pathnameInfo.locale ?? defaultLocale this[Internal].trailingSlash = pathnameInfo.trailingSlash + + // Clear the flight data. + delete this[Internal].flightData + + // Check if there's flight data in the URL, and if there is, extract it. + for (const name of FLIGHT_PARAMETERS) { + const value = this[Internal].url.searchParams.get(name) + if (value !== null) { + // Set the flight data if it wasn't defined. + this[Internal].flightData ??= {} + + // Set this flight parameter. + this[Internal].flightData[name] = value + } + } } private formatPathname() { @@ -112,6 +134,26 @@ export class NextURL { this[Internal].buildId = buildId } + public get flightData() { + return this[Internal].flightData + } + + public set flightData(flightData: Record | undefined) { + if (flightData) { + for (const name of FLIGHT_PARAMETERS) { + this[Internal].url.searchParams.set(name, flightData[name]) + } + + this[Internal].flightData = flightData + } else { + for (const name of FLIGHT_PARAMETERS) { + this[Internal].url.searchParams.delete(name) + } + + delete this[Internal].flightData + } + } + public get locale() { return this[Internal].locale ?? '' } diff --git a/test/e2e/app-dir/app/app/internal/failure/page.server.js b/test/e2e/app-dir/app/app/internal/failure/page.server.js new file mode 100644 index 000000000000000..9bd06ec2a30c81b --- /dev/null +++ b/test/e2e/app-dir/app/app/internal/failure/page.server.js @@ -0,0 +1,3 @@ +export default function Page() { + return
Failure
+} diff --git a/test/e2e/app-dir/app/app/internal/page.server.js b/test/e2e/app-dir/app/app/internal/page.server.js new file mode 100644 index 000000000000000..ddbc93ea5585432 --- /dev/null +++ b/test/e2e/app-dir/app/app/internal/page.server.js @@ -0,0 +1,18 @@ +import Link from 'next/link' + +export default function Page() { + return ( +
+
+ + Navigate Rewrite + +
+
+ + Navigate Redirect + +
+
+ ) +} diff --git a/test/e2e/app-dir/app/app/internal/success/page.server.js b/test/e2e/app-dir/app/app/internal/success/page.server.js new file mode 100644 index 000000000000000..90c6aecd4e4dc48 --- /dev/null +++ b/test/e2e/app-dir/app/app/internal/success/page.server.js @@ -0,0 +1,3 @@ +export default function Page() { + return
Success
+} diff --git a/test/e2e/app-dir/app/middleware.js b/test/e2e/app-dir/app/middleware.js index 389865fff93b371..3727d56df10b685 100644 --- a/test/e2e/app-dir/app/middleware.js +++ b/test/e2e/app-dir/app/middleware.js @@ -1,8 +1,25 @@ +// @ts-check import { NextResponse } from 'next/server' +/** + * @param {import('next/server').NextRequest} request + * @returns {NextResponse | undefined} + */ export function middleware(request) { if (request.nextUrl.pathname === '/middleware-to-dashboard') { - // TODO: this does not copy __flight__ and __flight_router_state_tree__ return NextResponse.rewrite(new URL('/dashboard', request.url)) } + + if (request.nextUrl.pathname.startsWith('/internal/test')) { + const method = request.nextUrl.pathname.endsWith('rewrite') + ? 'rewrite' + : 'redirect' + + const internal = ['__flight__', '__props__', '__flight_router_state_tree__'] + if (internal.some((name) => request.nextUrl.searchParams.has(name))) { + return NextResponse[method](new URL('/internal/failure', request.url)) + } + + return NextResponse[method](new URL('/internal/success', request.url)) + } } diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index 6ff89b27588809b..6f3cc192688c656 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -737,6 +737,26 @@ describe('app dir', () => { }) }) + describe('middleware', () => { + ;['rewrite', 'redirect'].map((method) => + it(`should strip internal query parameters from requests to middleware for ${method}`, async () => { + const browser = await webdriver(next.url, '/internal/test') + + try { + // Wait for and click the navigation element, this should trigger + // the flight request that'll be caught by the middleware. If the + // middleware sees any flight data on the request it'll redirect to + // a page with an element of #failure, otherwise, we'll see the + // element for #success. + await browser.elementById(`navigate-${method}`).click() + await browser.waitForElementByCss('#success') + } finally { + await browser.close() + } + }) + ) + }) + describe('next/router', () => { it('should always return null when accessed from /app', async () => { const browser = await webdriver(next.url, '/old-router') From 91aa0ac7ec305a7dd1b42045f64264285e88f050 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 25 Aug 2022 14:59:51 +0100 Subject: [PATCH 02/20] fix: fixed ts types --- packages/next/server/web/next-url.ts | 48 ++++++++++++++++------------ 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/packages/next/server/web/next-url.ts b/packages/next/server/web/next-url.ts index c688b4d0fe3de50..3455bfd611b10dd 100644 --- a/packages/next/server/web/next-url.ts +++ b/packages/next/server/web/next-url.ts @@ -31,6 +31,28 @@ function parseURL(url: string | URL, base?: string | URL) { ) } +function parseFlightParameters( + searchParams: URLSearchParams +): Record | undefined { + let flightData: Record = {} + let flightDataUpdated = false + for (const name of FLIGHT_PARAMETERS) { + const value = searchParams.get(name) + if (value === null) { + continue + } + + flightData[name] = value + flightDataUpdated = true + } + + if (!flightDataUpdated) { + return undefined + } + + return flightData +} + const Internal = Symbol('NextURLInternal') export class NextURL { @@ -96,21 +118,9 @@ export class NextURL { this[Internal].buildId = pathnameInfo.buildId this[Internal].locale = pathnameInfo.locale ?? defaultLocale this[Internal].trailingSlash = pathnameInfo.trailingSlash - - // Clear the flight data. - delete this[Internal].flightData - - // Check if there's flight data in the URL, and if there is, extract it. - for (const name of FLIGHT_PARAMETERS) { - const value = this[Internal].url.searchParams.get(name) - if (value !== null) { - // Set the flight data if it wasn't defined. - this[Internal].flightData ??= {} - - // Set this flight parameter. - this[Internal].flightData[name] = value - } - } + this[Internal].flightData = parseFlightParameters( + this[Internal].url.searchParams + ) } private formatPathname() { @@ -141,17 +151,15 @@ export class NextURL { public set flightData(flightData: Record | undefined) { if (flightData) { for (const name of FLIGHT_PARAMETERS) { - this[Internal].url.searchParams.set(name, flightData[name]) + this[Internal].url.searchParams.set(name, flightData[name] ?? '') } - - this[Internal].flightData = flightData } else { for (const name of FLIGHT_PARAMETERS) { this[Internal].url.searchParams.delete(name) } - - delete this[Internal].flightData } + + this[Internal].flightData = flightData } public get locale() { From cc28c2405e1d87b0597a83233799ad174b9a7ef2 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 25 Aug 2022 15:06:35 +0100 Subject: [PATCH 03/20] fix: moved internal utils to separate file to avoid poluting edge runtime --- packages/next/server/app-render.tsx | 3 ++- packages/next/server/internal-utils.ts | 27 ++++++++++++++++++++++++++ packages/next/server/render.tsx | 3 ++- packages/next/server/utils.ts | 27 -------------------------- packages/next/server/web/adapter.ts | 2 +- 5 files changed, 32 insertions(+), 30 deletions(-) create mode 100644 packages/next/server/internal-utils.ts diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index 24f651fcf73716c..b48ef722a46d007 100644 --- a/packages/next/server/app-render.tsx +++ b/packages/next/server/app-render.tsx @@ -18,7 +18,7 @@ import { } from './node-web-streams-helper' import { isDynamicRoute } from '../shared/lib/router/utils' import { htmlEscapeJsonString } from './htmlescape' -import { shouldUseReactRoot, stripInternalQueries } from './utils' +import { shouldUseReactRoot } from './utils' import { NextApiRequestCookies } from './api-utils' import { matchSegment } from '../client/components/match-segments' import { @@ -26,6 +26,7 @@ import { FlightManifest, } from '../build/webpack/plugins/flight-manifest-plugin' import { FlushEffectsContext } from '../client/components/hooks-client' +import { stripInternalQueries } from './internal-utils' // this needs to be required lazily so that `next-server` can set // the env before we require diff --git a/packages/next/server/internal-utils.ts b/packages/next/server/internal-utils.ts new file mode 100644 index 000000000000000..e77842d13983c8b --- /dev/null +++ b/packages/next/server/internal-utils.ts @@ -0,0 +1,27 @@ +import type { NextParsedUrlQuery } from './request-meta' + +const INTERNAL_QUERY_NAMES = [ + '__nextFallback', + '__nextLocale', + '__nextDefaultLocale', + '__nextIsNotFound', + // RSC + '__flight__', + '__props__', + // Routing + '__flight_router_state_tree__', +] as const + +export function stripInternalQueries(query: NextParsedUrlQuery) { + for (const name of INTERNAL_QUERY_NAMES) { + delete query[name] + } +} + +export function stripInternalSearchParams(searchParams: URLSearchParams) { + for (const name of INTERNAL_QUERY_NAMES) { + searchParams.delete(name) + } + + return searchParams +} diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx index c352429e7bf6ddd..16cb8d389c6b087 100644 --- a/packages/next/server/render.tsx +++ b/packages/next/server/render.tsx @@ -79,7 +79,8 @@ import { } from './node-web-streams-helper' import { ImageConfigContext } from '../shared/lib/image-config-context' import stripAnsi from 'next/dist/compiled/strip-ansi' -import { shouldUseReactRoot, stripInternalQueries } from './utils' +import { shouldUseReactRoot } from './utils' +import { stripInternalQueries } from './internal-utils' let tryGetPreviewData: typeof import('./api-utils/node').tryGetPreviewData let warn: typeof import('../build/output/log').warn diff --git a/packages/next/server/utils.ts b/packages/next/server/utils.ts index dd378fa9e70dd24..da8c7cb6883fbe5 100644 --- a/packages/next/server/utils.ts +++ b/packages/next/server/utils.ts @@ -1,4 +1,3 @@ -import type { NextParsedUrlQuery } from './request-meta' import React from 'react' import { BLOCKED_PAGES } from '../shared/lib/constants' @@ -29,31 +28,5 @@ export function isTargetLikeServerless(target: string) { return isServerless || isServerlessTrace } -const INTERNAL_QUERY_NAMES = [ - '__nextFallback', - '__nextLocale', - '__nextDefaultLocale', - '__nextIsNotFound', - // RSC - '__flight__', - '__props__', - // Routing - '__flight_router_state_tree__', -] as const - -export function stripInternalQueries(query: NextParsedUrlQuery) { - for (const name of INTERNAL_QUERY_NAMES) { - delete query[name] - } -} - -export function stripInternalSearchParams(searchParams: URLSearchParams) { - for (const name of INTERNAL_QUERY_NAMES) { - searchParams.delete(name) - } - - return searchParams -} - // When react version is >= 18 opt-in using reactRoot export const shouldUseReactRoot = parseInt(React.version) >= 18 diff --git a/packages/next/server/web/adapter.ts b/packages/next/server/web/adapter.ts index b94bfff6bae66f6..1539eb1ca7b2654 100644 --- a/packages/next/server/web/adapter.ts +++ b/packages/next/server/web/adapter.ts @@ -8,7 +8,7 @@ import { NextResponse } from './spec-extension/response' import { relativizeURL } from '../../shared/lib/router/utils/relativize-url' import { waitUntilSymbol } from './spec-extension/fetch-event' import { NextURL } from './next-url' -import { stripInternalSearchParams } from '../utils' +import { stripInternalSearchParams } from '../internal-utils' class NextRequestHint extends NextRequest { sourcePage: string From 5eda4321daa774d4cceb8c239a05821140ec8e12 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 25 Aug 2022 16:39:12 +0100 Subject: [PATCH 04/20] fix: fixed tests --- test/e2e/app-dir/index.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index 6f3cc192688c656..62d4a6b9ee0e61d 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -740,7 +740,7 @@ describe('app dir', () => { describe('middleware', () => { ;['rewrite', 'redirect'].map((method) => it(`should strip internal query parameters from requests to middleware for ${method}`, async () => { - const browser = await webdriver(next.url, '/internal/test') + const browser = await webdriver(next.url, '/internal') try { // Wait for and click the navigation element, this should trigger From 1910e488b8f4e363985e7998eb6fd5e3ebddd104 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 26 Aug 2022 11:35:29 +0100 Subject: [PATCH 05/20] fix: added middleware file to tests via files ref --- test/e2e/app-dir/index.test.ts | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index 62d4a6b9ee0e61d..3f9b076c2aa2765 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -22,15 +22,7 @@ describe('app dir', () => { function runTests({ assetPrefix }: { assetPrefix?: boolean }) { beforeAll(async () => { next = await createNext({ - files: { - public: new FileRef(path.join(__dirname, 'app/public')), - styles: new FileRef(path.join(__dirname, 'app/styles')), - pages: new FileRef(path.join(__dirname, 'app/pages')), - app: new FileRef(path.join(__dirname, 'app/app')), - 'next.config.js': new FileRef( - path.join(__dirname, 'app/next.config.js') - ), - }, + files: new FileRef(path.join(__dirname, 'app')), dependencies: { react: 'experimental', 'react-dom': 'experimental', @@ -748,6 +740,7 @@ describe('app dir', () => { // middleware sees any flight data on the request it'll redirect to // a page with an element of #failure, otherwise, we'll see the // element for #success. + await browser.waitForElementByCss(`#navigate-${method}`) await browser.elementById(`navigate-${method}`).click() await browser.waitForElementByCss('#success') } finally { From 675e8623b4ac3b4026b7b002602f676b1a90af92 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 26 Aug 2022 13:20:53 +0100 Subject: [PATCH 06/20] fix: added extedned internal param removal --- packages/next/server/internal-utils.ts | 13 ++++++++++++- packages/next/server/web/adapter.ts | 2 +- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/next/server/internal-utils.ts b/packages/next/server/internal-utils.ts index e77842d13983c8b..d2637536d56b75d 100644 --- a/packages/next/server/internal-utils.ts +++ b/packages/next/server/internal-utils.ts @@ -12,16 +12,27 @@ const INTERNAL_QUERY_NAMES = [ '__flight_router_state_tree__', ] as const +const EXTENDED_INTERNAL_QUERY_NAMES = ['__nextDataReq'] as const + export function stripInternalQueries(query: NextParsedUrlQuery) { for (const name of INTERNAL_QUERY_NAMES) { delete query[name] } } -export function stripInternalSearchParams(searchParams: URLSearchParams) { +export function stripInternalSearchParams( + searchParams: URLSearchParams, + extended?: boolean +) { for (const name of INTERNAL_QUERY_NAMES) { searchParams.delete(name) } + if (extended) { + for (const name of EXTENDED_INTERNAL_QUERY_NAMES) { + searchParams.delete(name) + } + } + return searchParams } diff --git a/packages/next/server/web/adapter.ts b/packages/next/server/web/adapter.ts index 1539eb1ca7b2654..c2ed309b588cacb 100644 --- a/packages/next/server/web/adapter.ts +++ b/packages/next/server/web/adapter.ts @@ -60,7 +60,7 @@ export async function adapter(params: { requestUrl.flightData = undefined // Strip internal query parameters off the request. - stripInternalSearchParams(requestUrl.searchParams) + stripInternalSearchParams(requestUrl.searchParams, true) const request = new NextRequestHint({ page: params.page, From 8a62c5eb814c6366cdc1802a43641cc90287a242 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Wed, 14 Sep 2022 12:21:39 +0200 Subject: [PATCH 07/20] Ensure prefetch error does not crash the app --- .../client/components/app-router.client.tsx | 42 ++++++++++--------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/packages/next/client/components/app-router.client.tsx b/packages/next/client/components/app-router.client.tsx index 4f23c4907395f6f..e3d4766652cfa96 100644 --- a/packages/next/client/components/app-router.client.tsx +++ b/packages/next/client/components/app-router.client.tsx @@ -202,28 +202,32 @@ export default function AppRouter({ } prefetched.add(href) - const url = new URL(href, location.origin) - // TODO-APP: handle case where history.state is not the new router history entry - const r = fetchServerResponse( - url, - // initialTree is used when history.state.tree is missing because the history state is set in `useEffect` below, it being missing means this is the hydration case. - window.history.state?.tree || initialTree, - true - ) + try { - r.readRoot() - } catch (e) { - await e - const flightData = r.readRoot() - // @ts-ignore startTransition exists - React.startTransition(() => { - dispatch({ - type: ACTION_PREFETCH, - url, - flightData, + // TODO-APP: handle case where history.state is not the new router history entry + const r = fetchServerResponse( + url, + // initialTree is used when history.state.tree is missing because the history state is set in `useEffect` below, it being missing means this is the hydration case. + window.history.state?.tree || initialTree, + true + ) + try { + r.readRoot() + } catch (e) { + await e + const flightData = r.readRoot() + // @ts-ignore startTransition exists + React.startTransition(() => { + dispatch({ + type: ACTION_PREFETCH, + url, + flightData, + }) }) - }) + } + } catch (err) { + console.error('PREFETCH ERROR', err) } }, replace: (href, options = {}) => { From a8518bdb389b94edcd0b83decad0ef98295a7b25 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Wed, 14 Sep 2022 15:10:22 +0200 Subject: [PATCH 08/20] Remove __props__ special case --- packages/next/server/internal-utils.ts | 1 - packages/next/server/web/next-url.ts | 1 - test/e2e/app-dir/app/middleware.js | 2 +- test/e2e/app-dir/rsc-basic.test.ts | 1 - 4 files changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/next/server/internal-utils.ts b/packages/next/server/internal-utils.ts index d2637536d56b75d..34e2fb024213d89 100644 --- a/packages/next/server/internal-utils.ts +++ b/packages/next/server/internal-utils.ts @@ -7,7 +7,6 @@ const INTERNAL_QUERY_NAMES = [ '__nextIsNotFound', // RSC '__flight__', - '__props__', // Routing '__flight_router_state_tree__', ] as const diff --git a/packages/next/server/web/next-url.ts b/packages/next/server/web/next-url.ts index 3455bfd611b10dd..e051f29f9be5963 100644 --- a/packages/next/server/web/next-url.ts +++ b/packages/next/server/web/next-url.ts @@ -17,7 +17,6 @@ interface Options { const FLIGHT_PARAMETERS = [ '__flight__', - '__props__', '__flight_router_state_tree__', ] as const diff --git a/test/e2e/app-dir/app/middleware.js b/test/e2e/app-dir/app/middleware.js index 3727d56df10b685..84563968b03f98b 100644 --- a/test/e2e/app-dir/app/middleware.js +++ b/test/e2e/app-dir/app/middleware.js @@ -15,7 +15,7 @@ export function middleware(request) { ? 'rewrite' : 'redirect' - const internal = ['__flight__', '__props__', '__flight_router_state_tree__'] + const internal = ['__flight__', '__flight_router_state_tree__'] if (internal.some((name) => request.nextUrl.searchParams.has(name))) { return NextResponse[method](new URL('/internal/failure', request.url)) } diff --git a/test/e2e/app-dir/rsc-basic.test.ts b/test/e2e/app-dir/rsc-basic.test.ts index e548c458445f90d..fb4a6fefdb26e75 100644 --- a/test/e2e/app-dir/rsc-basic.test.ts +++ b/test/e2e/app-dir/rsc-basic.test.ts @@ -94,7 +94,6 @@ describe('app dir - react server components', () => { '__nextDefaultLocale', '__nextIsNotFound', '__flight__', - '__props__', '__flight_router_path__', ] From 98fd35152570894694e7b4eef9fcf1b1cc1aa6b3 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Wed, 14 Sep 2022 15:30:45 +0200 Subject: [PATCH 09/20] Add __flight_prefetch__ --- packages/next/server/internal-utils.ts | 1 + packages/next/server/web/next-url.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/next/server/internal-utils.ts b/packages/next/server/internal-utils.ts index 34e2fb024213d89..1dbfef90cd9231f 100644 --- a/packages/next/server/internal-utils.ts +++ b/packages/next/server/internal-utils.ts @@ -9,6 +9,7 @@ const INTERNAL_QUERY_NAMES = [ '__flight__', // Routing '__flight_router_state_tree__', + '__flight_prefetch__', ] as const const EXTENDED_INTERNAL_QUERY_NAMES = ['__nextDataReq'] as const diff --git a/packages/next/server/web/next-url.ts b/packages/next/server/web/next-url.ts index e051f29f9be5963..f9426beb84f0317 100644 --- a/packages/next/server/web/next-url.ts +++ b/packages/next/server/web/next-url.ts @@ -18,6 +18,7 @@ interface Options { const FLIGHT_PARAMETERS = [ '__flight__', '__flight_router_state_tree__', + '__flight_prefetch__', ] as const const REGEX_LOCALHOST_HOSTNAME = From ffa5b436792885300e26951fb410404a2cc2bbb6 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Wed, 14 Sep 2022 15:36:39 +0200 Subject: [PATCH 10/20] Rename flightData -> flightSearchParameters --- packages/next/server/web/adapter.ts | 10 ++++++---- packages/next/server/web/next-url.ts | 20 +++++++++++--------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/packages/next/server/web/adapter.ts b/packages/next/server/web/adapter.ts index c2ed309b588cacb..7a90119623effa3 100644 --- a/packages/next/server/web/adapter.ts +++ b/packages/next/server/web/adapter.ts @@ -56,8 +56,8 @@ export async function adapter(params: { } // Preserve flight data. - const flightData = requestUrl.flightData - requestUrl.flightData = undefined + const flightSearchParameters = requestUrl.flightSearchParameters + requestUrl.flightSearchParameters = undefined // Strip internal query parameters off the request. stripInternalSearchParams(requestUrl.searchParams, true) @@ -106,7 +106,8 @@ export async function adapter(params: { if (rewriteUrl.host === request.nextUrl.host) { rewriteUrl.buildId = buildId || rewriteUrl.buildId - rewriteUrl.flightData = flightData || rewriteUrl.flightData + rewriteUrl.flightSearchParameters = + flightSearchParameters || rewriteUrl.flightSearchParameters response.headers.set('x-middleware-rewrite', String(rewriteUrl)) } @@ -144,7 +145,8 @@ export async function adapter(params: { if (redirectURL.host === request.nextUrl.host) { redirectURL.buildId = buildId || redirectURL.buildId - redirectURL.flightData = flightData || redirectURL.flightData + redirectURL.flightSearchParameters = + flightSearchParameters || redirectURL.flightSearchParameters response.headers.set('Location', String(redirectURL)) } diff --git a/packages/next/server/web/next-url.ts b/packages/next/server/web/next-url.ts index f9426beb84f0317..5ef4037ad01dffc 100644 --- a/packages/next/server/web/next-url.ts +++ b/packages/next/server/web/next-url.ts @@ -34,7 +34,7 @@ function parseURL(url: string | URL, base?: string | URL) { function parseFlightParameters( searchParams: URLSearchParams ): Record | undefined { - let flightData: Record = {} + let flightSearchParameters: Record = {} let flightDataUpdated = false for (const name of FLIGHT_PARAMETERS) { const value = searchParams.get(name) @@ -42,7 +42,7 @@ function parseFlightParameters( continue } - flightData[name] = value + flightSearchParameters[name] = value flightDataUpdated = true } @@ -50,7 +50,7 @@ function parseFlightParameters( return undefined } - return flightData + return flightSearchParameters } const Internal = Symbol('NextURLInternal') @@ -59,7 +59,7 @@ export class NextURL { [Internal]: { basePath: string buildId?: string - flightData?: Record + flightSearchParameters?: Record defaultLocale?: string domainLocale?: DomainLocale locale?: string @@ -118,7 +118,7 @@ export class NextURL { this[Internal].buildId = pathnameInfo.buildId this[Internal].locale = pathnameInfo.locale ?? defaultLocale this[Internal].trailingSlash = pathnameInfo.trailingSlash - this[Internal].flightData = parseFlightParameters( + this[Internal].flightSearchParameters = parseFlightParameters( this[Internal].url.searchParams ) } @@ -144,11 +144,13 @@ export class NextURL { this[Internal].buildId = buildId } - public get flightData() { - return this[Internal].flightData + public get flightSearchParameters() { + return this[Internal].flightSearchParameters } - public set flightData(flightData: Record | undefined) { + public set flightSearchParameters( + flightData: Record | undefined + ) { if (flightData) { for (const name of FLIGHT_PARAMETERS) { this[Internal].url.searchParams.set(name, flightData[name] ?? '') @@ -159,7 +161,7 @@ export class NextURL { } } - this[Internal].flightData = flightData + this[Internal].flightSearchParameters = flightData } public get locale() { From 61214863604a02cc8a99347b801e66c117a21ead Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Thu, 15 Sep 2022 11:13:14 +0200 Subject: [PATCH 11/20] Ensure flightSearchParameters are added when creating href --- packages/next/server/web/next-url.ts | 48 ++++++++++++++-------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/packages/next/server/web/next-url.ts b/packages/next/server/web/next-url.ts index 5ef4037ad01dffc..b751a5ae1be96d7 100644 --- a/packages/next/server/web/next-url.ts +++ b/packages/next/server/web/next-url.ts @@ -35,7 +35,7 @@ function parseFlightParameters( searchParams: URLSearchParams ): Record | undefined { let flightSearchParameters: Record = {} - let flightDataUpdated = false + let flightSearchParametersUpdated = false for (const name of FLIGHT_PARAMETERS) { const value = searchParams.get(name) if (value === null) { @@ -43,10 +43,10 @@ function parseFlightParameters( } flightSearchParameters[name] = value - flightDataUpdated = true + flightSearchParametersUpdated = true } - if (!flightDataUpdated) { + if (!flightSearchParametersUpdated) { return undefined } @@ -136,6 +136,25 @@ export class NextURL { }) } + private formatSearch() { + const flightSearchParameters = this[Internal].flightSearchParameters + // If no flight parameters are set, return the search string as is. + // This is a fast path to ensure URLSearchParams only has to be recreated on Flight requests. + if (!flightSearchParameters) { + return this[Internal].url.search + } + + // Create separate URLSearchParams to ensure the original search string is not modified. + const searchParams = new URLSearchParams(this[Internal].url.searchParams) + // If any exist this loop is always limited to the amount of FLIGHT_PARAMETERS. + for (const name in flightSearchParameters) { + searchParams.set(name, flightSearchParameters[name]) + } + + const params = searchParams.toString() + return params === '' ? '' : `?${params}` + } + public get buildId() { return this[Internal].buildId } @@ -144,26 +163,6 @@ export class NextURL { this[Internal].buildId = buildId } - public get flightSearchParameters() { - return this[Internal].flightSearchParameters - } - - public set flightSearchParameters( - flightData: Record | undefined - ) { - if (flightData) { - for (const name of FLIGHT_PARAMETERS) { - this[Internal].url.searchParams.set(name, flightData[name] ?? '') - } - } else { - for (const name of FLIGHT_PARAMETERS) { - this[Internal].url.searchParams.delete(name) - } - } - - this[Internal].flightSearchParameters = flightData - } - public get locale() { return this[Internal].locale ?? '' } @@ -227,7 +226,8 @@ export class NextURL { get href() { const pathname = this.formatPathname() - return `${this.protocol}//${this.host}${pathname}${this[Internal].url.search}` + const search = this.formatSearch() + return `${this.protocol}//${this.host}${pathname}${search}` } set href(url: string) { From 400e8412eea16d80be31657a22139fbf087d1020 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Thu, 15 Sep 2022 11:24:46 +0200 Subject: [PATCH 12/20] Ensure test for rewrites is correct --- test/e2e/app-dir/index.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index 5e79693a3ec76b0..fee45f9a5d9cb3b 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -219,8 +219,9 @@ describe('app dir', () => { }) it('should support rewrites', async () => { - const html = await renderViaHTTP(next.url, '/rewritten-to-dashboard') - expect(html).toContain('hello from app/dashboard') + const browser = await webdriver(next.url, '/rewritten-to-dashboard') + expect(await browser.elementByCss('h1')).toContain('Dashboard') + expect(await browser.url()).toBe(`${next.url}/rewritten-to-dashboard`) }) // TODO-APP: Enable in development From 0c77102a57322dddeeb1da0e63eff05d9c63874c Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Thu, 15 Sep 2022 11:29:43 +0200 Subject: [PATCH 13/20] Combine rewrites tests in describe. Replace browser.eval --- test/e2e/app-dir/index.test.ts | 52 +++++++++++++++++----------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index fee45f9a5d9cb3b..221750837f023ee 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -218,10 +218,32 @@ describe('app dir', () => { expect(html).toContain('hello from app/partial-match-[id]. ID is: 123') }) - it('should support rewrites', async () => { - const browser = await webdriver(next.url, '/rewritten-to-dashboard') - expect(await browser.elementByCss('h1')).toContain('Dashboard') - expect(await browser.url()).toBe(`${next.url}/rewritten-to-dashboard`) + describe('rewrites', () => { + it('should support rewrites on initial load', async () => { + const browser = await webdriver(next.url, '/rewritten-to-dashboard') + expect(await browser.elementByCss('h1').text()).toBe('Dashboard') + expect(await browser.url()).toBe(`${next.url}/rewritten-to-dashboard`) + }) + + it('should support rewrites on client-side navigation', async () => { + const browser = await webdriver(next.url, '/rewrites') + + try { + // Click the link. + await browser.elementById('link').click() + await browser.waitForElementByCss('#from-dashboard') + + // Check to see that we were rewritten and not redirected. + expect(await browser.url()).toBe(`${next.url}/rewritten-to-dashboard`) + + // Check to see that the page we navigated to is in fact the dashboard. + expect(await browser.elementByCss('#from-dashboard').text()).toBe( + 'hello from app/dashboard' + ) + } finally { + await browser.close() + } + }) }) // TODO-APP: Enable in development @@ -454,28 +476,6 @@ describe('app dir', () => { } }) - it('should respect rewrites', async () => { - const browser = await webdriver(next.url, '/rewrites') - - try { - // Click the link. - await browser.elementById('link').click() - await browser.waitForElementByCss('#from-dashboard') - - // Check to see that we were rewritten and not redirected. - const pathname = await browser.eval('window.location.pathname') - expect(pathname).toBe('/rewritten-to-dashboard') - - // Check to see that the page we navigated to is in fact the dashboard. - const html = await browser.eval( - 'window.document.documentElement.innerText' - ) - expect(html).toContain('hello from app/dashboard') - } finally { - await browser.close() - } - }) - // TODO-APP: should enable when implemented it.skip('should allow linking from app page to pages page', async () => { const browser = await webdriver(next.url, '/pages-linking') From 6a3c812d567c1477214ebda4aee4649484fda1d4 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Thu, 15 Sep 2022 11:33:59 +0200 Subject: [PATCH 14/20] Bring back getter for flightSearchParameters --- packages/next/server/web/next-url.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/next/server/web/next-url.ts b/packages/next/server/web/next-url.ts index b751a5ae1be96d7..9bd9999d58c7935 100644 --- a/packages/next/server/web/next-url.ts +++ b/packages/next/server/web/next-url.ts @@ -163,6 +163,16 @@ export class NextURL { this[Internal].buildId = buildId } + public get flightSearchParameters() { + return this[Internal].flightSearchParameters + } + + public set flightSearchParameters( + flightData: Record | undefined + ) { + this[Internal].flightSearchParameters = flightData + } + public get locale() { return this[Internal].locale ?? '' } From 7a877d0ee66d0f9adeded61a80fc5b40b3909d65 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Thu, 15 Sep 2022 13:03:32 +0200 Subject: [PATCH 15/20] Ensure flightSearchParameters is not undefined --- packages/next/server/web/adapter.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/next/server/web/adapter.ts b/packages/next/server/web/adapter.ts index 7a90119623effa3..28a52ad568f86e6 100644 --- a/packages/next/server/web/adapter.ts +++ b/packages/next/server/web/adapter.ts @@ -57,7 +57,6 @@ export async function adapter(params: { // Preserve flight data. const flightSearchParameters = requestUrl.flightSearchParameters - requestUrl.flightSearchParameters = undefined // Strip internal query parameters off the request. stripInternalSearchParams(requestUrl.searchParams, true) From 3fd50e895bd58a4275846e3ccf1a52486e1bef2c Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Thu, 15 Sep 2022 13:11:11 +0200 Subject: [PATCH 16/20] Update TODO: to TODO-APP:. Skip failing test --- test/e2e/app-dir/index.test.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index 221750837f023ee..af619389934b003 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -105,7 +105,7 @@ describe('app dir', () => { expect($('p').text()).toBe('hello from app/dashboard/integrations') }) - // TODO: handle new root layout + // TODO-APP: handle new root layout it.skip('should not include parent when not in parent directory with route in directory', async () => { const html = await renderViaHTTP(next.url, '/dashboard/hello') const $ = cheerio.load(html) @@ -211,7 +211,7 @@ describe('app dir', () => { expect(await res.text()).toContain('This page could not be found') }) - // TODO: do we want to make this only work for /root or is it allowed + // TODO-APP: do we want to make this only work for /root or is it allowed // to work for /pages as well? it.skip('should match partial parameters', async () => { const html = await renderViaHTTP(next.url, '/partial-match-123') @@ -219,7 +219,8 @@ describe('app dir', () => { }) describe('rewrites', () => { - it('should support rewrites on initial load', async () => { + // TODO-APP: + it.skip('should support rewrites on initial load', async () => { const browser = await webdriver(next.url, '/rewritten-to-dashboard') expect(await browser.elementByCss('h1').text()).toBe('Dashboard') expect(await browser.url()).toBe(`${next.url}/rewritten-to-dashboard`) @@ -495,7 +496,7 @@ describe('app dir', () => { }) describe('server components', () => { - // TODO: why is this not servable but /dashboard+rootonly/hello.server.js + // TODO-APP: why is this not servable but /dashboard+rootonly/hello.server.js // should be? Seems like they both either should be servable or not it('should not serve .server.js as a path', async () => { // Without .server.js should serve @@ -614,7 +615,7 @@ describe('app dir', () => { ) }) - // TODO: investigate hydration not kicking in on some runs + // TODO-APP: investigate hydration not kicking in on some runs it.skip('should serve client-side', async () => { const browser = await webdriver(next.url, '/client-component-route') @@ -635,7 +636,7 @@ describe('app dir', () => { expect($('p').text()).toBe('hello from app/client-nested') }) - // TODO: investigate hydration not kicking in on some runs + // TODO-APP: investigate hydration not kicking in on some runs it.skip('should include it client-side', async () => { const browser = await webdriver(next.url, '/client-nested') @@ -663,7 +664,7 @@ describe('app dir', () => { const browser = await webdriver(next.url, '/slow-page-with-loading', { waitHydration: false, }) - // TODO: `await webdriver()` causes waiting for the full page to complete streaming. At that point "Loading..." is replaced by the actual content + // TODO-APP: `await webdriver()` causes waiting for the full page to complete streaming. At that point "Loading..." is replaced by the actual content // expect(await browser.elementByCss('#loading').text()).toBe('Loading...') expect(await browser.elementByCss('#slow-page-message').text()).toBe( @@ -689,7 +690,7 @@ describe('app dir', () => { waitHydration: false, } ) - // TODO: `await webdriver()` causes waiting for the full page to complete streaming. At that point "Loading..." is replaced by the actual content + // TODO-APP: `await webdriver()` causes waiting for the full page to complete streaming. At that point "Loading..." is replaced by the actual content // expect(await browser.elementByCss('#loading').text()).toBe('Loading...') expect( @@ -720,7 +721,7 @@ describe('app dir', () => { waitHydration: false, } ) - // TODO: `await webdriver()` causes waiting for the full page to complete streaming. At that point "Loading..." is replaced by the actual content + // TODO-APP: `await webdriver()` causes waiting for the full page to complete streaming. At that point "Loading..." is replaced by the actual content // expect(await browser.elementByCss('#loading-layout').text()).toBe('Loading...') // expect(await browser.elementByCss('#loading-page').text()).toBe('Loading...') From 7288eb60428d75ef7696b6b25e0d25d19b57c01c Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Thu, 15 Sep 2022 14:49:51 +0200 Subject: [PATCH 17/20] Bring back changes to setter --- packages/next/server/web/adapter.ts | 1 + packages/next/server/web/next-url.ts | 10 ++++++++++ test/e2e/app-dir/app/app/dashboard/page.server.js | 4 ---- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/next/server/web/adapter.ts b/packages/next/server/web/adapter.ts index 28a52ad568f86e6..7a90119623effa3 100644 --- a/packages/next/server/web/adapter.ts +++ b/packages/next/server/web/adapter.ts @@ -57,6 +57,7 @@ export async function adapter(params: { // Preserve flight data. const flightSearchParameters = requestUrl.flightSearchParameters + requestUrl.flightSearchParameters = undefined // Strip internal query parameters off the request. stripInternalSearchParams(requestUrl.searchParams, true) diff --git a/packages/next/server/web/next-url.ts b/packages/next/server/web/next-url.ts index 9bd9999d58c7935..be822cc27c24f03 100644 --- a/packages/next/server/web/next-url.ts +++ b/packages/next/server/web/next-url.ts @@ -170,6 +170,16 @@ export class NextURL { public set flightSearchParameters( flightData: Record | undefined ) { + if (flightData) { + for (const name of FLIGHT_PARAMETERS) { + this[Internal].url.searchParams.set(name, flightData[name] ?? '') + } + } else { + for (const name of FLIGHT_PARAMETERS) { + this[Internal].url.searchParams.delete(name) + } + } + this[Internal].flightSearchParameters = flightData } diff --git a/test/e2e/app-dir/app/app/dashboard/page.server.js b/test/e2e/app-dir/app/app/dashboard/page.server.js index a4773bfaba832c6..9c11228fd54a877 100644 --- a/test/e2e/app-dir/app/app/dashboard/page.server.js +++ b/test/e2e/app-dir/app/app/dashboard/page.server.js @@ -11,7 +11,3 @@ export default function DashboardPage(props) { ) } - -export const config = { - runtime: 'experimental-edge', -} From e0be34faaf6270ce315984a37b4b2643bf853865 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Thu, 15 Sep 2022 15:12:26 +0200 Subject: [PATCH 18/20] Fix edge-runtime ssr breaking --- packages/next/server/web/adapter.ts | 8 +++++++- test/e2e/app-dir/app/app/dashboard/page.server.js | 4 ++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/next/server/web/adapter.ts b/packages/next/server/web/adapter.ts index 7a90119623effa3..0a03f3f8936c5ff 100644 --- a/packages/next/server/web/adapter.ts +++ b/packages/next/server/web/adapter.ts @@ -40,6 +40,9 @@ export async function adapter(params: { page: string request: RequestData }): Promise { + // TODO-APP: use explicit marker for this + const isEdgeRendering = typeof self.__BUILD_MANIFEST !== 'undefined' + const requestUrl = new NextURL(params.request.url, { headers: params.request.headers, nextConfig: params.request.nextConfig, @@ -57,7 +60,10 @@ export async function adapter(params: { // Preserve flight data. const flightSearchParameters = requestUrl.flightSearchParameters - requestUrl.flightSearchParameters = undefined + // Parameters should only be stripped for middleware + if (!isEdgeRendering) { + requestUrl.flightSearchParameters = undefined + } // Strip internal query parameters off the request. stripInternalSearchParams(requestUrl.searchParams, true) diff --git a/test/e2e/app-dir/app/app/dashboard/page.server.js b/test/e2e/app-dir/app/app/dashboard/page.server.js index 9c11228fd54a877..a4773bfaba832c6 100644 --- a/test/e2e/app-dir/app/app/dashboard/page.server.js +++ b/test/e2e/app-dir/app/app/dashboard/page.server.js @@ -11,3 +11,7 @@ export default function DashboardPage(props) { ) } + +export const config = { + runtime: 'experimental-edge', +} From 941dd8b754a3c8b7a50388f7f746377027da41ac Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Thu, 15 Sep 2022 15:30:18 +0200 Subject: [PATCH 19/20] Use it.each --- test/e2e/app-dir/index.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index af619389934b003..83659296412d243 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -736,8 +736,9 @@ describe('app dir', () => { }) describe('middleware', () => { - ;['rewrite', 'redirect'].map((method) => - it(`should strip internal query parameters from requests to middleware for ${method}`, async () => { + it.each(['rewrite', 'redirect'])( + `should strip internal query parameters from requests to middleware for %s`, + async (method) => { const browser = await webdriver(next.url, '/internal') try { @@ -752,7 +753,7 @@ describe('app dir', () => { } finally { await browser.close() } - }) + } ) }) From e5724de11990fa388a54da9c917368f8cc0f44cb Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Thu, 15 Sep 2022 15:46:56 +0200 Subject: [PATCH 20/20] Fix additional flight parameter being applied accidentally --- packages/next/server/web/next-url.ts | 14 ++++++++++---- test/e2e/app-dir/index.test.ts | 10 +++++++--- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/packages/next/server/web/next-url.ts b/packages/next/server/web/next-url.ts index be822cc27c24f03..5f05ec8e8ddb854 100644 --- a/packages/next/server/web/next-url.ts +++ b/packages/next/server/web/next-url.ts @@ -168,11 +168,17 @@ export class NextURL { } public set flightSearchParameters( - flightData: Record | undefined + flightSearchParams: Record | undefined ) { - if (flightData) { + if (flightSearchParams) { for (const name of FLIGHT_PARAMETERS) { - this[Internal].url.searchParams.set(name, flightData[name] ?? '') + // Ensure only the provided values are set + if (flightSearchParams[name]) { + this[Internal].url.searchParams.set(name, flightSearchParams[name]) + } else { + // Delete the ones that are not provided as flightData should be overridden. + this[Internal].url.searchParams.delete(name) + } } } else { for (const name of FLIGHT_PARAMETERS) { @@ -180,7 +186,7 @@ export class NextURL { } } - this[Internal].flightSearchParameters = flightData + this[Internal].flightSearchParameters = flightSearchParams } public get locale() { diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index 83659296412d243..2da374ee05a14ae 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -747,9 +747,13 @@ describe('app dir', () => { // middleware sees any flight data on the request it'll redirect to // a page with an element of #failure, otherwise, we'll see the // element for #success. - await browser.waitForElementByCss(`#navigate-${method}`) - await browser.elementById(`navigate-${method}`).click() - await browser.waitForElementByCss('#success') + await browser + .waitForElementByCss(`#navigate-${method}`) + .elementById(`navigate-${method}`) + .click() + expect( + await browser.waitForElementByCss('#success', 3000).text() + ).toBe('Success') } finally { await browser.close() }