fix(start): skip plugin-injected entries in manifest chunk scanner#7089
fix(start): skip plugin-injected entries in manifest chunk scanner#7089rr-jimmy-multani wants to merge 3 commits intoTanStack:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughModified chunk-scanning so Rollup “entry” chunks injected by the MF plugin are recognized via Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Bundle Size Benchmarks
Trend sparkline is historical gzip bytes ending with this PR measurement; lower is better. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/start-plugin-core/tests/start-manifest-plugin/manifestBuilder.test.ts (1)
197-255: Add one bundle-order regression case.These cases only cover the path where the real app entry is scanned before the synthetic one. Adding a reversed-order fixture would lock in the behavior for the other traversal path too.
💡 Suggested test
+ test('skips plugin-injected entries even when scanned before the app entry', () => { + const mfHostInit = makeChunk({ + fileName: 'hostInit.js', + isEntry: true, + facadeModuleId: + '/project/node_modules/__mf__virtual/host__H_A_I__hostAutoInit__H_A_I__.js', + }) + const appEntry = makeChunk({ + fileName: 'main.js', + isEntry: true, + facadeModuleId: '/project/src/client.tsx', + }) + + const scanned = scanClientChunks({ + 'hostInit.js': mfHostInit, + 'main.js': appEntry, + }) + + expect(scanned.entryChunk).toBe(appEntry) + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/start-plugin-core/tests/start-manifest-plugin/manifestBuilder.test.ts` around lines 197 - 255, Add a regression test that mirrors the existing "skips __mf__virtual plugin-injected entry chunks" and "skips virtual:mf- plugin-injected entry chunks" cases but supplies the synthetic/plugin-injected entry chunk before the real app entry in the input object to exercise the reversed traversal order; use the same helpers (makeChunk) and call scanClientChunks with the object keys ordered so the plugin chunk comes first (e.g., {'hostInit.js': mfHostInit, 'main.js': appEntry}) and assert scanClientChunks().entryChunk is the appEntry to lock in correct behavior for both traversal orders.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@packages/start-plugin-core/tests/start-manifest-plugin/manifestBuilder.test.ts`:
- Around line 197-255: Add a regression test that mirrors the existing "skips
__mf__virtual plugin-injected entry chunks" and "skips virtual:mf-
plugin-injected entry chunks" cases but supplies the synthetic/plugin-injected
entry chunk before the real app entry in the input object to exercise the
reversed traversal order; use the same helpers (makeChunk) and call
scanClientChunks with the object keys ordered so the plugin chunk comes first
(e.g., {'hostInit.js': mfHostInit, 'main.js': appEntry}) and assert
scanClientChunks().entryChunk is the appEntry to lock in correct behavior for
both traversal orders.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a2f98581-b186-46bd-874a-edd262648500
📒 Files selected for processing (2)
packages/start-plugin-core/src/start-manifest-plugin/manifestBuilder.tspackages/start-plugin-core/tests/start-manifest-plugin/manifestBuilder.test.ts
| // emits hostInit and remoteEntry entries). These are not the app entry. | ||
| const facadeId = bundleEntry.facadeModuleId ?? '' | ||
| const isPluginInjectedEntry = | ||
| facadeId.includes('__mf__virtual') || facadeId.startsWith('virtual:mf-') |
There was a problem hiding this comment.
we will not hardcode "arbitrary" strings here. if at all, i could imagine a generic, configurable filter logic (via a callback for example). however we are restructuring plugins right now, so this has to wait
There was a problem hiding this comment.
No worries! Thank you for taking the time to review this! Is there a thread or PR I can follow while we wait?
There was a problem hiding this comment.
Hey @schiller-manuel! Would TanStack Start ever expose a programmatic API that bundler plugins could use to register entry filters themselves? Or is the expectation that this kind of integration lives entirely in user config?
2 participants
Hey everyone! I'm working on a POC for TanStack Start + Module Federation and hit this issue. This is my first PR here so apologies if I've missed anything from the contributing guide.
The problem
When using
@module-federation/viteas a host,vite buildfails with:@module-federation/viteemits additional entry chunks into the client bundle —hostInit(via__mf__virtual/...) andremoteEntry(viavirtual:mf-...). ThescanClientChunksfunction inmanifestBuilder.tsthrows as soon as it sees more than oneisEntrychunk, which makes it incompatible with any bundler plugin that emits its own entry points.This is being tracked in #7032.
What I changed
scanClientChunksnow usesfacadeModuleIdto detect plugin-injected entries and skip them.facadeModuleIdpreserves the original entry module ID before Rolldown resolves it to a real file path, making it a reliable discriminator:__mf__virtualin theirfacadeModuleId→ MFhostInitentry, skipvirtual:mf-prefix in theirfacadeModuleId→ MFremoteEntry, skipHow I tested it
manifestBuilder.test.tscovering both skip patterns and the existing throw-on-duplicate behaviourtanstack-start-example-basicstill builds cleanlySummary by CodeRabbit
Release Notes
Bug Fixes
Tests