Core and Vite: Use story index as source of truth for Vite paths #30612
Conversation
…stom-indexer-importpath
|
View your CI Pipeline Execution ↗ for commit 43774a7
☁️ Nx Cloud last updated this comment at |
JReinhold
changed the title
importPath in inexersimportPath in indexers
…stom-indexer-importpath
📝 WalkthroughWalkthroughIntroduces a preset-backed, promise-based StoryIndexGenerator singleton with onInvalidated listeners; rewires builder-vite to consume StoryIndex objects and deduplicated import paths; unifies virtual-module loading; batches file events and simplifies invalidate(path, removed); makes importPath optional in index inputs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Preset as Preset System
participant Gen as StoryIndexGenerator (promise)
participant Dev as Core Dev Server
participant Builder as Builder-Vite Plugin
participant Vite as Vite Dev Server
Dev->>Preset: request storyIndexGenerator preset
Preset-->>Dev: storyIndexGeneratorPromise
Dev->>Gen: await storyIndexGeneratorPromise.getIndex()
Gen-->>Dev: StoryIndex
Vite->>Builder: import virtual STORIES_FILE
Builder->>Gen: await storyIndexGeneratorPromise.getIndex()
Gen-->>Builder: StoryIndex
Builder->>Builder: generate importFn script from StoryIndex
Builder-->>Vite: serve virtual module content
Note over Gen,Builder: On file changes (batched):
Gen->>Gen: invalidate(path, removed)
Gen-->>Builder: notify via onInvalidated()
Builder->>Vite: invalidate virtual module / HMR
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
✨ Finishing touches
Comment |
…kjs/storybook into jeppe/custom-indexer-importpath
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 17 | 17 | 0 |
| Self size | 325 KB | 125 KB | 🎉 -200 KB 🎉 |
| Dependency size | 2.00 MB | 2.00 MB | 🎉 -12 B 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 191 | 192 | 🚨 +1 🚨 |
| Self size | 118 KB | 118 KB | 0 B |
| Dependency size | 30.15 MB | 30.39 MB | 🚨 +241 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/html-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 20 | 20 | 0 |
| Self size | 22 KB | 22 KB | 0 B |
| Dependency size | 2.36 MB | 2.16 MB | 🎉 -200 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 | 0 B |
| Dependency size | 21.97 MB | 21.77 MB | 🎉 -200 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/preact-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 20 | 20 | 0 |
| Self size | 13 KB | 13 KB | 0 B |
| Dependency size | 2.35 MB | 2.15 MB | 🎉 -200 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 | 23.15 MB | 22.95 MB | 🎉 -200 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 117 | 117 | 0 |
| Self size | 35 KB | 35 KB | 🎉 -18 B 🎉 |
| Dependency size | 19.77 MB | 19.57 MB | 🎉 -200 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 24 | 24 | 0 |
| Self size | 55 KB | 55 KB | 0 B |
| Dependency size | 27.02 MB | 26.82 MB | 🎉 -200 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/sveltekit
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 25 | 25 | 0 |
| Self size | 56 KB | 56 KB | 🎉 -24 B 🎉 |
| Dependency size | 27.08 MB | 26.88 MB | 🎉 -200 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 114 | 114 | 0 |
| Self size | 35 KB | 35 KB | 0 B |
| Dependency size | 44.18 MB | 43.98 MB | 🎉 -200 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/web-components-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 21 | 21 | 0 |
| Self size | 19 KB | 19 KB | 🚨 +12 B 🚨 |
| Dependency size | 2.39 MB | 2.19 MB | 🎉 -200 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
…stom-indexer-importpath
…stom-indexer-importpath
| } | ||
|
|
||
| invalidate(specifier: NormalizedStoriesSpecifier, importPath: Path, removed: boolean) { | ||
| invalidate(importPath: Path, removed: boolean) { |
There was a problem hiding this comment.
The reason I'm removing the specifier from the invalidate signature, is because it meant that the StoryIndexGenerator implicitly relied on the specifiers to be referential equal, as they were used as keys for Map lookups. This was not obvious, so during my refactor I was creating multiple specifiers with normalizeStories, and they would not be referential equal, which I spent a lot of time debugging.
This change means we rely on the importPath to lookup the specifier, similar to what used to happen in watchStorySpecifier. importPath is just a string, so equality is simpler.
There was a problem hiding this comment.
Did you think about what happens if multiple specifiers match the same import path?
There was a problem hiding this comment.
I didn't think about that case specifically, and that also doesn't work today on next. What I did here, was just copy the existing check that happens in watchStorySpecifier, and run that same check in StoryIndexGenerator too, so no behavior changes: https://github.com/storybookjs/storybook/pull/30612/changes#diff-e677174c1c4567773a801f4662a199c0975fe538f86bdf43d3362dc2c58c3033R72-R76
I've confirmed the behavior is the same between next and this branch, by having the following stories globs:
[
"../src/**/*.stories.@(js|jsx|mjs|ts|tsx)",
{
directory: "../src/stories",
titlePrefix: "Extra",
files: "Button.@(mdx|stories.@(js|jsx|mjs|ts|tsx))",
},
]Which correctly shows the Button stories twice in the sidebar, but when updating the file to add another story, only the first entry is updated in the sidebar, the second stays static.
This seems weird anyway, is there an actual use case for this that I'm missing?
There was a problem hiding this comment.
Funnily enough, the change I'm making here actually makes it easier to support that use case, if that is something that we want?
JReinhold
changed the title
importPath in indexers![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
storybook.code-workspace (1)
18-19: Minor: Redundant Vitest configuration.Both
vitest.workspaceConfigandvitest.rootConfigpoint to the same file./code/vitest.workspace.ts. Verify if this is intentional or if one should point to a different configuration.code/core/src/core-server/utils/watch-story-specifiers.test.ts (1)
11-11: Missingspy: trueoption on mock.Per coding guidelines, use
vi.mock()with thespy: trueoption for all package mocks in Vitest tests.-vi.mock('watchpack'); +vi.mock('watchpack', { spy: true });code/builders/builder-vite/src/plugins/code-generator-plugin.ts (1)
26-34: Consider storing the unsubscribe function for cleanup.The
onInvalidatedmethod returns an unsubscribe function that isn't captured. While this may not be an issue in typical dev server lifecycles, storing and calling it on server close would be cleaner.async configureServer(server) { - (await storyIndexGeneratorPromise).onInvalidated(() => { + const unsubscribe = (await storyIndexGeneratorPromise).onInvalidated(() => { // TODO: this is only necessary when new files are added. // Changes and removals are already watched and handled by Vite, so they actually trigger a double HMR event right now. server.watcher.emit( 'change', getResolvedVirtualModuleId(SB_VIRTUAL_FILES.VIRTUAL_STORIES_FILE) ); }); + server.httpServer?.on('close', unsubscribe); },code/core/src/core-server/utils/index-json.test.ts (1)
18-18: Missingspy: trueoption on Watchpack mock.Per coding guidelines, use
vi.mock()with thespy: trueoption for package mocks.-vi.mock('watchpack'); +vi.mock('watchpack', { spy: true });code/builders/builder-vite/src/codegen-importfn-script.test.ts (1)
9-9: Consider adding mock cleanup for consistency.The
process.cwd()spy is created in each test but not explicitly restored. While Vitest's default behavior handles this, adding anafterEachwithvi.restoreAllMocks()would make the cleanup explicit and prevent potential test pollution if the default behavior changes.import { afterEach, describe, expect, it, vi } from 'vitest'; // Add after imports afterEach(() => { vi.restoreAllMocks(); });code/core/src/core-server/utils/index-json.ts (1)
17-24: Parameter naming inconsistency with the new convention.The parameter is named
initializedStoryIndexGeneratorbut the new convention elsewhere usesstoryIndexGeneratorPromise. While this works, the inconsistency could cause confusion when reading code across files.export async function writeIndexJson( outputFile: string, - initializedStoryIndexGenerator: Promise<StoryIndexGenerator> + storyIndexGeneratorPromise: Promise<StoryIndexGenerator> ) { - const generator = await initializedStoryIndexGenerator; + const generator = await storyIndexGeneratorPromise; const storyIndex = await generator.getIndex(); await writeFile(outputFile, JSON.stringify(storyIndex)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
code/.eslintrc.js(0 hunks)code/builders/builder-vite/src/codegen-importfn-script.test.ts(3 hunks)code/builders/builder-vite/src/codegen-importfn-script.ts(1 hunks)code/builders/builder-vite/src/list-stories.ts(0 hunks)code/builders/builder-vite/src/optimizeDeps.ts(3 hunks)code/builders/builder-vite/src/plugins/code-generator-plugin.ts(3 hunks)code/builders/builder-vite/src/utils/unique-import-paths.ts(1 hunks)code/builders/builder-vite/src/virtual-file-names.ts(0 hunks)code/builders/builder-vite/src/vite-config.test.ts(0 hunks)code/core/src/core-server/build-static.ts(3 hunks)code/core/src/core-server/dev-server.ts(7 hunks)code/core/src/core-server/presets/common-preset.ts(4 hunks)code/core/src/core-server/utils/StoryIndexGenerator.test.ts(9 hunks)code/core/src/core-server/utils/StoryIndexGenerator.ts(7 hunks)code/core/src/core-server/utils/doTelemetry.ts(1 hunks)code/core/src/core-server/utils/getStoryIndexGenerator.ts(0 hunks)code/core/src/core-server/utils/index-json.test.ts(10 hunks)code/core/src/core-server/utils/index-json.ts(3 hunks)code/core/src/core-server/utils/watch-story-specifiers.test.ts(5 hunks)code/core/src/core-server/utils/watch-story-specifiers.ts(3 hunks)code/core/src/csf-tools/CsfFile.test.ts(8 hunks)code/core/src/csf-tools/CsfFile.ts(0 hunks)code/core/src/types/modules/core-common.ts(1 hunks)code/core/src/types/modules/indexer.ts(1 hunks)docs/api/main-config/main-config-indexers.mdx(1 hunks)storybook.code-workspace(1 hunks)
💤 Files with no reviewable changes (6)
- code/.eslintrc.js
- code/builders/builder-vite/src/vite-config.test.ts
- code/builders/builder-vite/src/virtual-file-names.ts
- code/builders/builder-vite/src/list-stories.ts
- code/core/src/core-server/utils/getStoryIndexGenerator.ts
- code/core/src/csf-tools/CsfFile.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/core/src/core-server/utils/doTelemetry.tscode/builders/builder-vite/src/optimizeDeps.tscode/core/src/types/modules/indexer.tscode/builders/builder-vite/src/utils/unique-import-paths.tscode/core/src/core-server/dev-server.tscode/core/src/csf-tools/CsfFile.test.tscode/core/src/core-server/utils/watch-story-specifiers.tscode/builders/builder-vite/src/plugins/code-generator-plugin.tscode/core/src/core-server/utils/StoryIndexGenerator.test.tscode/builders/builder-vite/src/codegen-importfn-script.tscode/builders/builder-vite/src/codegen-importfn-script.test.tscode/core/src/core-server/utils/StoryIndexGenerator.tscode/core/src/core-server/presets/common-preset.tscode/core/src/core-server/build-static.tscode/core/src/types/modules/core-common.tscode/core/src/core-server/utils/index-json.tscode/core/src/core-server/utils/watch-story-specifiers.test.tscode/core/src/core-server/utils/index-json.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/core/src/core-server/utils/doTelemetry.tscode/builders/builder-vite/src/optimizeDeps.tscode/core/src/types/modules/indexer.tscode/builders/builder-vite/src/utils/unique-import-paths.tscode/core/src/core-server/dev-server.tscode/core/src/csf-tools/CsfFile.test.tscode/core/src/core-server/utils/watch-story-specifiers.tscode/builders/builder-vite/src/plugins/code-generator-plugin.tscode/core/src/core-server/utils/StoryIndexGenerator.test.tscode/builders/builder-vite/src/codegen-importfn-script.tscode/builders/builder-vite/src/codegen-importfn-script.test.tscode/core/src/core-server/utils/StoryIndexGenerator.tscode/core/src/core-server/presets/common-preset.tscode/core/src/core-server/build-static.tscode/core/src/types/modules/core-common.tscode/core/src/core-server/utils/index-json.tscode/core/src/core-server/utils/watch-story-specifiers.test.tscode/core/src/core-server/utils/index-json.test.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/core-server/utils/doTelemetry.tscode/builders/builder-vite/src/optimizeDeps.tscode/core/src/types/modules/indexer.tscode/builders/builder-vite/src/utils/unique-import-paths.tscode/core/src/core-server/dev-server.tscode/core/src/csf-tools/CsfFile.test.tscode/core/src/core-server/utils/watch-story-specifiers.tscode/builders/builder-vite/src/plugins/code-generator-plugin.tscode/core/src/core-server/utils/StoryIndexGenerator.test.tscode/builders/builder-vite/src/codegen-importfn-script.tscode/builders/builder-vite/src/codegen-importfn-script.test.tscode/core/src/core-server/utils/StoryIndexGenerator.tscode/core/src/core-server/presets/common-preset.tscode/core/src/core-server/build-static.tscode/core/src/types/modules/core-common.tscode/core/src/core-server/utils/index-json.tscode/core/src/core-server/utils/watch-story-specifiers.test.tscode/core/src/core-server/utils/index-json.test.ts
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/core/src/core-server/utils/doTelemetry.tscode/builders/builder-vite/src/optimizeDeps.tscode/core/src/types/modules/indexer.tscode/builders/builder-vite/src/utils/unique-import-paths.tscode/core/src/core-server/dev-server.tscode/core/src/csf-tools/CsfFile.test.tscode/core/src/core-server/utils/watch-story-specifiers.tscode/builders/builder-vite/src/plugins/code-generator-plugin.tscode/core/src/core-server/utils/StoryIndexGenerator.test.tscode/builders/builder-vite/src/codegen-importfn-script.tscode/builders/builder-vite/src/codegen-importfn-script.test.tscode/core/src/core-server/utils/StoryIndexGenerator.tscode/core/src/core-server/presets/common-preset.tscode/core/src/core-server/build-static.tscode/core/src/types/modules/core-common.tscode/core/src/core-server/utils/index-json.tscode/core/src/core-server/utils/watch-story-specifiers.test.tscode/core/src/core-server/utils/index-json.test.ts
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/core/src/core-server/utils/doTelemetry.tscode/builders/builder-vite/src/optimizeDeps.tscode/core/src/types/modules/indexer.tscode/builders/builder-vite/src/utils/unique-import-paths.tscode/core/src/core-server/dev-server.tscode/core/src/csf-tools/CsfFile.test.tscode/core/src/core-server/utils/watch-story-specifiers.tscode/builders/builder-vite/src/plugins/code-generator-plugin.tscode/core/src/core-server/utils/StoryIndexGenerator.test.tscode/builders/builder-vite/src/codegen-importfn-script.tscode/builders/builder-vite/src/codegen-importfn-script.test.tscode/core/src/core-server/utils/StoryIndexGenerator.tscode/core/src/core-server/presets/common-preset.tscode/core/src/core-server/build-static.tscode/core/src/types/modules/core-common.tscode/core/src/core-server/utils/index-json.tscode/core/src/core-server/utils/watch-story-specifiers.test.tscode/core/src/core-server/utils/index-json.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern*.test.ts,*.test.tsx,*.spec.ts, or*.spec.tsx
Follow the spy mocking rules defined in.cursor/rules/spy-mocking.mdcfor consistent mocking patterns with Vitest
Files:
code/core/src/csf-tools/CsfFile.test.tscode/core/src/core-server/utils/StoryIndexGenerator.test.tscode/builders/builder-vite/src/codegen-importfn-script.test.tscode/core/src/core-server/utils/watch-story-specifiers.test.tscode/core/src/core-server/utils/index-json.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Usevi.mock()with thespy: trueoption for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Usevi.mocked()to type and access the mocked functions in Vitest tests
Implement mock behaviors inbeforeEachblocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking withoutvi.mocked()in Vitest tests
Avoid mock implementations outside ofbeforeEachblocks in Vitest tests
Avoid mocking without thespy: trueoption in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking withvi.mocked()in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests
Files:
code/core/src/csf-tools/CsfFile.test.tscode/core/src/core-server/utils/StoryIndexGenerator.test.tscode/builders/builder-vite/src/codegen-importfn-script.test.tscode/core/src/core-server/utils/watch-story-specifiers.test.tscode/core/src/core-server/utils/index-json.test.ts
code/**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested
Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Aim for high test coverage of business logic (75%+ for statements/lines) using coverage reports
Files:
code/core/src/csf-tools/CsfFile.test.tscode/core/src/core-server/utils/StoryIndexGenerator.test.tscode/builders/builder-vite/src/codegen-importfn-script.test.tscode/core/src/core-server/utils/watch-story-specifiers.test.tscode/core/src/core-server/utils/index-json.test.ts
🧠 Learnings (31)
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Applied to files:
code/builders/builder-vite/src/optimizeDeps.tscode/core/src/core-server/dev-server.tscode/builders/builder-vite/src/plugins/code-generator-plugin.tscode/core/src/core-server/presets/common-preset.tscode/core/src/core-server/build-static.tscode/core/src/core-server/utils/index-json.tscode/core/src/core-server/utils/index-json.test.ts
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/builders/builder-vite/src/optimizeDeps.tscode/core/src/core-server/dev-server.tscode/core/src/core-server/utils/StoryIndexGenerator.test.tscode/core/src/core-server/presets/common-preset.tscode/core/src/types/modules/core-common.tscode/core/src/core-server/utils/index-json.ts
📚 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:
storybook.code-workspace
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to **/*.{js,jsx,json,html,ts,tsx,mjs} : Use ESLint and Prettier configurations that are enforced in the codebase
Applied to files:
storybook.code-workspace
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/core/src/core-server/dev-server.tscode/core/src/core-server/presets/common-preset.tscode/core/src/types/modules/core-common.tscode/core/src/core-server/utils/index-json.tscode/core/src/core-server/utils/index-json.test.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use client-side logger from 'storybook/internal/client-logger' for browser code
Applied to files:
code/core/src/core-server/dev-server.tscode/core/src/core-server/presets/common-preset.tscode/core/src/core-server/utils/index-json.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Follow existing patterns and conventions in the Storybook codebase
Applied to files:
code/core/src/core-server/dev-server.tscode/core/src/core-server/presets/common-preset.tscode/core/src/core-server/utils/index-json.ts
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Applied to files:
code/core/src/core-server/dev-server.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/core/src/csf-tools/CsfFile.test.tscode/builders/builder-vite/src/codegen-importfn-script.test.tscode/core/src/core-server/utils/watch-story-specifiers.test.tscode/core/src/core-server/utils/index-json.test.ts
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/core/src/csf-tools/CsfFile.test.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Write meaningful unit tests that actually import and call the functions being tested
Applied to files:
code/core/src/csf-tools/CsfFile.test.tscode/builders/builder-vite/src/codegen-importfn-script.test.tscode/core/src/core-server/utils/watch-story-specifiers.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/core/src/csf-tools/CsfFile.test.tscode/core/src/core-server/utils/StoryIndexGenerator.test.tscode/builders/builder-vite/src/codegen-importfn-script.test.tscode/core/src/core-server/utils/watch-story-specifiers.test.tscode/core/src/core-server/utils/index-json.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests
Applied to files:
code/core/src/core-server/utils/StoryIndexGenerator.test.tscode/builders/builder-vite/src/codegen-importfn-script.test.tscode/core/src/core-server/utils/watch-story-specifiers.test.tscode/core/src/core-server/utils/index-json.test.ts
📚 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 **/*.{test,spec}.{ts,tsx} : Follow the spy mocking rules defined in `.cursor/rules/spy-mocking.mdc` for consistent mocking patterns with Vitest
Applied to files:
code/core/src/core-server/utils/StoryIndexGenerator.test.tscode/core/src/core-server/utils/watch-story-specifiers.test.tscode/core/src/core-server/utils/index-json.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/core/src/core-server/utils/StoryIndexGenerator.test.tscode/builders/builder-vite/src/codegen-importfn-script.test.tscode/core/src/core-server/utils/watch-story-specifiers.test.tscode/core/src/core-server/utils/index-json.test.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx} : Export functions that need to be tested from their modules
Applied to files:
code/builders/builder-vite/src/codegen-importfn-script.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/builders/builder-vite/src/codegen-importfn-script.test.tscode/core/src/core-server/utils/watch-story-specifiers.test.tscode/core/src/core-server/utils/index-json.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Applied to files:
code/builders/builder-vite/src/codegen-importfn-script.test.tscode/core/src/core-server/utils/watch-story-specifiers.test.tscode/core/src/core-server/utils/index-json.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Each mock implementation should return a Promise for async functions in Vitest
Applied to files:
code/builders/builder-vite/src/codegen-importfn-script.test.tscode/core/src/core-server/utils/index-json.test.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Applied to files:
code/builders/builder-vite/src/codegen-importfn-script.test.tscode/core/src/core-server/utils/watch-story-specifiers.test.tscode/core/src/core-server/utils/index-json.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid direct function mocking without `vi.mocked()` in Vitest tests
Applied to files:
code/builders/builder-vite/src/codegen-importfn-script.test.tscode/core/src/core-server/utils/watch-story-specifiers.test.tscode/core/src/core-server/utils/index-json.test.ts
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: The useGlobals hook from storybook/manager-api returns a tuple where the third element (storyGlobals) is typed as Globals, not Globals | undefined. This means TypeScript guarantees it's always defined, making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/core-server/presets/common-preset.tscode/core/src/core-server/utils/index-json.ts
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: In viewport tool code, when using the `useGlobals` hook from storybook/manager-api, the third returned value `storyGlobals` is guaranteed by TypeScript to be defined (not undefined/null), making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/core-server/presets/common-preset.ts
📚 Learning: 2025-09-17T07:31:04.432Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32484
File: code/core/package.json:326-326
Timestamp: 2025-09-17T07:31:04.432Z
Learning: In Storybook's core package, dependencies like `open` are bundled into the final distribution during the build process, so they should remain in devDependencies rather than being moved to dependencies. End users don't need these packages as separate runtime dependencies since they're included in the bundled code.
Applied to files:
code/core/src/core-server/build-static.ts
📚 Learning: 2025-09-29T13:20:23.346Z
Learnt from: mrginglymus
Repo: storybookjs/storybook PR: 32556
File: code/core/package.json:309-313
Timestamp: 2025-09-29T13:20:23.346Z
Learning: The `fast-printf` dependency in Storybook's core package is bundled into the final distribution during the build process, so it should remain in devDependencies rather than being moved to dependencies, following the same pattern as other bundled dependencies like `open`.
Applied to files:
code/core/src/core-server/build-static.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Applied to files:
code/core/src/core-server/utils/watch-story-specifiers.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Applied to files:
code/core/src/core-server/utils/watch-story-specifiers.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mock()` with the `spy: true` option for all package and file mocks in Vitest tests
Applied to files:
code/core/src/core-server/utils/watch-story-specifiers.test.tscode/core/src/core-server/utils/index-json.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking without the `spy: true` option in Vitest tests
Applied to files:
code/core/src/core-server/utils/watch-story-specifiers.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should match the expected return type of the original function
Applied to files:
code/core/src/core-server/utils/watch-story-specifiers.test.ts
📚 Learning: 2025-11-24T17:49:59.279Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Applied to files:
code/core/src/core-server/utils/watch-story-specifiers.test.ts
🧬 Code graph analysis (11)
code/core/src/core-server/utils/doTelemetry.ts (2)
code/core/src/core-server/utils/StoryIndexGenerator.ts (1)
StoryIndexGenerator(105-906)code/core/src/core-server/withTelemetry.ts (1)
sendTelemetryError(89-149)
code/builders/builder-vite/src/optimizeDeps.ts (3)
code/core/src/core-server/presets/common-preset.ts (1)
storyIndexGenerator(298-331)code/core/src/core-server/utils/StoryIndexGenerator.ts (1)
StoryIndexGenerator(105-906)code/builders/builder-vite/src/utils/unique-import-paths.ts (1)
getUniqueImportPaths(4-6)
code/core/src/core-server/dev-server.ts (3)
code/core/src/common/utils/normalize-stories.ts (1)
normalizeStories(108-114)code/core/src/core-server/utils/StoryIndexGenerator.ts (1)
StoryIndexGenerator(105-906)code/core/src/core-server/utils/index-json.ts (1)
registerIndexJsonRoute(26-67)
code/core/src/core-server/utils/watch-story-specifiers.ts (1)
scripts/task.ts (1)
Path(44-44)
code/builders/builder-vite/src/codegen-importfn-script.ts (1)
code/builders/builder-vite/src/utils/unique-import-paths.ts (1)
getUniqueImportPaths(4-6)
code/core/src/core-server/utils/StoryIndexGenerator.ts (2)
code/core/src/csf-tools/CsfFile.ts (1)
indexInputs(980-1040)scripts/task.ts (1)
Path(44-44)
code/core/src/core-server/presets/common-preset.ts (2)
code/core/src/core-server/utils/StoryIndexGenerator.ts (1)
StoryIndexGenerator(105-906)code/core/src/common/utils/normalize-stories.ts (1)
normalizeStories(108-114)
code/core/src/types/modules/core-common.ts (1)
code/core/src/core-server/utils/StoryIndexGenerator.ts (1)
StoryIndexGenerator(105-906)
code/core/src/core-server/utils/index-json.ts (2)
code/core/src/core-server/utils/StoryIndexGenerator.ts (1)
StoryIndexGenerator(105-906)code/core/src/core-server/utils/watch-story-specifiers.ts (1)
watchStorySpecifiers(42-159)
code/core/src/core-server/utils/watch-story-specifiers.test.ts (2)
code/core/src/common/utils/normalize-stories.ts (1)
normalizeStoriesEntry(38-100)code/core/src/core-server/utils/watch-story-specifiers.ts (1)
watchStorySpecifiers(42-159)
code/core/src/core-server/utils/index-json.test.ts (1)
code/core/src/core-server/utils/index-json.ts (1)
registerIndexJsonRoute(26-67)
🪛 Biome (2.1.2)
code/builders/builder-vite/src/plugins/code-generator-plugin.ts
[error] 68-68: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 69-69: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
🔇 Additional comments (28)
code/builders/builder-vite/src/utils/unique-import-paths.ts (1)
1-6: LGTM!The utility function is well-implemented with clear purpose and concise logic. The use of
Setfor deduplication is appropriate.code/core/src/types/modules/core-common.ts (1)
535-535: LGTM - Public API extension.The addition of the optional
storyIndexGeneratorproperty toStorybookConfigRawis well-typed and maintains backward compatibility. This enables the preset-based initialization pattern described in the PR objectives.code/core/src/csf-tools/CsfFile.test.ts (1)
2110-2156: LGTM - Snapshot updates reflect API changes.The removal of
importPathfromindexInputssnapshots aligns with the PR's shift to a StoryIndex-centric approach where import paths are derived from the index rather than stored in individual inputs.code/builders/builder-vite/src/optimizeDeps.ts (2)
39-39: LGTM - Clean integration of new utility.The use of
getUniqueImportPaths(index)to derive optimization entries from the story index is straightforward and aligns with the StoryIndex-based architecture.
19-27: The fallback in this code is not a silent failure mechanism as suggested. TheStoryIndexGenerator.getIndex()method either returns a validStoryIndexor throws an error viathis.lastError—it does not returnundefinedornullunder normal circumstances. The nullish coalescing operator (??) fallback is unreachable under the current implementation and appears to be defensive programming rather than masking indexing errors.Errors from the story index generation are explicitly thrown and propagated, not silently suppressed. If this code is problematic, it's not because of the error-handling logic shown here—the existing TODO comment (lines 16-17) already flags this function as outdated and in need of reworking.
Likely an incorrect or invalid review comment.
code/core/src/core-server/utils/watch-story-specifiers.ts (3)
42-46: LGTM - API simplification.The signature change from
(specifier, path, removed)to(path, removed)simplifies the invalidation callback by removing the specifier parameter, aligning with the path-based invalidation approach used byStoryIndexGenerator.
119-142: LGTM - Effective event batching.The batching mechanism with a 100ms debounce effectively deduplicates rapid Watchpack events. The approach of storing the last event type per path and processing them in bulk is sound.
144-156: LGTM - Event handlers properly integrated.The event handlers correctly delegate to the batching queue, maintaining the existing logic for determining removal status from the
mtimeparameter.code/core/src/core-server/build-static.ts (2)
99-103: LGTM - Cleaner initialization via presets.The shift from local construction to preset-based retrieval of
storyIndexGeneratorimproves code organization and enables sharing the generator instance across the codebase.Also applies to: 138-142
144-148: LGTM - Type cast is safe in this context.The call to
writeIndexJsonwith the type cast toPromise<StoryIndexGenerator>is safe because the code is within the!options.ignorePreviewblock, ensuring the generator will be initialized.code/core/src/core-server/utils/doTelemetry.ts (2)
19-34: Improved error handling with early return.The error handling correctly catches failures from
getIndexAndStats(), reports them to telemetry, and returns early to avoid sending incomplete telemetry data. This prevents cascading errors while ensuring the failure is tracked.
36-50: LGTM - Payload construction preserved.The telemetry payload construction logic is correctly preserved, with appropriate conditional handling when
indexAndStatsis available. The invariant check ensuresversionCheckis defined whenversionUpdatesis enabled.code/core/src/core-server/utils/watch-story-specifiers.test.ts (2)
23-35: Well-structured timer and event flushing setup.The fake timer lifecycle management and the
flushEventshelper correctly support the new batching behavior inwatchStorySpecifiers. This pattern ensures deterministic test execution for debounced events.
91-114: Good use of real timers for filesystem I/O test.Correctly switches to real timers for the globby-based directory scanning test, with a clear explanatory comment. The
vi.waitFor()pattern is appropriate for asserting on async operations.code/builders/builder-vite/src/plugins/code-generator-plugin.ts (1)
65-83: Clean refactor to switch-based virtual module loading.The unified switch pattern for handling virtual files is cleaner than previous branching logic. The implicit
undefinedreturn for unmatched IDs correctly defers to other plugins.code/core/src/core-server/utils/index-json.test.ts (1)
581-637: Comprehensive debounce behavior test.Good coverage of the batching and debouncing interaction, testing both leading and trailing edge emissions. The use of
vi.importActualto test real debounce behavior alongside mocked implementations is a solid pattern.code/core/src/core-server/utils/StoryIndexGenerator.ts (2)
438-446: Clean helper for custom importPath support.The
toImportPathhelper correctly handles the three cases: undefined (falls back to fileName), virtual modules (preserved as-is), and filesystem paths (normalized to relative). This enables the customimportPathfeature for Vite indexers.
861-866: Well-designed invalidation listener API.The
onInvalidatedmethod follows a clean subscription pattern with proper unsubscribe capability. This enables the Vite builder to react to index changes for HMR.docs/api/main-config/main-config-indexers.mdx (1)
120-127: Clear documentation of importPath behavior and limitations.The added default value and the Vite-only warning accurately reflect the implementation. This provides clear guidance for users wanting to use custom
importPathvalues with virtual modules.code/core/src/types/modules/indexer.ts (1)
106-107: LGTM! Clear documentation for the optional importPath field.The type change from required to optional is well-documented, explaining that it defaults to the
fileNameargument passed tocreateIndex. This enables custom indexers to specify alternative import paths (including virtual modules) while maintaining backward compatibility.code/core/src/core-server/utils/StoryIndexGenerator.test.ts (1)
2121-2121: LGTM! Test updates correctly reflect the simplified invalidate API.All
invalidate()calls have been updated from the three-argument signature(specifier, path, removed)to the new two-argument signature(path, removed). The tests maintain comprehensive coverage for file changes, file removals, and dependent invalidation scenarios.code/builders/builder-vite/src/codegen-importfn-script.test.ts (1)
8-45: Tests correctly validate the new StoryIndex-based API.The test properly validates:
- POSIX path handling with correct normalization
- Virtual module imports (
virtual:story.js) are passed through unchanged- The generated
importFnstructure matches expectationscode/core/src/core-server/dev-server.ts (2)
49-59: LGTM! Clean integration of promise-based generator pattern.The setup correctly:
- Obtains the generator promise via presets
- Passes all required parameters to
registerIndexJsonRoute- Maintains proper async flow throughout
42-47: The review comment describing duplicate story normalization as problematic is inaccurate.The implementation is sound. Both
dev-server.tsandcommon-preset.tscallnormalizeStoriesindependently, but this does not cause issues. The invalidation mechanism inStoryIndexGeneratoruses regex pattern matching (viaimportPathMatcher.exec()), not object reference comparison. When files change,watchStorySpecifiersinvokes the generator'sinvalidate()method with only the file path, and the generator locates the matching specifier in its own cache using regex matching. SincenormalizeStoriesis deterministic, both calls produce specifiers with identical regex patterns, making the separate instances functionally equivalent.Likely an incorrect or invalid review comment.
code/builders/builder-vite/src/codegen-importfn-script.ts (1)
16-35: LGTM! Clean refactor to index-based import generation.The implementation correctly:
- Handles virtual modules by passing them through unchanged
- Normalizes relative paths to always start with
./or../- Uses absolute paths for the actual import to ensure correct resolution
The use of
patheensures cross-platform compatibility for path operations.code/core/src/core-server/utils/index-json.ts (1)
26-66: LGTM! Clean route registration with proper async handling.The function correctly:
- Sets up debounced invalidation for the server channel
- Watches story specifiers and config files for changes
- Invalidates the generator appropriately on file changes
- Serves the index via the
/index.jsonroute with proper error handlingThe double-await pattern on line 59 is correct for the promise-wrapped generator.
code/core/src/core-server/presets/common-preset.ts (2)
306-328: LGTM! Well-structured async initialization.The async IIFE pattern correctly:
- Gathers all required configuration
- Normalizes stories with proper context
- Fetches indexers and docs in parallel for efficiency
- Creates and initializes the generator before returning
The initialization is properly awaited before returning the generator instance.
294-304: The module-level singleton pattern is intentional for caching the generator within a dev server session. Tests are unaffected as they instantiateStoryIndexGeneratordirectly rather than through the preset system, and hot reload is handled by the generator'sinvalidateAll()method which clears caches on config changes.
code/builders/builder-vite/src/plugins/code-generator-plugin.ts
Outdated
Show resolved
Hide resolved
code/builders/builder-vite/src/plugins/code-generator-plugin.ts
Outdated
Show resolved
Hide resolved
…kjs/storybook into jeppe/custom-indexer-importpath
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
code/builders/builder-vite/src/plugins/code-generator-plugin.ts (1)
71-75: Consider removing optional chaining if typings guarantee non-null values.The optional chaining on line 73 (
storyIndexGenerator?.getIndex()) suggests defensive programming, but a past review indicated that typings should ensuregetIndexalways returns an index. If the presets system guarantees thatstoryIndexGeneratorPromiseresolves to a non-nullStoryIndexGenerator, the optional chaining is unnecessary.Apply this diff if typings guarantee non-null values:
const storyIndexGenerator = await storyIndexGeneratorPromise; - const index = await storyIndexGenerator?.getIndex(); + const index = await storyIndexGenerator.getIndex(); return generateImportFnScriptCode(index);code/core/src/core-server/utils/StoryIndexGenerator.ts (2)
438-446: Clarify the expected format of customimportPathvalues.The
toImportPathhelper assumes that non-virtualpathvalues are absolute. If a custom indexer mistakenly provides a relative path (e.g.,'./stories/Button.js'), thenrelative(workingDir, path)may produce unexpected results because Node.js'srelativefunction treats both arguments as relative to the current working directory when the second argument is relative.Consider either:
- Documenting that custom indexers must provide absolute paths (or virtual paths), or
- Adding defensive handling to resolve relative paths first:
const toImportPath = (path: string | undefined) => { if (!path) { return importPath; } if (path.startsWith('virtual:')) { return path; } + const absolutePath = resolve(this.options.workingDir, path); - return slash(normalizeStoryPath(relative(this.options.workingDir, path))); + return slash(normalizeStoryPath(relative(this.options.workingDir, absolutePath))); };If the API contract already requires absolute paths, then this is fine as-is and just needs documentation.
806-806: Consider error handling for listener notifications.The invalidation listener pattern is well-implemented with a standard subscribe/unsubscribe API. However, listener invocations (lines 806, 858) lack error handling. If a listener throws, subsequent listeners won't be called, which could lead to inconsistent state.
Consider wrapping listener calls in try-catch blocks:
- this.invalidationListeners.forEach((listener) => listener()); + this.invalidationListeners.forEach((listener) => { + try { + listener(); + } catch (err) { + logger.error('Error in invalidation listener:', err); + } + });This ensures all listeners are called even if one fails, improving robustness.
Also applies to: 858-859, 861-866
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
code/builders/builder-vite/src/plugins/code-generator-plugin.ts(3 hunks)code/core/src/core-server/dev-server.ts(7 hunks)code/core/src/core-server/utils/StoryIndexGenerator.ts(7 hunks)code/core/src/core-server/utils/index-json.test.ts(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- code/core/src/core-server/dev-server.ts
- code/core/src/core-server/utils/index-json.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/builders/builder-vite/src/plugins/code-generator-plugin.tscode/core/src/core-server/utils/StoryIndexGenerator.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/builders/builder-vite/src/plugins/code-generator-plugin.tscode/core/src/core-server/utils/StoryIndexGenerator.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/builders/builder-vite/src/plugins/code-generator-plugin.tscode/core/src/core-server/utils/StoryIndexGenerator.ts
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/builders/builder-vite/src/plugins/code-generator-plugin.tscode/core/src/core-server/utils/StoryIndexGenerator.ts
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/builders/builder-vite/src/plugins/code-generator-plugin.tscode/core/src/core-server/utils/StoryIndexGenerator.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/builders/builder-vite/src/plugins/code-generator-plugin.tscode/core/src/core-server/utils/StoryIndexGenerator.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Applied to files:
code/builders/builder-vite/src/plugins/code-generator-plugin.ts
🧬 Code graph analysis (1)
code/core/src/core-server/utils/StoryIndexGenerator.ts (2)
code/core/src/csf-tools/CsfFile.ts (1)
indexInputs(980-1040)scripts/task.ts (1)
Path(44-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
🔇 Additional comments (6)
code/builders/builder-vite/src/plugins/code-generator-plugin.ts (2)
30-38: Acknowledge the TODO about double HMR events.The TODO comment indicates that changes and removals currently trigger duplicate HMR events. While this doesn't break functionality, it may impact developer experience during hot reload.
Consider verifying the impact of these double HMR events on development workflow, or track this as a follow-up optimization task.
59-67: LGTM! Generalized virtual file resolution.The approach now handles multiple virtual file IDs generically, which is cleaner than hard-coded cases. The explicit
undefinedreturn is good practice for Vite plugins.code/core/src/core-server/utils/StoryIndexGenerator.ts (4)
124-125: LGTM: Listener infrastructure added.The invalidation listener Set is properly initialized and typed.
448-484: LGTM: Story entries now usetoImportPathfor flexible import paths.The use of
toImportPath(input.importPath)correctly supports custom indexers providing alternative import paths, including virtual modules.
496-504: LGTM: Autodocs entries now use processed story data.Using
storyEntries[0]instead ofindexInputs[0]ensures that autodocs entries use the same transformedimportPathandtitleas their corresponding story entries, maintaining consistency.
809-817: LGTM: Dynamic specifier lookup avoids referential equality issues.The signature change to
invalidate(importPath, removed)removes the implicit reliance on specifier referential equality, as explained in the PR author's comment. TheArray.from()conversion is correctly applied.
| } | ||
|
|
||
| invalidate(specifier: NormalizedStoriesSpecifier, importPath: Path, removed: boolean) { | ||
| invalidate(importPath: Path, removed: boolean) { |
There was a problem hiding this comment.
Did you think about what happens if multiple specifiers match the same import path?
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/core/src/core-server/build-static.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/core/src/core-server/build-static.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/core/src/core-server/build-static.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/core-server/build-static.ts
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/core/src/core-server/build-static.ts
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/core/src/core-server/build-static.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx,mjs} : Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Applied to files:
code/core/src/core-server/build-static.ts
📚 Learning: 2025-09-17T07:31:04.432Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32484
File: code/core/package.json:326-326
Timestamp: 2025-09-17T07:31:04.432Z
Learning: In Storybook's core package, dependencies like `open` are bundled into the final distribution during the build process, so they should remain in devDependencies rather than being moved to dependencies. End users don't need these packages as separate runtime dependencies since they're included in the bundled code.
Applied to files:
code/core/src/core-server/build-static.ts
📚 Learning: 2025-09-29T13:20:23.346Z
Learnt from: mrginglymus
Repo: storybookjs/storybook PR: 32556
File: code/core/package.json:309-313
Timestamp: 2025-09-29T13:20:23.346Z
Learning: The `fast-printf` dependency in Storybook's core package is bundled into the final distribution during the build process, so it should remain in devDependencies rather than being moved to dependencies, following the same pattern as other bundled dependencies like `open`.
Applied to files:
code/core/src/core-server/build-static.ts
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/core/src/core-server/build-static.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
🔇 Additional comments (4)
code/core/src/core-server/build-static.ts (4)
22-26: LGTM!Type-only import for
StoryIndexGeneratoris appropriate since it's only used in type positions. The newwriteIndexJsonimport aligns with the refactored index utilities.
99-103: LGTM!Simplified parallel initialization reflects the new architecture where story index generation is sourced from presets rather than constructed locally.
154-155: LGTM!Proper null guard before using
indexGeneratorfor manifest generation.
225-226: LGTM!Optional chaining safely handles potentially undefined generator, and the surrounding try-catch ensures telemetry failures don't break the build.
Supersedes #26010
Todo
importPathinStoryIndexGeneratorand ensure their behavior still work.importPathis expected to be a file path, but can now be a virtual module, and I'm not sure yet if it's supported in all cases there.initializedStoryIndexGeneratorto a preset instead if possibleAlso with virtual filesimportPathas optional, both in types and docsWhat I did
This PR does a few things, all based on making the Vite builder depend on the story index for a list of stories, instead of re-globbing the files manually like the StoryIndexGenerator already does.
Untying the Vite builder from the file system means it no longer has to do duplicate file watching to make HMR work, instead
StoryIndexGeneratorwill directly notify the Vite builder when the virtual modules are invalidated.This enables the feature of custom indexers specifying an alternative path as the
importPathfor an index, as those are now the single source of truth. The paths can even be virtual modules, or paths with query param suffixes. However virtual modules will not work with the Vitest integration as Vitest doesn't support that.An important addition to this PR, is that the StoryIndexGenerator is now available as a preset property for anyone to access on the server side. We've been needing this for quite a few features now, and we finally don't have to work around it anymore.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
In a
react-vitesandbox, modify.storybook/main.tsstoriesandaddonsto:Add the following
.storybook/custom-indexers.tsfile:Add the following file to
src/stories/my.stories.json:{ "stories": [ { "title": "Bit", "name": "First" }, { "title": "Bit", "name": "Second" }, { "title": "Bit", "name": "Third" }, { "title": "Bot", "name": "First" }, { "title": "Bot", "name": "Second" }, { "title": "Bot", "name": "Third" } ] }And this file too:
src/stories.Flimflom.stories-emoji.ts:Documentation
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 canary-release-pr.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
Refactor
Performance
Public API / Config
Documentation
Tooling
✏️ Tip: You can customize this high-level summary in your review settings.