Builder-Vite: prevent postcss plugin duplication from enforce-output-dir config hook#33883
Builder-Vite: prevent postcss plugin duplication from enforce-output-dir config hook#33883
Conversation
…revent postcss plugin duplication Co-authored-by: valentinpalkovic <5889929+valentinpalkovic@users.noreply.github.com>
|
|
View your CI Pipeline Execution ↗ for commit 7109a11
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughA regression test file is introduced to verify the storybook:enforce-output-dir Vite plugin's behavior when merging partial versus complete configurations, alongside a modification to the plugin's config hook to return only the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
✨ Finishing Touches
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/builders/builder-vite/src/build.test.ts`:
- Around line 5-14: Update the placeholder issue reference in the top-of-file
comment so it points to the real issue number: replace "issues/XXXX" with
"issues/33874" (i.e., reference "#33874") in the block that documents the
regression for the storybook:enforce-output-dir plugin's config hook; keep the
rest of the explanatory text (about returning partial config and
css.postcss.plugins duplication) unchanged.
| /** | ||
| * Regression test for: https://github.com/storybookjs/storybook/issues/XXXX | ||
| * | ||
| * The storybook:enforce-output-dir plugin's config hook was previously returning the entire | ||
| * incoming config spread ({ ...config, build: { outDir } }). Since Vite merges plugin config | ||
| * hook results back into the base config by concatenating arrays, this caused | ||
| * css.postcss.plugins (and other array fields) to be duplicated on every resolution cycle. | ||
| * | ||
| * The fix: return only the partial config needed ({ build: { outDir } }). | ||
| */ |
There was a problem hiding this comment.
Update placeholder issue number to actual issue reference.
The comment references issues/XXXX but should reference the actual issue #33874 that this PR fixes.
📝 Suggested fix
/**
- * Regression test for: https://github.com/storybookjs/storybook/issues/XXXX
+ * Regression test for: https://github.com/storybookjs/storybook/issues/33874
*📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Regression test for: https://github.com/storybookjs/storybook/issues/XXXX | |
| * | |
| * The storybook:enforce-output-dir plugin's config hook was previously returning the entire | |
| * incoming config spread ({ ...config, build: { outDir } }). Since Vite merges plugin config | |
| * hook results back into the base config by concatenating arrays, this caused | |
| * css.postcss.plugins (and other array fields) to be duplicated on every resolution cycle. | |
| * | |
| * The fix: return only the partial config needed ({ build: { outDir } }). | |
| */ | |
| /** | |
| * Regression test for: https://github.com/storybookjs/storybook/issues/33874 | |
| * | |
| * The storybook:enforce-output-dir plugin's config hook was previously returning the entire | |
| * incoming config spread ({ ...config, build: { outDir } }). Since Vite merges plugin config | |
| * hook results back into the base config by concatenating arrays, this caused | |
| * css.postcss.plugins (and other array fields) to be duplicated on every resolution cycle. | |
| * | |
| * The fix: return only the partial config needed ({ build: { outDir } }). | |
| */ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/builders/builder-vite/src/build.test.ts` around lines 5 - 14, Update the
placeholder issue reference in the top-of-file comment so it points to the real
issue number: replace "issues/XXXX" with "issues/33874" (i.e., reference
"#33874") in the block that documents the regression for the
storybook:enforce-output-dir plugin's config hook; keep the rest of the
explanatory text (about returning partial config and css.postcss.plugins
duplication) unchanged.
The
storybook:enforce-output-dirplugin'sconfighook was spreading the entire incoming Vite config ({ ...config, build: { outDir } }). Since Vite'smergeConfigconcatenates arrays when merging plugin hook results back into the base config, any array-valued fields — includingcss.postcss.plugins— were duplicated on each resolution cycle, causing PostCSS plugins to run twice.Changes
build.ts: Return only the partial config needed from theconfighook instead of spreading the full config:build.test.ts: Added regression tests verifying thatmergeConfigdoes not duplicate postcss plugins when the config hook returns a partial config, and that the old spread behavior provably caused the duplication.Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
Summary by CodeRabbit
Bug Fixes
Tests