perf: startup and cache micro-optimizations#436
perf: startup and cache micro-optimizations#436Divkix wants to merge 2 commits intocloudflare:mainfrom
Conversation
…ization - Cache hasMdxFiles() result per root directory to avoid redundant recursive filesystem walks when config() fires per Vite environment - Cache resolvePostcssStringPlugins() result per project root to skip repeated existsSync checks across 17 PostCSS config file candidates - Replace byte-by-byte base64 encode/decode in KV cache handler with Buffer APIs for significantly faster serialization - Pass pre-normalized URL to nodeToWebRequest() in App Router prod server to eliminate redundant path normalization in the RSC handler
commit: |
There was a problem hiding this comment.
Pull request overview
This PR targets startup-time performance by caching expensive filesystem operations during repeated Vite config() invocations, and by reducing overhead in request normalization and KV cache serialization.
Changes:
- Add module-level caches for MDX directory scanning and PostCSS string-plugin resolution.
- Optimize KV cache ArrayBuffer base64 serialization/deserialization using
Buffer. - Allow passing a pre-normalized URL into
nodeToWebRequest()and add coverage for the new behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
packages/vinext/src/index.ts |
Adds per-root caches for MDX scanning and PostCSS plugin resolution; exports internals for tests. |
packages/vinext/src/cloudflare/kv-cache-handler.ts |
Switches base64 encode/decode to Buffer and adds input validation before decoding. |
packages/vinext/src/server/prod-server.ts |
Adds urlOverride support to nodeToWebRequest() and passes a pre-normalized URL from the App Router server path. |
tests/startup-cache.test.ts |
New tests covering MDX scan caching and PostCSS resolution caching behaviors. |
tests/node-to-web-request.test.ts |
New tests covering nodeToWebRequest() urlOverride semantics. |
tests/kv-cache-handler.test.ts |
Adds edge-case ArrayBuffer base64 roundtrip tests (large buffers, null bytes, all byte values). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** Matches a valid base64 string (standard alphabet with optional padding). */ | ||
| const BASE64_RE = /^[A-Za-z0-9+/]*={0,2}$/; | ||
|
|
There was a problem hiding this comment.
BASE64_RE only checks the alphabet/padding characters but doesn’t enforce base64 structural validity (e.g., length multiple of 4 / disallowing length % 4 === 1). Because Buffer.from(str, "base64") is permissive, some malformed inputs made of valid characters can decode without throwing, which would bypass the “corrupted entry => cache miss” logic. Consider tightening validation (e.g., a stricter regex or explicit length/padding checks) so invalid base64 reliably triggers the safeBase64ToArrayBuffer() null path.
tests/startup-cache.test.ts
Outdated
|
|
||
| // Second call — should use cache, no filesystem reads | ||
| const result2 = hasMdxFiles(tmpDir, appDir, null); | ||
| expect(result2).toBe(true); | ||
| expect(readdirSpy).not.toHaveBeenCalled(); | ||
|
|
||
| readdirSpy.mockRestore(); |
There was a problem hiding this comment.
The spy is manually restored at the end of the test, but if an assertion throws before reaching mockRestore(), the spy can leak into later tests. Consider wrapping the spy in a try/finally (or adding an afterEach(() => vi.restoreAllMocks()) for the suite) to guarantee cleanup on failures.
| // Second call — should use cache, no filesystem reads | |
| const result2 = hasMdxFiles(tmpDir, appDir, null); | |
| expect(result2).toBe(true); | |
| expect(readdirSpy).not.toHaveBeenCalled(); | |
| readdirSpy.mockRestore(); | |
| try { | |
| // Second call — should use cache, no filesystem reads | |
| const result2 = hasMdxFiles(tmpDir, appDir, null); | |
| expect(result2).toBe(true); | |
| expect(readdirSpy).not.toHaveBeenCalled(); | |
| } finally { | |
| readdirSpy.mockRestore(); | |
| } |
| // Spy on existsSync after first call | ||
| const existsSpy = vi.spyOn(fs, "existsSync"); | ||
|
|
There was a problem hiding this comment.
Similar to the MDX caching test, this spy is restored only on the happy path. If an assertion fails before mockRestore(), the spy remains active and may affect subsequent tests. Consider using try/finally around the spy or an afterEach(() => vi.restoreAllMocks()) cleanup.
tests/startup-cache.test.ts
Outdated
|
|
||
| // Second call — should use cache | ||
| const result2 = await resolvePostcssStringPlugins(dir); | ||
| expect(result2).toBeUndefined(); | ||
| expect(existsSpy).not.toHaveBeenCalled(); | ||
|
|
||
| existsSpy.mockRestore(); |
There was a problem hiding this comment.
This spy cleanup has the same failure-path leak risk as the other existsSync spy above (mockRestore() won’t run if an assertion throws first). Using try/finally around the spy (or afterEach restoreAllMocks) would make the test isolation more robust.
| // Second call — should use cache | |
| const result2 = await resolvePostcssStringPlugins(dir); | |
| expect(result2).toBeUndefined(); | |
| expect(existsSpy).not.toHaveBeenCalled(); | |
| existsSpy.mockRestore(); | |
| try { | |
| // Second call — should use cache | |
| const result2 = await resolvePostcssStringPlugins(dir); | |
| expect(result2).toBeUndefined(); | |
| expect(existsSpy).not.toHaveBeenCalled(); | |
| } finally { | |
| existsSpy.mockRestore(); | |
| } |
Add length % 4 check to base64ToArrayBuffer to reject structurally invalid base64 strings that pass the character-set regex but produce empty/truncated buffers via Buffer.from(). Replace manual mockRestore() calls with afterEach(vi.restoreAllMocks) to prevent spy leaks on assertion failure.
Closes #437
Summary
hasMdxFiles()result per root directory — avoids redundant recursive filesystem walks whenconfig()fires per Vite environment (RSC/SSR/Client = 3+ times)resolvePostcssStringPlugins()result per project root — skips repeatedexistsSyncchecks across 17 PostCSS config file candidates on eachconfig()invocationBufferAPIs for significantly faster serializationnodeToWebRequest()in App Router prod server — eliminates redundantnormalizePathcall in the RSC handler for pathological URLsTest plan
tests/startup-cache.test.ts— MDX scan caching (5) and PostCSS config caching (4)tests/kv-cache-handler.test.ts— 1 MiB buffer, null bytes, all 256 byte valuestests/node-to-web-request.test.ts—urlOverrideparameter behaviortsgo --noEmit) cleanoxlint) cleanoxfmt --check) clean