perf(pages): reuse dev stylesheet dependency analysis

[james-elicx]

Summary

• cache parsed and resolved dependency metadata while building the Pages Router dev stylesheet manifest
• reuse that dependency graph across _app and all page entry traversals while preserving per-entry CSS order and deduplication
• retain full resolved IDs as cache identities while reading their underlying filesystem paths
• keep the generic promise cache in an internal module with direct regression coverage
• move the existing dynamic request source prescan into the native transform filter so irrelevant modules skip JavaScript hook dispatch
• cover shared dependency reuse, query-distinct cache identities, and escaped require identifiers without admitting unrelated Unicode escapes

Performance

Measured with an excluded local Pages Router fixture containing 485 routes and a heavily shared import graph. Each round cleared the fixture's Vite caches and waited for a rendered route marker.

After the latest review fixes, six alternating cold rounds at the exact PR base and head:

Revision	Mean	Median	Range
base	4.033s	4.034s	3.986–4.070s
head	0.996s	0.994s	0.990–1.009s

Mean cold readiness improved by 75.3%.

Validation

• vp test run tests/pages-router.test.ts -t 'dev Pages (client assets expose|cached ISR HTML keeps|custom error HTML includes|_app stylesheet links use|_app stylesheet metadata|client assets do not treat|dependency metadata reuses exact module ids)'
• vp test run tests/dynamic-requests-build.test.ts
• vp run vinext#build

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@2550

npm i https://pkg.pr.new/create-vinext-app@2550

npm i https://pkg.pr.new/vinext@2550

commit: e51b4a8

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original
workers-cache	preview	production
web	preview	production

[github-actions]

Performance benchmarks

Compared e51b4a8 against base 2efac2c using alternating same-runner rounds. Next.js was unchanged and skipped.

1 improved · 0 regressed · 5 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	126.3 KB	126.3 KB	⚫ +0.0%
Client entry size (gzip)	vinext	120.3 KB	120.3 KB	⚫ +0.0%
Dev server cold start	vinext	2.85 s	2.83 s	⚫ -0.8%
Production build time	vinext	3.40 s	3.34 s	🟢 -1.9%
RSC entry closure size (gzip)	vinext	98.0 KB	98.0 KB	⚫ +0.0%
Server bundle size (gzip)	vinext	164.4 KB	164.4 KB	⚫ +0.0%

View detailed results and traces

_{🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head}

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: perf(pages): reuse dev stylesheet dependency analysis

Reviewed the diff, traced the graph-traversal semantics, ran the targeted tests, and ran vp check (fmt + lint + types all green). No blocking correctness issues — the caching refactor preserves per-entry CSS order and dedup. A few minor/observational notes below.

Verified safe

• Dedup key change (seenModules now keyed by raw modulePath instead of the query-stripped cleanModulePath): I traced diamond/shared-import cases. CSS correctness holds because seenAssets (per-entry) dedups by asset value and preserves first-encounter order; a query-distinct script id only causes a redundant re-traversal of the same file, never a duplicated or reordered stylesheet. The PR body flags "query-distinct cache identities" as intentional, which matches.
• Cache layering: the shared getModuleDependencies cache is graph-level while stylesheetAssets/seenAssets stay per-entry — per-entry order/dedup is correctly recomputed on each traversal. Good separation.
• No rejected-promise caching: collectModuleDependencies catches parse errors and returns [] (never throws), so the generic promise cache never memoizes a rejection.
• Escaped require prescan: correct. require is an identifier (unicode-escapable), whereas import is a keyword (cannot be escaped), so only handling escapes on the require side is the right asymmetry. isIdentifierNamed(callee, "require") matches on the parser-decoded name, so requ\u0069re(...) is correctly transformed.

Minor / non-blocking

1. DYNAMIC_REQUEST_PRESCAN adds an i flag over the composed DYNAMIC_IMPORT_PRESCAN (ignore-dynamic-requests.ts:29-32). DYNAMIC_IMPORT_PRESCAN is /\bimport\s*[(/]/ (no flag), but the combined filter is case-insensitive, so the code filter will pass e.g. IMPORT (x) while the in-handler gate mayContainDynamicImport (no i) rejects it. Harmless — the filter is a superset and the handler gate is authoritative — but it means the two gates aren't strictly identical. Worth a one-line comment noting the filter is deliberately a superset of the handler gate.

2. Test coverage gap for the real dev-Pages path. tests/pages-router.test.ts "dev Pages dependency metadata reuses exact module ids" only exercises createModuleDependencyCache with a mock collect; it doesn't touch createDevPagesModuleDependencyReader / collectDevPagesAppStylesheetAssets. It's effectively a duplicate unit test of the generic cache under a name that implies integration coverage. The existing _app stylesheet metadata updates when transitive imports change test does cover the real traversal, so this isn't a coverage hole per se — but consider renaming this new test (e.g. "module dependency cache dedups by exact id") so the intent is clear, or extending it to assert shared reuse across two page entries through the real reader.

3. Over-inclusive REQUIRE_PRESCAN on non-word-boundary matches (e.g. myrequireFn, prerequire match via the un-escaped-letter branch since that branch has no \b). This is strictly narrower than the previous code.includes("require") and only causes redundant parses (AST check is authoritative), so it's not a regression — just noting it's not a true word-boundary gate for the escaped branch.

Nothing here blocks merge. The perf win is real and the refactor is clean.

[ask-bonk]

Review posted to PR #2550.

Summary

I reviewed PR #2550 (perf(pages): reuse dev stylesheet dependency analysis), traced the traversal semantics, ran the targeted tests, and ran vp check — all green. No blocking issues found. The caching refactor is clean and correctly preserves per-entry CSS order and dedup.

Key findings I verified as safe:

• The seenModules dedup key change (raw id vs query-stripped) doesn't affect CSS correctness — seenAssets still dedups by value and preserves order; worst case is a redundant re-traversal.
• Cache layering is correct: graph-level dependency cache is shared, but per-entry order/dedup is recomputed.
• No rejected-promise caching (collectModuleDependencies never throws).
• The escaped-require prescan asymmetry is correct (require is escapable identifier; import is a non-escapable keyword).

Three minor/non-blocking notes posted:

1. DYNAMIC_REQUEST_PRESCAN adds an i flag making the code filter a superset of the in-handler gate — harmless but worth a comment.
2. The new "dev Pages dependency metadata reuses exact module ids" test only exercises the generic cache with a mock, not the real dev-Pages reader — suggest renaming or extending for clarity.
3. The escaped branch of REQUIRE_PRESCAN lacks word boundaries (over-inclusive), but strictly narrower than the old includes("require") — not a regression.

github run