feat: precompile next.config matchers at build time#536
feat: precompile next.config matchers at build time#536SeolJaeHyeok wants to merge 3 commits intocloudflare:mainfrom
Conversation
commit: |
|
/bigbonk review |
There was a problem hiding this comment.
Review: Precompile config matchers at build time
Good idea — moving regex compilation out of the request path is a clear cold-start win. The refactoring of compileConfigPattern and compileHeaderSourcePattern into reusable helpers is clean, and the threading through all four server paths (dev, App Router entry, Pages Router entry, deploy entry) is thorough.
However, there's one significant correctness issue: the precompiled matchRedirect path regresses to O(n) linear scanning, bypassing the locale-static index optimization that was specifically added to handle apps with 63+ locale-prefixed redirect rules. There are also a couple of smaller issues worth addressing.
Summary of issues
-
[Blocking]
matchRedirectprecompiled path bypasses the locale-static index — theif (compiledPatterns)early-return path does a linear scan over all redirects, losing the O(1) map-lookup optimization for locale-prefixed rules. For apps with many locale redirects, this reintroduces the exact O(n × regex) cost that the index was built to eliminate. -
[Non-blocking] No serialization roundtrip test — the tests verify that generated code contains
__compiledRedirectsand regex-looking strings, but no test evaluates the serialized code to confirm the regex survives theRegExp.toString()→ code-embed →evalcycle. A single roundtrip test would catch subtle serialization bugs (e.g., regex flags, special characters in param names). -
[Non-blocking]
serializeCompiledPatterndoesn't escapeparamNames—JSON.stringifyhandles this correctly for normal param names, but it's worth noting that if a param name ever contained characters that are special in JSON strings (unlikely but possible with future pattern extensions), the current approach is safe because ofJSON.stringify. -
[Non-blocking] Comment stale in
matchHeaders— the comment on line 1107 still referencesescapeHeaderSource() + safeRegExp()as the cache rationale, but the code now has a two-tier lookup (precompiled array first, then module-level cache). The comment should be updated to reflect the new lookup order.
| if (compiledPatterns) { | ||
| for (let i = 0; i < redirects.length; i++) { | ||
| const redirect = redirects[i]; | ||
| const compiled = compiledPatterns[i]; | ||
| const params = compiled | ||
| ? execCompiledConfigPattern(pathname, compiled) | ||
| : matchConfigPattern(pathname, redirect.source); | ||
| if (!params) continue; | ||
| if (redirect.has || redirect.missing) { | ||
| if (!checkHasConditions(redirect.has, redirect.missing, ctx)) { | ||
| continue; | ||
| } | ||
| } | ||
| let dest = substituteDestinationParams(redirect.destination, params); | ||
| dest = sanitizeDestination(dest); | ||
| return { destination: dest, permanent: redirect.permanent }; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Blocking: This precompiled fast path does a linear O(n) scan over all redirects, which bypasses the locale-static index optimization (_getRedirectIndex) that exists specifically for apps with many locale-prefixed redirect rules (63+ rules in the motivating profile).
When compiledPatterns is passed, the locale-static O(1) map lookup + tiny-regex validation is never used — every request linearly tests every redirect pattern. For simple apps this is fine, but for the apps that motivated the locale-static index, this is a regression.
Suggested approach: instead of a separate early-return branch, integrate the precompiled patterns into the existing matchRedirect flow. The locale-static index handles locale patterns; for the linear fallback rules, you could pass the precompiled pattern by original index so matchConfigPattern is skipped when a precompiled version exists. Something like:
// In the linear fallback loop:
for (const [origIdx, redirect] of index.linear) {
if (origIdx >= localeMatchIndex) break;
const compiled = compiledPatterns?.[origIdx];
const params = compiled
? execCompiledConfigPattern(pathname, compiled)
: matchConfigPattern(pathname, redirect.source);
// ...
}This preserves the locale-static fast path while still benefiting from precompilation for non-locale rules.
| // Cache the compiled source regex — escapeHeaderSource() + safeRegExp() are | ||
| // pure functions of rule.source and the result never changes between requests. | ||
| let sourceRegex = _compiledHeaderSourceCache.get(rule.source); | ||
| let sourceRegex = compiledSources?.[i]; |
There was a problem hiding this comment.
Subtle but correct: when compiledSources[i] is null (safeRegExp rejected the pattern at build time), null !== undefined so we correctly skip the runtime cache lookup and fall through to the if (sourceRegex && ...) check which is falsy for null. Good.
However, the comment above (line 1107-1108) is now stale — it describes the old single-tier cache but the code now has a two-tier lookup (precompiled array → module-level cache). Consider updating the comment to reflect the new flow.
|
|
||
| function serializeCompiledPattern(pattern: CompiledConfigPattern | null): string { | ||
| if (!pattern) return "null"; | ||
| return `{ re: ${pattern.re.toString()}, paramNames: ${JSON.stringify(pattern.paramNames)} }`; |
There was a problem hiding this comment.
Minor concern: RegExp.prototype.toString() produces /pattern/flags, which is valid JavaScript when embedded directly in generated code. However, if the regex pattern itself contains a forward slash (e.g., from an escaped path separator), toString() should still produce correct output since the slash is escaped in the regex source. This works correctly for all patterns I can see in the codebase.
That said, it would be good to have a roundtrip test that:
- Calls
buildPrecompiledConfigCodewith a non-trivial config - Evaluates the generated code string (e.g., via
new Function) - Verifies the deserialized regex matches the same inputs as the original
This would catch any edge cases in the serialization format.
| } | ||
|
|
||
| export function compileConfigPattern(pattern: string): CompiledConfigPattern | null { | ||
| if (!usesRegexBranch(pattern)) return null; |
There was a problem hiding this comment.
Worth noting: compileConfigPattern returns null for "simple" patterns (no regex groups, no backslashes, no catch-all suffixes, no dot after param). This means the precompiled arrays will contain null for these patterns, and the matching functions correctly fall back to matchConfigPattern at runtime.
This is a sensible design — simple segment-based patterns are already fast (string split + compare), so there's no need to precompile them into regexes. The doc comment on the exported function should mention this return-null behavior since callers need to understand that null means "use the runtime matcher" rather than "pattern is invalid."
| @@ -1472,7 +1477,7 @@ export default async function handler(request, ctx) { | |||
| let pathname; | |||
There was a problem hiding this comment.
Nit: this line is quite long (exceeds typical line width). Consider breaking it for readability, matching the style used for the afterFiles and fallback calls below:
| let pathname; | |
| const __rewritten = matchRewrite( | |
| cleanPathname, | |
| __configRewrites.beforeFiles, | |
| __postMwReqCtx, | |
| __compiledRewrites.beforeFiles, | |
| ); |
|
Review submitted with "Changes Requested". Here's a summary of the feedback: Blocking issue: The precompiled Non-blocking items:
|
Closes #389
summary
compileConfigPatternandcompileHeaderSourcePatternhelpers fromconfig-matcherswhat changed
packages/vinext/src/config/precompiled-config.tsto build and serialize precompiled config matcher statepackages/vinext/src/config/config-matchers.tsto expose reusable compile helpers and optional compiled matcher inputs for redirects, rewrites,and headers
generateRscEntry()to emit__compiledRedirects,__compiledRewrites, and__compiledHeadersalongside the existing config payloadsgenerateServerEntry()to exportvinextCompiledConfigfor Pages Router production and deploy pathsindex.ts,server/prod-server.ts, anddeploy.tsto thread the precompiled matcher state through the existing config matching flowwhy
PR #387 removed the steady-state bottleneck by caching compiled config patterns per isolate. That keeps warm isolates fast, but low-traffic routes can still
pay the compilation cost on the first request after a cold start.
This PR moves that work out of the request path by compiling config patterns ahead of time for generated entries, and once during dev startup for the live
config path. Matching behavior stays the same, but the first matching request no longer needs to tokenize patterns, run the safe-regex check, and construct
RegExpinstances.test plan
pnpm test tests/shims.test.ts tests/app-router.test.ts tests/entry-templates.test.tspnpm run typecheckpnpm run lint