refactor: Extract early config header and public file pipeline#983
Conversation
commit: |
|
/bigbonk review |
There was a problem hiding this comment.
Clean refactor that successfully extracts duplicated config-header and public-file logic from generated entries and server hot paths into testable helpers in request-pipeline.ts. All CI green.
What I verified:
applyConfigHeadersToResponsefaithfully reproduces the old inline App Router entry behavior:appendfor vary/set-cookie,setonly when!has().applyConfigHeadersToHeaderRecordfaithfully reproduces the old inline Pages Router prod-server and Worker entry behavior, with one inconsequential difference: for set-cookie with no pre-existing value, the old code stored[value](always array) while the newappendHeaderRecordstores a plainvalue(string). This is safe because all downstream consumers (toWebHeaders,mergeHeaders,sendCompressed) handle bothstringandstring[]correctly.findHeaderRecordKeyadds case-insensitive key lookup, which is a minor improvement over the oldlk in middlewareHeaders(which happened to work because both sides were lowercased).resolvePublicFileRouteexactly reproduces the old inline checks: method guard (GET/HEAD),.rscexclusion onpathname, public-file lookup oncleanPathname, andcreateStaticFileSignalmatches the old__createStaticFileSignal.- The new
./server/request-pipelinepackage.json export is correctly placed for the generated worker entry to consume. - Tests cover the important contracts: original-request matcher context, middleware precedence, multi-value header appends, and public-file routing policy.
- Snapshot updates are mechanical and consistent across all fixture variants.
One small observation below, but nothing blocking.
| function appendHeaderRecord(headers: HeaderRecord, lowerName: string, value: string): void { | ||
| const key = findHeaderRecordKey(headers, lowerName) ?? lowerName; | ||
| const existing = headers[key]; | ||
| if (existing === undefined) { |
There was a problem hiding this comment.
Minor observation: the old inline set-cookie code in prod-server.ts and deploy.ts always stored a fresh set-cookie as an array (middlewareHeaders[lk] = [h.value]), whereas appendHeaderRecord stores it as a plain string when there's no existing value. This is safe today because all downstream consumers (toWebHeaders, mergeHeaders) handle both string and string[], but worth noting in case a future consumer assumes set-cookie values are always arrays.
|
Review submitted. Approved with one minor observation. Summary: This is a well-executed refactor that extracts duplicated config-header application and public-file routing logic from 3 places (generated App Router RSC entry, Pages Router prod-server, and generated Worker entry) into shared, tested helpers in |
Summary
next.config.jsheader application intorequest-pipelinehelpers for both WebHeadersand early header records.request-pipeline, leaving generated RSC entry code to describe app shape and route manifests.vinext/server/request-pipelinefor generated worker code.Next.js references
Vinext should keep the route policy aligned with Next.js while keeping behavior out of generated templates:
headers,redirects, proxy/middleware,beforeFiles, then filesystem routes includingpublic/: https://github.com/vercel/next.js/blob/canary/docs/01-app/03-api-reference/03-file-conventions/proxy.mdxresolve-routes.tsmodels that same order withfsChecker.headers,fsChecker.redirects, middleware,rewrites.beforeFiles, and filesystem checks: https://github.com/vercel/next.js/blob/canary/packages/next/src/server/lib/router-utils/resolve-routes.ts/publicfiles: https://github.com/vercel/next.js/blob/canary/docs/01-app/03-api-reference/05-config/01-next-config-js/redirects.mdxfilesystem.ts: https://github.com/vercel/next.js/blob/canary/packages/next/src/server/lib/router-utils/filesystem.tsTests
vp test run tests/request-pipeline.test.tsvp test run tests/app-router.test.ts -t "custom header handling|empty config arrays|rewrite handling code|config pattern"vp test run tests/deploy.test.ts -t "next.config.js custom headers|next.config.js redirects|next.config.js rewrites|basePath stripping"vp test run tests/request-pipeline.test.ts tests/deploy.test.tsvp test run tests/app-router.test.ts -t "custom headers from next.config.js|serves public files from the build output|serves public files under basePath|App Router next.config.js features"vp test run tests/entry-templates.test.tsvp fmt --check packages/vinext/src/server/request-pipeline.ts packages/vinext/src/entries/app-rsc-entry.ts packages/vinext/src/server/prod-server.ts packages/vinext/src/deploy.ts tests/request-pipeline.test.ts tests/app-router.test.ts tests/deploy.test.ts packages/vinext/package.jsonvp checkvp run vinext#build