Webpack: Gate lazy-compilation import pipeline on webpack version#34931
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR enables the pipelined-import workaround only when lazy compilation is enabled, the build is not production, and the runtime webpack version is older than 5.101.3 (checked via semver). Tests verify toImportFn output with and without the pipeline implementation. ChangesWebpack pipelined-import version gating and tests
🎯 3 (Moderate) | ⏱️ ~20 minutes Comment |
Co-authored-by: Sidnioulz <5108577+Sidnioulz@users.noreply.github.com>
Sidnioulz
left a comment
There was a problem hiding this comment.
Code LGTM. Let's see what CI says.
There was a problem hiding this comment.
Pull request overview
This PR removes the webpack lazy-compilation import serialization workaround now that the upstream webpack invalidation issue is described as fixed, simplifying Storybook’s generated webpack story import function.
Changes:
- Removed
importPipelineand its dedicated tests. - Simplified
toImportFnto directly await generated importers. - Removed lazy-compilation pipeline option threading from webpack5 virtual module generation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
code/lib/core-webpack/src/to-importFn.ts |
Simplifies generated story import function to call importers directly. |
code/lib/core-webpack/src/importPipeline.ts |
Deletes the previous serialization helper. |
code/lib/core-webpack/src/importPipeline.test.ts |
Deletes tests for the removed helper. |
code/lib/core-webpack/src/index.ts |
Stops exposing the removed pipeline helper through the package barrel. |
code/builders/builder-webpack5/src/preview/virtual-module-mapping.ts |
Removes builder option lookup and pipeline flag passing for generated story modules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…mpilation Co-authored-by: Sidnioulz <5108577+Sidnioulz@users.noreply.github.com>
Co-authored-by: Sidnioulz <5108577+Sidnioulz@users.noreply.github.com>
Co-authored-by: Sidnioulz <5108577+Sidnioulz@users.noreply.github.com>
Co-authored-by: Sidnioulz <5108577+Sidnioulz@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@code/builders/builder-webpack5/src/preview/virtual-module-mapping.ts`:
- Around line 24-35: The isWebpackVersionAtLeast function currently uses
parseInt without handling NaN; update the parsing so each parts[i] and min[i] is
coerced to a numeric value (e.g., const p = Number.isNaN(parsed) ? 0 : parsed)
before comparison to avoid NaN causing incorrect true results, and adjust the
comment above the parsing to accurately state that parseInt stops at non-numeric
characters and we coerce non-numeric parts to 0; reference the function
isWebpackVersionAtLeast and the variables parts, min, and parseInt when making
the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 737366e3-f3ca-45ca-8ae1-f16e2657da87
📒 Files selected for processing (1)
code/builders/builder-webpack5/src/preview/virtual-module-mapping.ts
…e tests Co-authored-by: Sidnioulz <5108577+Sidnioulz@users.noreply.github.com>
Co-authored-by: Sidnioulz <5108577+Sidnioulz@users.noreply.github.com>
Co-authored-by: Sidnioulz <5108577+Sidnioulz@users.noreply.github.com>
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 72 | 74 | 🚨 +2 🚨 |
| Self size | 20.73 MB | 20.43 MB | 🎉 -308 KB 🎉 |
| Dependency size | 36.11 MB | 36.65 MB | 🚨 +539 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 188 | 189 | 🚨 +1 🚨 |
| Self size | 15 KB | 15 KB | 🎉 -18 B 🎉 |
| Dependency size | 30.67 MB | 30.77 MB | 🚨 +101 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 203 | 205 | 🚨 +2 🚨 |
| Self size | 908 KB | 908 KB | 🎉 -144 B 🎉 |
| Dependency size | 88.98 MB | 89.21 MB | 🚨 +231 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 196 | 198 | 🚨 +2 🚨 |
| Self size | 32 KB | 32 KB | 🎉 -36 B 🎉 |
| Dependency size | 87.47 MB | 87.70 MB | 🚨 +231 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 73 | 75 | 🚨 +2 🚨 |
| Self size | 1.08 MB | 1.08 MB | 🚨 +66 B 🚨 |
| Dependency size | 56.84 MB | 57.08 MB | 🚨 +231 KB 🚨 |
| Bundle Size Analyzer | node | node |
Closes #32302
Manual testing
Run/Build webpack project with lazy compilation on
Webpack has fixed the lazy-compilation invalidation bug that originally required Storybook's import serialization workaround. This PR removes that workaround for webpack >= 5.101.3, while keeping it as a fallback for older webpack 5 installations where the bug is still present.
Version-gated pipeline
importPipelineis retained incode/lib/core-webpack/src/importPipeline.tsas a fallback.toImportFnaccepts aneedPipelinedImportoption to conditionally embed the pipeline into the generated virtual module.builder-webpack5readswebpackModule.versionat build time and enables the pipeline only when lazy compilation is active, not in production, and the effective webpack version is below 5.101.3.semver.lt()withsemver.coerce()to safely handle prerelease version strings (e.g.,5.101.3-alpha.1).Simplified import path for modern webpack
Test coverage
importPipeline.test.tsto verify the pipeline serialization behavior remains correct for older webpack versions.to-importFn.test.tsto verify the generatedimportFncode behavior for both pipeline and direct import modes.Summary by CodeRabbit
importPipeline.test.tstests with improved type safety to verify serialization behavior for older webpack versions.to-importFn.test.tsthat verifies generated import behavior and whether a pipelined import implementation is emitted.Summary by CodeRabbit
Bug Fixes
Tests
Chores