fix(frontend): pre-bundle the one dependency a story imports at runtime - #2398
Conversation
The xlsx export story fails on CI with "Failed to fetch dynamically imported
module … exceljs.js", and the file it lands on is whichever test was running at
the time rather than the one at fault.
`exceljs` is reached by `await import("exceljs")` inside the export path, so
vite's pre-bundling scan never sees it from the story entry. It is discovered
while the story runs, vite re-optimizes and reloads the page, and the import
that triggered the discovery dies with the page it was loading into.
This is the case the list already exists for, and the comment above it names
this exact symptom. It is also the only third-party package the source imports
dynamically — everything else behind `await import(...)` is an internal alias,
which vite resolves as source and never pre-bundles.
Not reproduced locally. A cold run with both `node_modules/.vite` and the
storybook cache removed still passes here, so the race is one this machine does
not lose; CI is the only place it has been seen. What would disprove the fix is
the same import failing again after this lands.
Signed-off-by: Alexey Panfilov <Alexey.Panfilov@constructor.tech>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Storybook Vitest configuration now includes ChangesStorybook Vitest configuration
Estimated code review effort: 1 (Trivial) | ~2 minutes Suggested reviewers: 🚥 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 |
|
Correcting this after the fact: the change here did not fix the failure, and my note about what would disprove it was met almost immediately. The body said "what would disprove the fix is the same import failing again after this lands." It did — on a merge-queue run whose base already contained this commit, with the identical
A dependency discovered mid-run makes vite reload the page, and the reload kills whichever import is in flight. That import is then reported as the failure — so the slowest thing to load takes the blame. I read the failing import as the cause instead of reading the line above it, and adding it to the list changed nothing. #2399 lists all eighteen Leaving this merged rather than reverting — the entry is correct on its own terms — but the claim in the body was wrong and should not be read as evidence that this class of failure was addressed. |
3 participants
The xlsx export story fails on CI with
and the failure is reported against whichever test happened to be running, not the one at fault — so it reads as an unrelated flake.
Why it happens
exceljsis reached throughawait import("exceljs")inside the export path, so vite's pre-bundling scan never reaches it from a story entry. It is discovered while the story runs: vite re-optimizes, reloads the page, and the very import that triggered the discovery dies with the page it was loading into.This is exactly the case
optimizeDeps.includealready exists for in this config, and the comment above the list names this symptom in as many words.exceljsis also the only third-party package the source imports dynamically — every otherawait import(...)resolves an internal@/alias, which vite treats as source and never pre-bundles. So this closes the class, not just one instance.What I could not do
Reproduce it locally. A cold run with both
node_modules/.viteand the storybook cache removed passes here with and without the change, so the race is one this machine does not lose. CI is the only place it has been observed.That makes the fix reasoned rather than demonstrated, and it is worth saying plainly. What would disprove it: the same import failing again after this lands.
Why it is on its own
It blocks #2340 today and will catch anything else that touches those stories, so it should not wait behind a product change. Nothing else is in this branch.
Summary by CodeRabbit