fix(start-plugin-core): keep import-protection virtual ids resolvable in server-fn compiler#7727
Conversation
… in server-fn compiler In build mode, the server-fn compiler's resolveId ran every resolved id through cleanId(), which strips the leading \0 that marks a Rollup virtual module. For import-protection's own virtual ids (e.g. mock-edge), that broke the subsequent this.load(): its load handler matches on the \0-prefixed matchers, so the stripped id fell through to vite:load-fallback, which fs.readFile()d the bare id and threw 'Could not load tanstack-start-import-protection:mock-edge:...: ENOENT'. Preserve the \0 prefix for import-protection ids so loadModule routes them to the correct load handler; other ids are still cleaned as before. Fixes TanStack#7725
📝 WalkthroughWalkthroughThe ChangesresolveId fix
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/start-plugin-core/src/vite/start-compiler-plugin/plugin.ts (1)
373-373: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider deriving this prefix from a shared constant to avoid drift.
The literal
'\0tanstack-start-import-protection:'hardcodes the namespace defined inimport-protection/constants.ts(tanstack-start-import-protection:*). If that namespace ever changes, this matcher silently stops preserving the\0prefix and reintroduces theENOENTregression. Sinceconstants.tscurrently exports only the full ids (MOCK_MODULE_ID,MOCK_EDGE_PREFIX, etc.) and no base prefix, exporting a shared base prefix constant and reusing it here (and in the import-protection plugin's exclude regex) would keep the contract in one place.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/start-plugin-core/src/vite/start-compiler-plugin/plugin.ts` at line 373, The matcher in start-compiler-plugin/plugin.ts hardcodes the import-protection namespace prefix, which can drift from the shared ids in import-protection/constants.ts. Add or reuse a shared base prefix constant from constants.ts and update the prefix check in the result handling around the startsWith guard to use that symbol instead of the literal, and also align the import-protection plugin’s exclude regex with the same shared prefix so the namespace contract stays centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/start-plugin-core/src/vite/start-compiler-plugin/plugin.ts`:
- Line 373: The matcher in start-compiler-plugin/plugin.ts hardcodes the
import-protection namespace prefix, which can drift from the shared ids in
import-protection/constants.ts. Add or reuse a shared base prefix constant from
constants.ts and update the prefix check in the result handling around the
startsWith guard to use that symbol instead of the literal, and also align the
import-protection plugin’s exclude regex with the same shared prefix so the
namespace contract stays centralized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c190805c-503c-40be-8e0b-2cf370c14d8c
📒 Files selected for processing (1)
packages/start-plugin-core/src/vite/start-compiler-plugin/plugin.ts
Fixes #7725
Problem
In build mode, when
createServerFn's static analysis (the server-fnstart-compiler-plugin) recursively loads a module whose dependency chain resolves to import-protection'smock-edgevirtual module, the build crashes:Root cause
In
packages/start-plugin-core/src/vite/start-compiler-plugin/plugin.ts, theresolveIdcallback ran every resolved id throughcleanId():cleanId()strips the leading\0that marks a Rollup-internal virtual module. That's correct for filesystem-backed ids, but import-protection's own ids (e.g.\0tanstack-start-import-protection:mock-edge:...) need that\0: the cleaned id is passed toloadModule→this.load({ id }), and import-protection'sloadhandler filters on the\0-prefixed matchers (getResolvedVirtualModuleMatchers()). Without the prefix the filter never matches, so Rollup falls through tovite:load-fallback, whichfs.readFile()s the bare string and throwsENOENT.Fix
Preserve the
\0prefix for import-protection virtual ids; everything else is cleaned as before:This mirrors the discriminator already used elsewhere in the plugin (
import-protection-plugin/plugin.tsspecial-cases\0tanstack-start-import-protection:too), and it fixes the build without disabling import-protection — the mock module resolves and loads exactly as designed.Credit to the detailed root-cause analysis in #7725.
Verification
nx run @tanstack/start-plugin-core:test:build(package + 12 dependency tasks) green.test:types(tsc) clean.vitest run: 486/486 tests pass, no type errors.The crash itself only reproduces in a full
vite buildof a Start app with a denied import in a server-fn's dependency chain (per #7725's repro), so it isn't covered by a unit test here; happy to add an e2e case if you'd like one.🤖 Generated with Claude Code
Summary by CodeRabbit