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

Change flight querystring to header #40752

Merged
merged 3 commits into from Sep 21, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
34 changes: 18 additions & 16 deletions packages/next/client/components/app-router.client.tsx
Expand Up @@ -29,11 +29,9 @@ import {
} from './hooks-client-context'
import { useReducerWithReduxDevtools } from './use-reducer-with-devtools'

function urlToUrlWithoutFlightParameters(url: string): URL {
function urlToUrlWithoutFlightMarker(url: string): URL {
const urlWithoutFlightParameters = new URL(url, location.origin)
urlWithoutFlightParameters.searchParams.delete('__flight__')
urlWithoutFlightParameters.searchParams.delete('__flight_router_state_tree__')
urlWithoutFlightParameters.searchParams.delete('__flight_prefetch__')
// TODO-APP: handle .rsc for static export case
return urlWithoutFlightParameters
}

Expand All @@ -45,22 +43,26 @@ export async function fetchServerResponse(
flightRouterState: FlightRouterState,
prefetch?: true
): Promise<[FlightData: FlightData, canonicalUrlOverride: URL | undefined]> {
const flightUrl = new URL(url)
const searchParams = flightUrl.searchParams
// Enable flight response
searchParams.append('__flight__', '1')
// Provide the current router state
searchParams.append(
'__flight_router_state_tree__',
JSON.stringify(flightRouterState)
)
const headers: {
__flight__: '1'
__flight_router_state_tree__: string
__flight_prefetch__?: '1'
} = {
// Enable flight response
__flight__: '1',
// Provide the current router state
__flight_router_state_tree__: JSON.stringify(flightRouterState),
}
if (prefetch) {
searchParams.append('__flight_prefetch__', '1')
// Enable prefetch response
headers.__flight_prefetch__ = '1'
}

const res = await fetch(flightUrl.toString())
const res = await fetch(url.toString(), {
headers,
})
const canonicalUrl = res.redirected
? urlToUrlWithoutFlightParameters(res.url)
? urlToUrlWithoutFlightMarker(res.url)
: undefined

// Handle the `fetch` readable stream that can be unwrapped by `React.use`.
Expand Down
24 changes: 19 additions & 5 deletions packages/next/server/app-render.tsx
Expand Up @@ -493,6 +493,20 @@ function getScriptNonceFromHeader(cspHeaderValue: string): string | undefined {
return nonce
}

const FLIGHT_PARAMETERS = [
'__flight__',
'__flight_router_state_tree__',
'__flight_prefetch__',
] as const

function headersWithoutFlight(headers: { [key: string]: string }) {
const newHeaders = { ...headers }
for (const param of FLIGHT_PARAMETERS) {
delete newHeaders[param]
}
return newHeaders
}

export async function renderToHTMLOrFlight(
req: IncomingMessage,
res: ServerResponse,
Expand Down Expand Up @@ -545,8 +559,8 @@ export async function renderToHTMLOrFlight(
ComponentMod,
} = renderOpts

const isFlight = query.__flight__ !== undefined
const isPrefetch = query.__flight_prefetch__ !== undefined
const isFlight = req.headers.__flight__ !== undefined
const isPrefetch = req.headers.__flight_prefetch__ !== undefined

// Handle client-side navigation to pages directory
if (isFlight && isPagesDir) {
Expand All @@ -569,8 +583,8 @@ export async function renderToHTMLOrFlight(
* Router state provided from the client-side router. Used to handle rendering from the common layout down.
*/
const providedFlightRouterState: FlightRouterState = isFlight
? query.__flight_router_state_tree__
? JSON.parse(query.__flight_router_state_tree__ as string)
? req.headers.__flight_router_state_tree__
? JSON.parse(req.headers.__flight_router_state_tree__ as string)
: {}
: undefined

Expand All @@ -584,7 +598,7 @@ export async function renderToHTMLOrFlight(
| typeof import('../client/components/hot-reloader.client').default
| null

const headers = req.headers
const headers = headersWithoutFlight(req.headers)
// TODO-APP: fix type of req
// @ts-expect-error
const cookies = req.cookies
Expand Down
2 changes: 1 addition & 1 deletion packages/next/server/base-server.ts
Expand Up @@ -1032,7 +1032,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {

// Don't delete query.__flight__ yet, it still needs to be used in renderToHTML later
const isFlightRequest = Boolean(
this.serverComponentManifest && query.__flight__
this.serverComponentManifest && req.headers.__flight__
)

// we need to ensure the status code if /404 is visited directly
Expand Down
3 changes: 1 addition & 2 deletions packages/next/server/next-server.ts
Expand Up @@ -825,7 +825,7 @@ export default class NextNodeServer extends BaseServer {

if (
this.nextConfig.experimental.appDir &&
(renderOpts.isAppPath || query.__flight__)
(renderOpts.isAppPath || req.headers.__flight__)
) {
const isPagesDir = !renderOpts.isAppPath
return appRenderToHTMLOrFlight(
Expand Down Expand Up @@ -981,7 +981,6 @@ export default class NextNodeServer extends BaseServer {
__nextDataReq: query.__nextDataReq,
__nextLocale: query.__nextLocale,
__nextDefaultLocale: query.__nextDefaultLocale,
__flight__: query.__flight__,
} as NextParsedUrlQuery)
: query),
// For appDir params is excluded.
Expand Down
19 changes: 11 additions & 8 deletions packages/next/server/web/adapter.ts
Expand Up @@ -35,6 +35,12 @@ class NextRequestHint extends NextRequest {
}
}

const FLIGHT_PARAMETERS = [
'__flight__',
'__flight_router_state_tree__',
'__flight_prefetch__',
] as const

export async function adapter(params: {
handler: NextMiddleware
page: string
Expand All @@ -58,11 +64,12 @@ export async function adapter(params: {
requestUrl.pathname = '/'
}

// Preserve flight data.
const flightSearchParameters = requestUrl.flightSearchParameters
const requestHeaders = fromNodeHeaders(params.request.headers)
// Parameters should only be stripped for middleware
if (!isEdgeRendering) {
requestUrl.flightSearchParameters = undefined
for (const param of FLIGHT_PARAMETERS) {
requestHeaders.delete(param)
}
}

// Strip internal query parameters off the request.
Expand All @@ -74,7 +81,7 @@ export async function adapter(params: {
init: {
body: params.request.body,
geo: params.request.geo,
headers: fromNodeHeaders(params.request.headers),
headers: requestHeaders,
ip: params.request.ip,
method: params.request.method,
nextConfig: params.request.nextConfig,
Expand Down Expand Up @@ -112,8 +119,6 @@ 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))
}

Expand Down Expand Up @@ -151,8 +156,6 @@ 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))
}

Expand Down
74 changes: 1 addition & 73 deletions packages/next/server/web/next-url.ts
Expand Up @@ -15,12 +15,6 @@ 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)/

Expand All @@ -31,28 +25,6 @@ function parseURL(url: string | URL, base?: string | URL) {
)
}

function parseFlightParameters(
searchParams: URLSearchParams
): Record<string, string> | undefined {
let flightSearchParameters: Record<string, string> = {}
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 {
Expand Down Expand Up @@ -118,9 +90,6 @@ 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() {
Expand All @@ -137,22 +106,7 @@ 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}`
return this[Internal].url.search
}

public get buildId() {
Expand All @@ -163,32 +117,6 @@ export class NextURL {
this[Internal].buildId = buildId
}

public get flightSearchParameters() {
return this[Internal].flightSearchParameters
}

public set flightSearchParameters(
flightSearchParams: Record<string, string> | 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 ?? ''
}
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/app-dir/app/middleware.js
Expand Up @@ -20,7 +20,7 @@ export function middleware(request) {
: 'redirect'

const internal = ['__flight__', '__flight_router_state_tree__']
if (internal.some((name) => request.nextUrl.searchParams.has(name))) {
if (internal.some((name) => request.headers.has(name))) {
return NextResponse[method](new URL('/internal/failure', request.url))
}

Expand Down
19 changes: 17 additions & 2 deletions test/e2e/app-dir/index.test.ts
Expand Up @@ -38,13 +38,28 @@ describe('app dir', () => {
it('should use application/octet-stream for flight', async () => {
const res = await fetchViaHTTP(
next.url,
'/dashboard/deployments/123?__flight__'
'/dashboard/deployments/123',
{},
{
headers: {
__flight__: '1',
},
}
)
expect(res.headers.get('Content-Type')).toBe('application/octet-stream')
})

it('should use application/octet-stream for flight with edge runtime', async () => {
const res = await fetchViaHTTP(next.url, '/dashboard?__flight__')
const res = await fetchViaHTTP(
next.url,
'/dashboard',
{},
{
headers: {
__flight__: '1',
},
}
)
expect(res.headers.get('Content-Type')).toBe('application/octet-stream')
})

Expand Down
27 changes: 18 additions & 9 deletions test/e2e/app-dir/rsc-basic.test.ts
Expand Up @@ -94,7 +94,8 @@ describe('app dir - react server components', () => {
'__nextDefaultLocale',
'__nextIsNotFound',
'__flight__',
'__flight_router_path__',
'__flight_router_state_tree__',
'__flight_prefetch__',
]

const hasNextInternalQuery = inlineFlightContents.some((content) =>
Expand All @@ -110,11 +111,11 @@ describe('app dir - react server components', () => {
beforePageLoad(page) {
page.on('request', (request) => {
requestsCount++
const url = request.url()
const headers = request.allHeaders()
if (
url.includes('__flight__=1') &&
headers.__flight__ === '1' &&
// Prefetches also include `__flight__`
!url.includes('__flight_prefetch__=1')
headers.__flight_prefetch__ !== '1'
) {
hasFlightRequest = true
}
Expand Down Expand Up @@ -215,11 +216,10 @@ describe('app dir - react server components', () => {
const browser = await webdriver(next.url, '/root', {
beforePageLoad(page) {
page.on('request', (request) => {
const url = request.url()
const headers = request.allHeaders()
if (
url.includes('__flight__=1') &&
// Prefetches also include `__flight__`
!url.includes('__flight_prefetch__=1')
headers.__flight__ === '1' &&
headers.__flight_prefetch__ !== '1'
) {
hasFlightRequest = true
}
Expand Down Expand Up @@ -336,7 +336,16 @@ describe('app dir - react server components', () => {
})

it('should support streaming for flight response', async () => {
await fetchViaHTTP(next.url, '/?__flight__=1').then(async (response) => {
await fetchViaHTTP(
next.url,
'/',
{},
{
headers: {
__flight__: '1',
},
}
).then(async (response) => {
const result = await resolveStreamResponse(response)
expect(result).toContain('component:index.server')
})
Expand Down