Skip to content

Commit

Permalink
conditionally send Next-URL in Vary response
Browse files Browse the repository at this point in the history
  • Loading branch information
ztanner committed Feb 8, 2024
1 parent b770e4e commit 0f77d79
Show file tree
Hide file tree
Showing 10 changed files with 95 additions and 24 deletions.
8 changes: 5 additions & 3 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ import {
NEXT_ROUTER_PREFETCH_HEADER,
RSC_HEADER,
RSC_CONTENT_TYPE_HEADER,
RSC_VARY_HEADER,
NEXT_ROUTER_STATE_TREE,
NEXT_DID_POSTPONE_HEADER,
} from '../client/components/app-router-headers'
import { webpackBuild } from './webpack-build'
Expand Down Expand Up @@ -261,7 +261,7 @@ export type RoutesManifest = {
rsc: {
header: typeof RSC_HEADER
didPostponeHeader: typeof NEXT_DID_POSTPONE_HEADER
varyHeader: typeof RSC_VARY_HEADER
varyHeader: string
prefetchHeader: typeof NEXT_ROUTER_PREFETCH_HEADER
suffix: typeof RSC_SUFFIX
prefetchSuffix: typeof RSC_PREFETCH_SUFFIX
Expand Down Expand Up @@ -1100,7 +1100,9 @@ export default async function build(
i18n: config.i18n || undefined,
rsc: {
header: RSC_HEADER,
varyHeader: RSC_VARY_HEADER,
// This vary header is used as a default. It is technically re-assigned in `base-server`,
// and may include an additional Vary option for `Next-URL`.
varyHeader: `${RSC_HEADER}, ${NEXT_ROUTER_STATE_TREE}, ${NEXT_ROUTER_PREFETCH_HEADER}`,
prefetchHeader: NEXT_ROUTER_PREFETCH_HEADER,
didPostponeHeader: NEXT_DID_POSTPONE_HEADER,
contentTypeHeader: RSC_CONTENT_TYPE_HEADER,
Expand Down
2 changes: 0 additions & 2 deletions packages/next/src/client/components/app-router-headers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ export const NEXT_ROUTER_STATE_TREE = 'Next-Router-State-Tree' as const
export const NEXT_ROUTER_PREFETCH_HEADER = 'Next-Router-Prefetch' as const
export const NEXT_URL = 'Next-Url' as const
export const RSC_CONTENT_TYPE_HEADER = 'text/x-component' as const
export const RSC_VARY_HEADER =
`${RSC_HEADER}, ${NEXT_ROUTER_STATE_TREE}, ${NEXT_ROUTER_PREFETCH_HEADER}, ${NEXT_URL}` as const

export const FLIGHT_PARAMETERS = [
[RSC_HEADER],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
isInterceptionRouteAppPath,
} from '../server/future/helpers/interception-routes'
import type { Rewrite } from './load-custom-routes'
import type { ManifestRewriteRoute } from '../build'

// a function that converts normalised paths (e.g. /foo/[bar]/[baz]) to the format expected by pathToRegexp (e.g. /foo/:bar/:baz)
function toPathToRegexpPath(path: string): string {
Expand Down Expand Up @@ -86,3 +87,8 @@ export function generateInterceptionRoutesRewrites(

return rewrites
}

export function isInterceptionRouteRewrite(route: ManifestRewriteRoute) {
// When we generate interception rewrites in the above implementation, we always do so with only a single `has` condition.
return route.has?.[0].key === NEXT_URL
}
57 changes: 48 additions & 9 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,11 @@ import { parseUrl as parseUrlUtil } from '../shared/lib/router/utils/parse-url'
import { getNextPathnameInfo } from '../shared/lib/router/utils/get-next-pathname-info'
import {
RSC_HEADER,
RSC_VARY_HEADER,
NEXT_RSC_UNION_QUERY,
NEXT_ROUTER_PREFETCH_HEADER,
NEXT_DID_POSTPONE_HEADER,
NEXT_URL,
NEXT_ROUTER_STATE_TREE,
} from '../client/components/app-router-headers'
import type {
MatchOptions,
Expand Down Expand Up @@ -129,6 +130,7 @@ import {
import { PrefetchRSCPathnameNormalizer } from './future/normalizers/request/prefetch-rsc'
import { NextDataPathnameNormalizer } from './future/normalizers/request/next-data'
import { getIsServerAction } from './lib/server-action-request-meta'
import { isInterceptionRouteAppPath } from './future/helpers/interception-routes'

export type FindComponentsResult = {
components: LoadComponentsReturnType
Expand Down Expand Up @@ -321,6 +323,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
protected readonly serverOptions: Readonly<ServerOptions>
protected readonly appPathRoutes?: Record<string, string[]>
protected readonly clientReferenceManifest?: ClientReferenceManifest
protected interceptionRouteRewrites: ManifestRewriteRoute[]
protected nextFontManifest?: NextFontManifest
private readonly responseCache: ResponseCacheBase

Expand All @@ -329,6 +332,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
protected abstract getPagesManifest(): PagesManifest | undefined
protected abstract getAppPathsManifest(): PagesManifest | undefined
protected abstract getBuildId(): string
protected abstract getInterceptionRouteRewrites(): ManifestRewriteRoute[]

protected readonly enabledDirectories: NextEnabledDirectories
protected abstract getEnabledDirectories(dev: boolean): NextEnabledDirectories
Expand Down Expand Up @@ -562,6 +566,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
this.pagesManifest = this.getPagesManifest()
this.appPathsManifest = this.getAppPathsManifest()
this.appPathRoutes = this.getAppPathRoutes()
this.interceptionRouteRewrites = this.getInterceptionRouteRewrites()

// Configure the routes.
this.matchers = this.getRouteMatchers()
Expand Down Expand Up @@ -1741,6 +1746,45 @@ export default abstract class Server<ServerOptions extends Options = Options> {
}
}

protected pathCouldBeIntercepted(resolvedPathname: string): boolean {
return (
isInterceptionRouteAppPath(resolvedPathname) ||
this.interceptionRouteRewrites?.some((rewrite) => {
return new RegExp(rewrite.regex).test(resolvedPathname)
})
)
}

protected setVaryHeader(
req: BaseNextRequest,
res: BaseNextResponse,
isAppPath: boolean,
resolvedPathname: string
): void {
const baseVaryHeader = `${RSC_HEADER}, ${NEXT_ROUTER_STATE_TREE}, ${NEXT_ROUTER_PREFETCH_HEADER}`
const isRSCRequest =
Boolean(req.headers[RSC_HEADER.toLowerCase()]) ||
getRequestMeta(req, 'isRSCRequest')
let addedNextUrlToVary = false

if (isAppPath && this.pathCouldBeIntercepted(resolvedPathname)) {
// Interception route responses can vary based on the `Next-URL` header.
// We use the Vary header to signal this behavior to the client to properly cache the response.
res.setHeader('vary', `${baseVaryHeader}, ${NEXT_URL}`)
addedNextUrlToVary = true
} else if (isAppPath || isRSCRequest) {
// We don't need to include `Next-URL` in the Vary header for non-interception routes since it won't affect the response.
// We also set this header for pages to avoid caching issues when navigating between pages and app.
res.setHeader('vary', baseVaryHeader)
}

if (!addedNextUrlToVary) {
// Remove `Next-URL` from the request headers we determined it wasn't necessary to include in the Vary header.
// This is to avoid any dependency on the `Next-URL` header being present when preparing the response.
delete req.headers[NEXT_URL]
}
}

private async renderToResponseWithComponentsImpl(
{ req, res, pathname, renderOpts: opts }: RequestContext,
{ components, query }: FindComponentsResult
Expand All @@ -1755,6 +1799,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {

const is500Page = pathname === '/500'
const isAppPath = components.isAppPath === true

const hasServerProps = !!components.getServerSideProps
let hasStaticPaths = !!components.getStaticPaths
const isServerAction = getIsServerAction(req)
Expand All @@ -1768,6 +1813,8 @@ export default abstract class Server<ServerOptions extends Options = Options> {

let resolvedUrlPathname = getRequestMeta(req, 'rewroteURL') || urlPathname

this.setVaryHeader(req, res, isAppPath, resolvedUrlPathname)

let staticPaths: string[] | undefined

let fallbackMode: FallbackMode
Expand Down Expand Up @@ -1897,12 +1944,6 @@ export default abstract class Server<ServerOptions extends Options = Options> {
const isDynamicRSCRequest =
opts.experimental.ppr && isRSCRequest && !isPrefetchRSCRequest

// For pages we need to ensure the correct Vary header is set too, to avoid
// caching issues when navigating between pages and app
if (!isAppPath && isRSCRequest) {
res.setHeader('vary', RSC_VARY_HEADER)
}

// we need to ensure the status code if /404 is visited directly
if (is404Page && !isDataReq && !isRSCRequest) {
res.statusCode = 404
Expand Down Expand Up @@ -1993,8 +2034,6 @@ export default abstract class Server<ServerOptions extends Options = Options> {
}

if (isAppPath) {
res.setHeader('vary', RSC_VARY_HEADER)

if (!this.renderOpts.dev && !isPreviewMode && isSSG && isRSCRequest) {
// If this is an RSC request but we aren't in minimal mode, then we mark
// that this is a data request so that we can generate the flight data
Expand Down
15 changes: 15 additions & 0 deletions packages/next/src/server/dev/next-dev-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import type { UnwrapPromise } from '../../lib/coalesced-function'
import type { NodeNextResponse, NodeNextRequest } from '../base-http/node'
import type { RouteEnsurer } from '../future/route-matcher-managers/dev-route-matcher-manager'
import type { PagesManifest } from '../../build/webpack/plugins/pages-manifest-plugin'
import type { ManifestRewriteRoute } from '../../build'

import fs from 'fs'
import { Worker } from 'next/dist/compiled/jest-worker'
Expand Down Expand Up @@ -62,6 +63,8 @@ import LRUCache from 'next/dist/compiled/lru-cache'
import { getMiddlewareRouteMatcher } from '../../shared/lib/router/utils/middleware-route-matcher'
import { DetachedPromise } from '../../lib/detached-promise'
import { isPostpone } from '../lib/router-utils/is-postpone'
import { generateInterceptionRoutesRewrites } from '../../lib/generate-interception-routes-rewrites'
import { buildCustomRoute } from '../../lib/build-custom-route'

// Load ReactDevOverlay only when needed
let ReactDevOverlayImpl: FunctionComponent
Expand Down Expand Up @@ -287,6 +290,9 @@ export default class DevServer extends Server {
this.ready?.resolve()
this.ready = undefined

// In dev, this needs to be called after prepare because the build entries won't be known in the constructor
this.interceptionRouteRewrites = this.getInterceptionRouteRewrites()

// This is required by the tracing subsystem.
setGlobal('appDir', this.appDir)
setGlobal('pagesDir', this.pagesDir)
Expand Down Expand Up @@ -543,6 +549,15 @@ export default class DevServer extends Server {
)
}

protected getInterceptionRouteRewrites(): ManifestRewriteRoute[] {
const rewrites = generateInterceptionRoutesRewrites(
Object.keys(this.appPathRoutes ?? {}),
this.nextConfig.basePath
).map((route) => buildCustomRoute('rewrite', route))

return rewrites ?? []
}

protected getMiddleware() {
// We need to populate the match
// field as it isn't serializable
Expand Down
11 changes: 10 additions & 1 deletion packages/next/src/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
import type { MiddlewareManifest } from '../build/webpack/plugins/middleware-plugin'
import type RenderResult from './render-result'
import type { FetchEventResult } from './web/types'
import type { PrerenderManifest } from '../build'
import type { ManifestRewriteRoute, PrerenderManifest } from '../build'
import type { BaseNextRequest, BaseNextResponse } from './base-http'
import type { PagesManifest } from '../build/webpack/plugins/pages-manifest-plugin'
import type { NextParsedUrlQuery, NextUrlWithParsedQuery } from './request-meta'
Expand Down Expand Up @@ -101,6 +101,7 @@ import { lazyRenderPagesPage } from './future/route-modules/pages/module.render'
import { interopDefault } from '../lib/interop-default'
import { formatDynamicImportPath } from '../lib/format-dynamic-import-path'
import type { NextFontManifest } from '../build/webpack/plugins/next-font-manifest-plugin'
import { isInterceptionRouteRewrite } from '../lib/generate-interception-routes-rewrites'

export * from './base-server'

Expand Down Expand Up @@ -371,6 +372,14 @@ export default class NextNodeServer extends BaseServer {
) as PagesManifest
}

protected getInterceptionRouteRewrites(): ManifestRewriteRoute[] {
const routesManifest = this.getRoutesManifest()
return (
routesManifest?.rewrites.beforeFiles.filter(isInterceptionRouteRewrite) ??
[]
)
}

protected async hasPage(pathname: string): Promise<boolean> {
return !!getMaybePagePath(
pathname,
Expand Down
8 changes: 6 additions & 2 deletions packages/next/src/server/web-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type RenderResult from './render-result'
import type { NextParsedUrlQuery, NextUrlWithParsedQuery } from './request-meta'
import type { Params } from '../shared/lib/router/utils/route-matcher'
import type { LoadComponentsReturnType } from './load-components'
import type { PrerenderManifest } from '../build'
import type { ManifestRewriteRoute, PrerenderManifest } from '../build'
import type {
LoadedRenderOpts,
MiddlewareRoutingItem,
Expand Down Expand Up @@ -374,7 +374,6 @@ export default class NextWebServer extends BaseServer<WebServerOptions> {
// The web server does not need to handle fallback errors in production.
return null
}

protected getRoutesManifest(): NormalizedRouteManifest | undefined {
// The web server does not need to handle rewrite rules. This is done by the
// upstream proxy (edge runtime or node server).
Expand All @@ -394,4 +393,9 @@ export default class NextWebServer extends BaseServer<WebServerOptions> {
protected async getPrefetchRsc(): Promise<string | null> {
return null
}

protected getInterceptionRouteRewrites(): ManifestRewriteRoute[] {
// TODO: This needs to be implemented.
return []
}
}
6 changes: 3 additions & 3 deletions test/e2e/app-dir/app/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ createNextDescribe(
const res = await next.fetch('/dashboard')
expect(res.headers.get('x-edge-runtime')).toBe('1')
expect(res.headers.get('vary')).toBe(
'RSC, Next-Router-State-Tree, Next-Router-Prefetch, Next-Url'
'RSC, Next-Router-State-Tree, Next-Router-Prefetch'
)
})

Expand All @@ -304,8 +304,8 @@ createNextDescribe(
})
expect(res.headers.get('vary')).toBe(
isNextDeploy
? 'RSC, Next-Router-State-Tree, Next-Router-Prefetch, Next-Url'
: 'RSC, Next-Router-State-Tree, Next-Router-Prefetch, Next-Url, Accept-Encoding'
? 'RSC, Next-Router-State-Tree, Next-Router-Prefetch'
: 'RSC, Next-Router-State-Tree, Next-Router-Prefetch, Accept-Encoding'
)
})

Expand Down
3 changes: 1 addition & 2 deletions test/integration/custom-routes/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2558,8 +2558,7 @@ const runTests = (isDev = false) => {
header: 'RSC',
contentTypeHeader: 'text/x-component',
didPostponeHeader: 'x-nextjs-postponed',
varyHeader:
'RSC, Next-Router-State-Tree, Next-Router-Prefetch, Next-Url',
varyHeader: 'RSC, Next-Router-State-Tree, Next-Router-Prefetch',
prefetchHeader: 'Next-Router-Prefetch',
prefetchSuffix: '.prefetch.rsc',
suffix: '.rsc',
Expand Down
3 changes: 1 addition & 2 deletions test/integration/dynamic-routing/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1513,8 +1513,7 @@ function runTests({ dev }) {
header: 'RSC',
contentTypeHeader: 'text/x-component',
didPostponeHeader: 'x-nextjs-postponed',
varyHeader:
'RSC, Next-Router-State-Tree, Next-Router-Prefetch, Next-Url',
varyHeader: 'RSC, Next-Router-State-Tree, Next-Router-Prefetch',
prefetchHeader: 'Next-Router-Prefetch',
prefetchSuffix: '.prefetch.rsc',
suffix: '.rsc',
Expand Down

0 comments on commit 0f77d79

Please sign in to comment.