Angular: only load webpack dependencies on demand#34043
Angular: only load webpack dependencies on demand#34043sod wants to merge 1 commit intostorybookjs:nextfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughTop-level synchronous imports of WebpackDefinePlugin, WebpackIgnorePlugin, and the webpack config helper were moved into the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes 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)
code/frameworks/angular/src/server/framework-preset-angular-cli.ts (1)
21-22: Minor: Remove extraneous space in dynamic import.Line 21 has an extra space between
importand((await import ('@storybook/builder-webpack5')), while line 22 uses the conventionalimport(...)without a space. For consistency, remove the space.Suggested fix
- const { WebpackDefinePlugin, WebpackIgnorePlugin } = await import ('@storybook/builder-webpack5'); + const { WebpackDefinePlugin, WebpackIgnorePlugin } = await import('@storybook/builder-webpack5');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code/frameworks/angular/src/server/framework-preset-angular-cli.ts` around lines 21 - 22, The dynamic import for '@storybook/builder-webpack5' contains an extraneous space ("await import ('@storybook/builder-webpack5')") causing inconsistent style; change it to use the conventional no-space syntax ("await import('@storybook/builder-webpack5')") to match the other dynamic import of './angular-cli-webpack' (refer to the symbols WebpackDefinePlugin, WebpackIgnorePlugin and getWebpackConfig/getCustomWebpackConfig) so both imports follow the same formatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@code/frameworks/angular/src/server/framework-preset-angular-cli.ts`:
- Around line 21-22: The dynamic import for '@storybook/builder-webpack5'
contains an extraneous space ("await import ('@storybook/builder-webpack5')")
causing inconsistent style; change it to use the conventional no-space syntax
("await import('@storybook/builder-webpack5')") to match the other dynamic
import of './angular-cli-webpack' (refer to the symbols WebpackDefinePlugin,
WebpackIgnorePlugin and getWebpackConfig/getCustomWebpackConfig) so both imports
follow the same formatting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3140984d-d102-498e-a495-80030970af66
📒 Files selected for processing (1)
code/frameworks/angular/src/server/framework-preset-angular-cli.ts
code/frameworks/angular/src/server/framework-preset-angular-cli.ts
Outdated
Show resolved
Hide resolved
|
Hi @sod, Thanks for your contribution! Could you please fix the eslint/Prettier issues? |
The dependencies `@angular-devkit/build-angular`, `@storybook/builder-webpack5` and `webpack` are only needed if you use webpack. But if you use https://analogjs.org/docs/integrations/storybook to use angular + storybook via vite, then you only need `@angular/build` and `@storybook/vite` instead. The analogjs plugin stil relies on `@storybook/angular`, but loading webpack dependencies on demand allows one to drop webpack dependencies.
|
@valentinpalkovic my bad. Rebased and ran prettier. |
|
Nice work @sod. How are you planning to drop the Webpack dependencies even though |
|
We use yarn 4. I added to the package.json, which removes it. |
The dependencies
@angular-devkit/build-angular,@storybook/builder-webpack5andwebpackare only needed if you use webpack.But if you use https://analogjs.org/docs/integrations/storybook to use angular + storybook via vite, then you only need
@angular/buildand@storybook/builder-viteinstead.The analogjs plugin still relies on
@storybook/angular, but loading webpack dependencies on demand allows one to drop webpack dependencies./cc @brandonroberts
Summary by CodeRabbit