feat: enable watch mode on in storybook #33482
Conversation
📝 WalkthroughWalkthroughAdds watch-mode handling to the Vite builder: imports Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/builders/builder-vite/src/build.ts (1)
71-90: Critical: Double build execution breaks watch mode.The code calls
viteBuild()twice—once on line 71 and again on line 90. This logic error:
- Breaks watch mode because the watcher attached to the first build result is overridden by the second build
- Wastes resources by building the project twice
- Sets up the custom logger (line 88) after the first build but before the second, meaning the first build lacks proper logging
Additionally, lines 79 and 83 violate coding guidelines by using
console.logandconsole.errordirectly instead of theloggerfromstorybook/internal/node-logger.🔧 Proposed fix
- logger.info('Building storybook with Vite...'); - - const result = await viteBuild(finalConfig as InlineConfig) - - // Narrow by feature, not instanceof - if (finalConfig.build?.watch && 'on' in result) { - const watcher = result as RollupWatcher - logger.info('Watching for changes...'); - watcher.on('event', (event: RollupWatcherEvent) => { - if (event.code === 'BUNDLE_END') { - console.log(`Rebuilt in ${event.duration}ms`) - } - - if (event.code === 'ERROR') { - console.error(event.error) - } - }) - } - finalConfig.customLogger ??= await createViteLogger(); + logger.info('Building storybook with Vite...'); + - await viteBuild(await sanitizeEnvVars(options, finalConfig)); + const result = await viteBuild(await sanitizeEnvVars(options, finalConfig)); + + // Narrow by feature, not instanceof + if (finalConfig.build?.watch && 'on' in result) { + const watcher = result as RollupWatcher; + logger.info('Watching for changes...'); + watcher.on('event', (event: RollupWatcherEvent) => { + if (event.code === 'BUNDLE_END') { + logger.info(`Rebuilt in ${event.duration}ms`); + } + + if (event.code === 'ERROR') { + logger.error(event.error); + } + }); + }This fix:
- Sets up the custom logger before building
- Calls
viteBuild()only once with sanitized env vars- Attaches the watcher to the correct build result
- Uses
logger.infoandlogger.errorinstead of console methodsAs per coding guidelines, use
loggerfromstorybook/internal/node-loggerfor server-side logging.
🤖 Fix all issues with AI agents
In @code/builders/builder-vite/src/build.ts:
- Around line 73-86: Replace direct console usage in the Rollup watcher event
handler with the injected logger: inside the block that checks
finalConfig.build?.watch and casts result to RollupWatcher (variable watcher),
change the BUNDLE_END branch to call logger.info with the rebuilt duration
message and change the ERROR branch to call logger.error with the event.error;
keep existing event.code checks and message formatting but use logger.info(...)
and logger.error(...) instead of console.log/console.error.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/builders/builder-vite/src/build.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/builders/builder-vite/src/build.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/builders/builder-vite/src/build.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/builders/builder-vite/src/build.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/builders/builder-vite/src/build.ts
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/builders/builder-vite/src/build.ts
🧠 Learnings (1)
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to code/vitest.workspace.ts : Vitest configuration is centralized in `code/vitest.workspace.ts` for workspace setup
Applied to files:
code/builders/builder-vite/src/build.ts
🔇 Additional comments (2)
code/builders/builder-vite/src/build.ts (2)
6-6: LGTM!The RollupWatcher type import is appropriate for implementing watch mode support.
69-69: LGTM!The log message appropriately uses the logger from
storybook/internal/node-loggerand provides clear feedback to users.
| // Narrow by feature, not instanceof | ||
| if (finalConfig.build?.watch && 'on' in result) { | ||
| const watcher = result as RollupWatcher | ||
| logger.info('Watching for changes...'); | ||
| watcher.on('event', (event: RollupWatcherEvent) => { | ||
| if (event.code === 'BUNDLE_END') { | ||
| console.log(`Rebuilt in ${event.duration}ms`) | ||
| } | ||
|
|
||
| if (event.code === 'ERROR') { | ||
| console.error(event.error) | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "build.ts" -path "*/builder-vite/src/*" | head -5Repository: storybookjs/storybook
Length of output: 106
🏁 Script executed:
wc -l code/builders/builder-vite/src/build.tsRepository: storybookjs/storybook
Length of output: 107
🏁 Script executed:
sed -n '70,90p' code/builders/builder-vite/src/build.tsRepository: storybookjs/storybook
Length of output: 681
🏁 Script executed:
head -30 code/builders/builder-vite/src/build.tsRepository: storybookjs/storybook
Length of output: 1251
Replace console.log and console.error with logger calls.
The code violates the coding guidelines by using console.log (line 78) and console.error (line 84) directly. Since the file is in code/builders/ and logger is already imported from storybook/internal/node-logger, use logger.info() and logger.error() instead:
if (event.code === 'BUNDLE_END') {
logger.info(`Rebuilt in ${event.duration}ms`)
}
if (event.code === 'ERROR') {
logger.error(event.error)
}
🤖 Prompt for AI Agents
In @code/builders/builder-vite/src/build.ts around lines 73 - 86, Replace direct
console usage in the Rollup watcher event handler with the injected logger:
inside the block that checks finalConfig.build?.watch and casts result to
RollupWatcher (variable watcher), change the BUNDLE_END branch to call
logger.info with the rebuilt duration message and change the ERROR branch to
call logger.error with the event.error; keep existing event.code checks and
message formatting but use logger.info(...) and logger.error(...) instead of
console.log/console.error.
bhavaniravi
force-pushed
the
sb-with-vite-watch
branch
from
13265a0 to
0abfacc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/builders/builder-vite/src/build.ts (1)
71-90: Critical: Duplicate builds detected - the code builds twice unnecessarily.The code now calls
viteBuildtwice:
- Line 71 builds without
sanitizeEnvVars- Line 90 builds with
sanitizeEnvVarsThis is incorrect because:
- It doubles the build time
- The first build bypasses
sanitizeEnvVars, which may be important for security or correctness- The
customLoggerinitialization (line 88) happens after the first build but before the second- Only the first build's result is captured for watch mode, but the second build runs with the proper configuration
🔧 Proposed fix: Use a single build with sanitized env vars
logger.info('Building storybook with Vite...'); - const result = await viteBuild(finalConfig as InlineConfig) + finalConfig.customLogger ??= await createViteLogger(); + const result = await viteBuild(await sanitizeEnvVars(options, finalConfig)); // Narrow by feature, not instanceof if (finalConfig.build?.watch && 'on' in result) { const watcher = result as RollupWatcher logger.info('Watching for changes...'); watcher.on('event', (event: RollupWatcherEvent) => { if (event.code === 'BUNDLE_END') { console.log(`Rebuilt in ${event.duration}ms`) } if (event.code === 'ERROR') { console.error(event.error) } }) } - finalConfig.customLogger ??= await createViteLogger(); - - await viteBuild(await sanitizeEnvVars(options, finalConfig)); const statsPlugin = findPlugin(
🤖 Fix all issues with AI agents
In @code/builders/builder-vite/src/build.ts:
- Around line 79-84: Replace direct console calls in the build watcher: change
console.log(`Rebuilt in ${event.duration}ms`) to logger.info with the same
message and change console.error(event.error) to logger.error(event.error);
ensure the module imports logger from 'storybook/internal/node-logger' at the
top if not already imported and use the existing event variable in the logger
calls to preserve the same messages and error objects.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/builders/builder-vite/src/build.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/builders/builder-vite/src/build.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/builders/builder-vite/src/build.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/builders/builder-vite/src/build.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/builders/builder-vite/src/build.ts
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/builders/builder-vite/src/build.ts
🧠 Learnings (4)
📚 Learning: 2025-11-24T17:49:31.838Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T17:49:31.838Z
Learning: Applies to code/vitest.workspace.ts : Vitest configuration is centralized in `code/vitest.workspace.ts` for workspace setup
Applied to files:
code/builders/builder-vite/src/build.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Do not use `console.log`, `console.warn`, or `console.error` directly unless in isolated files where importing loggers would significantly increase bundle size
Applied to files:
code/builders/builder-vite/src/build.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx} : Use `logger` from `storybook/internal/node-logger` for server-side logging in Node.js code
Applied to files:
code/builders/builder-vite/src/build.ts
📚 Learning: 2025-12-22T22:03:40.123Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-22T22:03:40.123Z
Learning: Applies to code/{renderers}/**/*.{ts,tsx,js,jsx} : Use `logger` from `storybook/internal/client-logger` for client-side logging in browser code
Applied to files:
code/builders/builder-vite/src/build.ts
🔇 Additional comments (2)
code/builders/builder-vite/src/build.ts (2)
6-7: LGTM!The RollupWatcher and RollupWatcherEvent type imports are correctly added to support the new watch mode functionality.
69-70: LGTM!The informational log message correctly uses
logger.info()as per coding guidelines for server-side logging.
bhavaniravi
force-pushed
the
sb-with-vite-watch
branch
from
0abfacc to
b11ca75
Compare
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 192 | 192 | 0 |
| Self size | 75 KB | 75 KB | 🎉 -48 B 🎉 |
| Dependency size | 32.21 MB | 32.23 MB | 🚨 +21 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 192 | 192 | 0 |
| Self size | 118 KB | 118 KB | 0 B |
| Dependency size | 30.40 MB | 30.42 MB | 🚨 +21 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 196 | 196 | 0 |
| Self size | 15 KB | 15 KB | 0 B |
| Dependency size | 28.93 MB | 28.95 MB | 🚨 +21 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 538 | 538 | 0 |
| Self size | 646 KB | 646 KB | 🚨 +120 B 🚨 |
| Dependency size | 59.17 MB | 59.19 MB | 🚨 +21 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 127 | 127 | 0 |
| Self size | 1.12 MB | 1.12 MB | 🎉 -36 B 🎉 |
| Dependency size | 21.78 MB | 21.81 MB | 🚨 +22 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 159 | 159 | 0 |
| Self size | 30 KB | 30 KB | 🚨 +18 B 🚨 |
| Dependency size | 22.96 MB | 22.99 MB | 🚨 +22 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 117 | 117 | 0 |
| Self size | 35 KB | 35 KB | 0 B |
| Dependency size | 19.58 MB | 19.60 MB | 🚨 +22 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 278 | 278 | 0 |
| Self size | 24 KB | 24 KB | 🚨 +12 B 🚨 |
| Dependency size | 44.10 MB | 44.12 MB | 🚨 +21 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 204 | 204 | 0 |
| Self size | 16 KB | 16 KB | 🚨 +12 B 🚨 |
| Dependency size | 33.47 MB | 33.49 MB | 🚨 +21 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 183 | 183 | 0 |
| Self size | 775 KB | 775 KB | 0 B |
| Dependency size | 67.24 MB | 67.26 MB | 🚨 +21 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 176 | 176 | 0 |
| Self size | 30 KB | 30 KB | 0 B |
| Dependency size | 65.81 MB | 65.83 MB | 🚨 +21 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-react-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 170 | 170 | 0 |
| Self size | 18 KB | 18 KB | 0 B |
| Dependency size | 31.24 MB | 31.26 MB | 🚨 +21 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 57 | 57 | 0 |
| Self size | 718 KB | 718 KB | 0 B |
| Dependency size | 12.92 MB | 12.94 MB | 🚨 +21 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
2 participants
Closes: #15946 (vite)
What I did
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
yarn task --task sandbox --start-from auto --template react-vite/default-tsyarn installmain.tsupdate theviteFinalfunction with following snippetyarn run build-storybookwatcherwaiting for new changesDocumentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.