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 = {}) => { diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index c24206ba2420561..a689c072523504b 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 { ESCAPE_REGEX, 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' import type { ComponentsType } from '../build/webpack/loaders/next-app-loader' // this needs to be required lazily so that `next-server` can set diff --git a/packages/next/server/internal-utils.ts b/packages/next/server/internal-utils.ts new file mode 100644 index 000000000000000..1dbfef90cd9231f --- /dev/null +++ b/packages/next/server/internal-utils.ts @@ -0,0 +1,38 @@ +import type { NextParsedUrlQuery } from './request-meta' + +const INTERNAL_QUERY_NAMES = [ + '__nextFallback', + '__nextLocale', + '__nextDefaultLocale', + '__nextIsNotFound', + // RSC + '__flight__', + // Routing + '__flight_router_state_tree__', + '__flight_prefetch__', +] 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, + 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/render.tsx b/packages/next/server/render.tsx index 2ec63d476a1b39b..613c8ea8ffdda65 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 9e4ca33e9abb246..eb75aa6af1ebef4 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' @@ -23,18 +22,5 @@ export function isTargetLikeServerless(target: string) { return isServerless || isServerlessTrace } -export function stripInternalQueries(query: NextParsedUrlQuery) { - delete query.__nextFallback - delete query.__nextLocale - delete query.__nextDefaultLocale - delete query.__nextIsNotFound - - // RSC - delete query.__flight__ - delete query.__props__ - // routing - delete query.__flight_router_state_tree__ -} - // 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 9c40ca31c056236..0a03f3f8936c5ff 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 '../internal-utils' class NextRequestHint extends NextRequest { sourcePage: string @@ -39,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, @@ -54,13 +58,16 @@ 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 flightSearchParameters = requestUrl.flightSearchParameters + // Parameters should only be stripped for middleware + if (!isEdgeRendering) { + requestUrl.flightSearchParameters = undefined } + // Strip internal query parameters off the request. + stripInternalSearchParams(requestUrl.searchParams, true) + const request = new NextRequestHint({ page: params.page, input: String(requestUrl), @@ -105,6 +112,8 @@ export async function adapter(params: { if (rewriteUrl.host === request.nextUrl.host) { rewriteUrl.buildId = buildId || rewriteUrl.buildId + rewriteUrl.flightSearchParameters = + flightSearchParameters || rewriteUrl.flightSearchParameters response.headers.set('x-middleware-rewrite', String(rewriteUrl)) } @@ -142,6 +151,8 @@ export async function adapter(params: { if (redirectURL.host === request.nextUrl.host) { redirectURL.buildId = buildId || redirectURL.buildId + 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 118c2820107cda1..5f05ec8e8ddb854 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__', + '__flight_router_state_tree__', + '__flight_prefetch__', +] as const + const REGEX_LOCALHOST_HOSTNAME = /(?!^https?:\/\/)(127(?:\.(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)){3}|::1|localhost)/ @@ -25,12 +31,35 @@ function parseURL(url: string | URL, base?: string | URL) { ) } +function parseFlightParameters( + searchParams: URLSearchParams +): Record | undefined { + let flightSearchParameters: Record = {} + let flightSearchParametersUpdated = false + for (const name of FLIGHT_PARAMETERS) { + const value = searchParams.get(name) + if (value === null) { + continue + } + + flightSearchParameters[name] = value + flightSearchParametersUpdated = true + } + + if (!flightSearchParametersUpdated) { + return undefined + } + + return flightSearchParameters +} + const Internal = Symbol('NextURLInternal') export class NextURL { [Internal]: { basePath: string buildId?: string + flightSearchParameters?: Record defaultLocale?: string domainLocale?: DomainLocale locale?: string @@ -89,6 +118,9 @@ export class NextURL { this[Internal].buildId = pathnameInfo.buildId this[Internal].locale = pathnameInfo.locale ?? defaultLocale this[Internal].trailingSlash = pathnameInfo.trailingSlash + this[Internal].flightSearchParameters = parseFlightParameters( + this[Internal].url.searchParams + ) } private formatPathname() { @@ -104,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 } @@ -112,6 +163,32 @@ export class NextURL { this[Internal].buildId = buildId } + public get flightSearchParameters() { + return this[Internal].flightSearchParameters + } + + public set flightSearchParameters( + flightSearchParams: Record | undefined + ) { + if (flightSearchParams) { + for (const name of FLIGHT_PARAMETERS) { + // 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) { + this[Internal].url.searchParams.delete(name) + } + } + + this[Internal].flightSearchParameters = flightSearchParams + } + public get locale() { return this[Internal].locale ?? '' } @@ -175,7 +252,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) { 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 ( +
+ + +
+ ) +} 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..84563968b03f98b 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__', '__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 00ece1b3d71bf82..2da374ee05a14ae 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,16 +211,40 @@ 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') expect(html).toContain('hello from app/partial-match-[id]. ID is: 123') }) - it('should support rewrites', async () => { - const html = await renderViaHTTP(next.url, '/rewritten-to-dashboard') - expect(html).toContain('hello from app/dashboard') + describe('rewrites', () => { + // 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`) + }) + + 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 @@ -453,28 +477,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') @@ -494,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 @@ -613,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') @@ -634,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') @@ -662,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( @@ -688,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( @@ -719,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...') @@ -733,6 +735,32 @@ describe('app dir', () => { }) }) + describe('middleware', () => { + 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 { + // 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 + .waitForElementByCss(`#navigate-${method}`) + .elementById(`navigate-${method}`) + .click() + expect( + await browser.waitForElementByCss('#success', 3000).text() + ).toBe('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') 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__', ]