Change Detection: Build module graph with oxc tooling#34625
Conversation
Registers .vue and .svelte parser plugins via the experimental_importParsers preset hook so single-file components become walkable in Storybook's change-detection dependency graph. Each parser cracks the SFC container with the framework's native compiler (vue/compiler-sfc, svelte/compiler), delegates script-block parsing back to the built-in oxc wrapper, and dedupes edges across <script>/<script setup>/<script module> blocks. Compiler deps are lazy-loaded so projects that opt out of change detection do not pay the import cost. vue/compiler-sfc is accessed as a deep import on the `vue` peer dep (available since Vue 3.2.13), avoiding an explicit @vue/compiler-sfc dependency. Also exposes ChangeDetectionFailureError and ChangeDetectionUnavailableError on storybook/internal/core-server so plugin code can throw the class the change-detection service already catches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `onModuleGraphChange` builder hook and its polling-based Vite module graph walker are superseded by the oxc-resolver–based change-detection service introduced in PR-A, which subscribes to raw file events via `changeDetectionAdapter` and builds its own graph independently of Vite's internal module graph. Removes: - `Builder.onModuleGraphChange` and the `ModuleGraph`/`ModuleGraphChangeEvent`/ `ModuleNode` types from storybook/internal/types - The `buildModuleGraph` walker over Vite's `moduleGraph.fileToModulesMap` - All polling/debounce/listener machinery in `@storybook/builder-vite` - The companion test suite (replaced by change-detection-adapter tests) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a dev-only profiler gated on `STORYBOOK_CHANGE_DETECTION_PROFILE=1` that emits one-line summaries for every dependency-graph build and every IncrementalPatcher patch, plus per-operation counters for files parsed, specifiers resolved, and parser dispatches by file extension. The profiler is a zero-cost no-op when the env flag is unset. Introduces a vitest benchmark at `code/core/src/core-server/change-detection/ChangeDetectionService.bench.ts` that synthesises fixture projects of (N, D) — N stories each importing a linear chain of D dep modules — and measures (a) a cold dependency-graph build and (b) a single IncrementalPatcher.patch round-trip on a warm graph. The CI matrix is capped at N=500/D=3; the worst-case N=5000/D=10 matrix runs locally with `STORYBOOK_CD_BENCH_BIG=1`. Baseline numbers on the current implementation (M-series Mac): N=50 D=1: cold build 4.22 ms mean / patch warm 0.142 ms N=50 D=3: cold build 3.85 ms mean / patch warm 0.151 ms N=500 D=1: cold build 37.40 ms mean / patch warm 0.120 ms N=500 D=3: cold build 37.55 ms mean / patch warm 0.152 ms Vitest config changes support running benches against the `core` project without resolving oxc-parser/oxc-resolver through their WASM browser entry points, and exclude `.bench.ts` files from the browser-based `storybook-ui` project. No behavioural change; every code path remains untouched when the profile env var is absent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…vel substring prefilter
`collectRequireSpecifiers` recursively visits every own-property of every
AST node because oxc-parser's EcmaScriptModule does not surface require()
calls separately. On modern code the walk is nearly always wasted: `.mjs`
and `.mts` files cannot contain a CommonJS `require` edge at all, and
other JS/TS extensions almost never do.
Two cheap gates before paying the recursive walk:
1. Extension check for `.mjs`/`.mts` — skip unconditionally.
2. `source.includes('require(')` substring prefilter for everything else —
a literal `require(` call MUST include that exact token.
Both gates preserve the edge-set exactly; no change for files that really
do contain `require(...)` calls. Existing `.cjs` / `.js` require tests
remain green.
Measured deltas on the synthesized bench (mean ms, vitest bench --run):
cold build patch warm
N=50 D=1 4.22 → 3.72 (-11.8%) 0.142 → 0.110 (-22.5%)
N=50 D=3 3.85 → 3.66 ( -4.9%) 0.151 → 0.135 (-10.6%)
N=500 D=1 37.40 → 35.12 ( -6.1%) 0.120 → 0.109 ( -9.2%)
N=500 D=3 37.55 → 35.05 ( -6.7%) 0.152 → 0.126 (-17.1%)
Patch deltas are the most signal-rich metric (the patch path re-parses a
single file, so the require-walk share of total work is highest there).
Cold build deltas compress at large N because parse time is dominated by
oxc-parser itself, not the JS-level walk.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`walkFromStory` dedupes parsing via a shared `parseCache`, but every story
walk that reaches a shared module was re-running `resolver.resolve()` for
every one of its imports. Running the profiler on a real project showed
the pathological case: 274 stories, 827 parsed files, **111,456 resolver
calls** — roughly 135× redundancy, driven by component-library modules
imported by hundreds of stories.
Introduces a `resolveCache: Map<string, Promise<Set<string>>>` that
hoists the parse + resolve + in-scope filter to once per unique file.
Subsequent story walks that reach the same file `await` the cached
Promise and just enumerate its resolved Set to enqueue their per-story
reverse-index entries. The graph's `Map<string, Set<string>>` shape is
unchanged.
Correctness is covered by a new test asserting that a module imported
by two stories has its outgoing resolver call invoked exactly once:
await builder.build([storyA, storyB]);
expect(sharedEdgeResolves).toHaveLength(1); // was 2
Also downgrades the "Could not resolve" warn to debug in
`IncrementalPatcher` — matches the builder's level for the same message
and avoids noisy warnings for legitimately unresolvable specifiers
(CSS/asset imports that the walker already treats as opaque leaves).
Adds a `shared=20` variant to the synth bench matrix so the scenario
that motivates this change is reproducible. The synth bench does not
show a large delta because vitest bench iterations warm oxc-resolver's
native cache between iterations; the win materialises on true cold-boot
where each resolve is uncached. Real-project measurement is the arbiter
here — the test proves the algorithmic fix.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ready The `STORY_RENDERED` / `DOCS_PREPARED` channel gate meant the change- detection graph build did not kick off until the preview iframe had loaded, requested `/index.json`, loaded a story module, and rendered it. On real projects that is 5–6 seconds of pure waiting before the graph build even starts — the user sees no status badges for most of Storybook's cold-start window. The preview builder's `changeDetectionAdapter()` is ready as soon as `previewBuilder.start()` resolves (the Vite dev server is up at that point). Nothing in the graph build or the scan depends on a story having been rendered — indexing is driven by `storyIndexGeneratorPromise` which is awaited inside `ChangeDetectionService.startInternal` and already runs independently of any browser request. Starting change detection right when the adapter becomes ready parallelises the graph build (≈600 ms after the resolver-cache fix) with the preview's first transform pass and first story render. Wall-clock time to first status badge drops from ~7–8 s to roughly the max of (preview render time, ~600 ms build), which on a warm machine is essentially "as fast as the preview itself can render". No behavioural change beyond the removal of the wait — the adapter, service, and error-handling paths are unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After the `STORY_RENDERED` gate was removed, the dependency-graph build starts right when the preview builder resolves — in parallel with the preview's first transform and first story render. That made latency worse in practice, because `oxc-parser.parse` does its CPU work on the main thread (the native addon posts work to its own pool, but the returned promise is resolved with a sync-feeling callback and every parse we schedule still eats Node's main event loop for long enough that the dev-server's WebSocket channel stalls, pushing the preview's first render behind the scan instead of alongside it). Move the per-file parse into a small `worker_threads` pool. Size defaults to `min(4, max(1, cpus()-1))` and is tunable via `STORYBOOK_CHANGE_DETECTION_WORKERS`; pool usage can be opted out with `STORYBOOK_CHANGE_DETECTION_NO_WORKER=1`. The pool is lazy-initialised on first use, attached as a module singleton, and torn down from `ChangeDetectionService.dispose()`. Both `oxcImportParser.parse` and `ParserRegistry.parseScriptWithOxc` route through the pool, which means Vue/Svelte/MDX plugin parsers that call `ctx.parseScriptWithOxc` also get off-thread treatment for SFC script blocks. Bundling wrinkle: the pool module is inlined into `dist/core-server/index.js` at build time, so `import.meta.url` resolves to the bundle rather than the source path. The pool probes two candidate locations — direct sibling, and the descendant path where the dedicated worker entry actually ships — so it works in both bundled and unbundled (source) layouts. A `STORYBOOK_CHANGE_DETECTION_WORKER_PATH` override is available for bench and custom setups. Fixture bench (`N=500 D=3 shared=20`, worker pool on via path override): cold build 127 ms → 90 ms (~29% faster wall-clock). Patch-on-warm goes 0.10 ms → 0.14 ms, which is the worker-message round-trip; negligible for a single-file save. The bigger win is invisible in the bench: the main thread stays responsive during the build, so channel traffic and the preview's first render are no longer sequenced behind parsing.
- Implemented a test for recovering from a scenario where a changed file is not recorded in the reverse index. - Enhanced the IncrementalPatcher to re-walk stories when a previously unrecorded helper file is added. refactor: update parser registry to remove unused methods - Removed the walkableExtensions method from ParserRegistry as it was not utilized. - Cleaned up related tests to reflect the removal of walkableExtensions. feat: introduce scope utility functions for workspace detection - Added isInsideAnyWorkspace and isInScope functions to determine if a file is within a workspace or project scope. fix: improve error handling in OxcWorkerPool - Enhanced the OxcWorkerPool to reject only the tasks associated with a crashed worker. - Implemented a timeout mechanism for tasks to prevent hanging. test: expand OxcWorkerPool tests for robustness - Added tests for worker failure scenarios, including handling of hung tasks and unexpected exits. - Improved the test structure to ensure proper cleanup and state management. chore: update change detection benchmarks - Removed placeholders for single-edit and bulk-edit scenarios in the change detection benchmark script. - Adjusted reporting scripts to reflect the changes in benchmark results.
Removes five env-var overrides in favour of sensible module-level defaults: - STORYBOOK_CHANGE_DETECTION_NO_WORKER — inline fallback already kicks in automatically when the compiled worker script isn't on disk; no separate opt-out is needed. - STORYBOOK_CHANGE_DETECTION_WORKERS — pool size is min(4, cpus()-1). - STORYBOOK_CHANGE_DETECTION_WORKER_PATH — resolveWorkerScriptPath probes the two import.meta.url-relative candidates; bundling regressions are bugs, not configuration knobs. - STORYBOOK_CHANGE_DETECTION_WORKER_TIMEOUT_MS — hardcoded 30 s; the constructor still accepts a third arg so the regression test can pass 50 ms directly. - STORYBOOK_CHANGE_DETECTION_REQUIRE_WORKER — bench-only guard removed along with its getOxcParsePool import; getOxcParsePool is no longer re-exported from workers/index.ts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
valentinpalkovic
left a comment
There was a problem hiding this comment.
📋 Review Summary
This PR introduces a significant overhaul of Storybook's change-detection pipeline, moving from a builder-coupled approach to a more performant, oxc-based dependency graph managed in the dev-server. The implementation is robust, well-structured, and includes comprehensive testing and worker-thread offloading for performance.
🔍 General Feedback
- MDX Parser: The current regex in
mdx-parse.tsonly handlesimport. It should be extended to supportexport ... fromas well. - Optimization: In
IncrementalPatcher, we can skip re-walking dependent stories if a file's dependency set hasn't changed (e.g., on comment-only changes). - Performance:
ReverseIndex.removeStoryis O(N_deps). For very large projects, a forward index might be beneficial to speed up story removals. - Worker Safety: The
OxcWorkerPoolimplementation is excellent, with proper timeout handling and worker respawning logic.
- Implement forward index (story -> Set<dep>) in ReverseIndex for O(N_story_deps) removeStory performance. - Optimize IncrementalPatcher to skip re-walking dependent stories when dependencies are unchanged. - Enhance MDX parser to capture 'export ... from' declarations. - Add unit test for MDX export extraction.
- Extract shared walkFromStory helper used by both DependencyGraphBuilder and IncrementalPatcher. - Maintain an inverse importer index in IncrementalPatcher so direct-importer recovery is O(1) instead of scanning the full graph on every non-story add. - Run per-story re-walks for unlink/change/recovery in parallel via Promise.all (the shared ParseResolveCache makes this safe). - Cache getStoryIdsByAbsolutePath by storyIndex identity so debounced scans don't rebuild the map every fire. - Replace `Math.min(...allEntries.values())` with an explicit loop to avoid the spread allocation and a RangeError on huge dep sets. - Skip the `previousStatuses` Map clone when nothing changed. - Drop the speculative second worker-script path; only one ever existed. - Simplify parseOnce return type from `ImportEdge[] | null` to `ImportEdge[]`. - Remove unused `private graph` on ChangeDetectionService. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Refresh storyFiles in onStoryIndexInvalidated and replay add/unlink events for stories that joined or left the index after startup, so the patcher recognises runtime-added story files. - Split OxcWorkerPool API: acquireOxcParsePool (init + ref) is paired with disposeOxcParsePool by ChangeDetectionService; getOxcParsePool peeks without changing the refcount and is what parseWithOxc consults. Fixes the unbounded refcount growth caused by per-parse acquires. - OxcWorkerPool.dispose now rejects queued tasks instead of silently clearing the queue, so callers can't hang on disposal. - Track oxcPoolAcquired flag on ChangeDetectionService so dispose only releases when an acquire actually succeeded. - Delete the duplicate argMappings.stories.ts (byte-for-byte copy of argMapping.stories.ts). - Update change-detection docs to describe the ChangeDetectionAdapter contract instead of the removed onModuleGraphChange hook. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move the generic oxc parsing infrastructure (worker pool, inline parser, parseWithOxc adapter) out of change-detection/parser-registry/workers/ into a sibling sub-package at code/core/src/oxc-parser/, exposed via storybook/internal/oxc-parser. Change detection now consumes it as one of many possible callers — pure move + rename, no behavior changes. - Introduce OxcParseError (extends StorybookError) so the sub-package no longer depends back into change-detection/errors.ts. - ImportEdge moves to oxc-parser/types.ts; change-detection re-exports it so existing consumers keep working without churn. - WORKER_RELATIVE_PATH updated to '../oxc-parser/worker.js' to match the new dist layout (sibling of dist/core-server/index.js). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Critical correctness: - ParseResolveCache: invalidate importers on unlink so re-walks don't re-add the deleted dep from a stale resolve cache. - ChangeDetectionService.start: dispose on startup error and re-check disposed before acquiring the worker pool ref. - ChangeDetectionService.scan: await currentPatch so scans don't observe a transiently empty reverseIndex mid-patch. - ChangeDetectionService.dispose: tear down GitDiffProvider watchers. OxcWorkerPool: - dispose rejects pending/queued tasks before awaiting terminate so callers don't hang if termination stalls. - Respawn-failure with no alive workers drains the queue and goes terminal instead of leaving tasks unresolved. - _resetOxcParsePoolForTesting awaits dispose to clean up worker threads between test cases. Cleanup: - mdx-parse strips fenced and inline code regions before scanning to drop false-positive deps from doc examples. - DependencyGraphBuilder doc comment matches reality (graceful per-file degradation, not all-or-nothing); drop unused concurrency knob. - Svelte parser maps the script lang attribute to the matching virtual extension instead of forcing TS mode. - ResolverFactory.warnedRegexAliases moves to instance state. Docs: clarify Vite is the currently-supported builder and add a Troubleshooting section to change-detection.mdx. Tests: 14 new regression tests across IncrementalPatcher, ChangeDetectionService, OxcWorkerPool, mdx parser, and svelteImportParser.
Removes unjustified complexity flagged by Codex+Gemini review: - Production profiler deleted; benchmarking stays in scripts/bench - Worker-pool refcount singleton + supervisor collapsed to lazy module pool with inline-fallback safety net - IncrementalPatcher: drop inverseImporters / recoverViaDirectImporters and collapse the diff-path to lookup-affected-stories + walkStory - WorkspaceLocator removed; scope check reduces to projectRoot exclusion - STORYBOOK_BENCH_MARKER plumbing removed from service - worker.ts asserts parentPort presence instead of optional-chaining - Naming: isSameData -> deepEqual, currentPatch -> patchQueue - Test cleanup: drop implementation-coupled assertions Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move the builder-supplied resolve-config interface out of the change-detection adapter into the shared `core-common` types module and rename to `ModuleResolveConfig`. Builder adapters (vite today, webpack/rspack later) consume the same shape, so the type belongs alongside other cross-cutting builder types rather than under change-detection. JSDoc is re-toned to drop oxc-resolver-specific "opaque-leaf" wording. Also strip "change-detection startup" framing from the ParserRegistry / ImportParser JSDoc so the registry is described in terms of its actual contract (default + plugin parsers) rather than its current single consumer.
- oxfmt format fix for svelteImportParser (line-length wrap) - IncrementalPatcher.patch: on 'change' events, resolve the changed file's new dep set and compare against the stored graph entry; return early if the dep set is unchanged (comment-only / logic-only edits skip the BFS re-walk entirely, leaving the graph and reverse-index accurate as-is) - Add test covering the skip-re-walk optimisation path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove 'A3 acceptance:' severity-label prefix from incremental-patch test title; pin parse-call count to exactly 1 instead of a loose bounds check - Fix misleading test title that claimed events were 'queued' during build when they are actually dropped (subscription not yet installed) - Remove redundant vi.clearAllMocks() before vi.resetAllMocks() in afterEach - Drain patchQueue before disposeOxcParsePool() in dispose() so in-flight patches cannot read the OXC pool after teardown - Add --no-verify flag to sandbox git commit inside e2e test to skip hooks Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…startup Move the git init + initial commit from the E2E test's beforeAll hook into the sandbox task (scripts/tasks/sandbox.ts). The Storybook dev server reads git state at startup — initialising git only in beforeAll (after Storybook is already running) means change detection could never track changes in CI. - sandbox.ts: git init + initial commit at end of run(), after all files are written, before the NX cache copy - change-detection.spec.ts: remove beforeAll/afterAll git setup, gitExec, hasGitHead, and gitInitialisedByTest — sandbox already has a git repo Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Move git init from sandbox.ts (every sandbox) into dev.ts gated on `selectedTask === 'e2e-tests-dev'`. Chromatic and other publishing flows consume the same sandboxes and complain about the extra commit, so the init must only happen in the change-detection E2E test path. Runs inside dev.ts before the storybook command starts so change detection has a HEAD to diff against from the moment the dev server boots. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Quick clarification request on docs wording: the current docs still mention "builder’s module graph" and list "Vite builder" as required, but this PR now introduces a builder-agnostic ChangeDetectionAdapter contract and also adds a webpack implementation. Should docs be updated in this PR to reflect the adapter architecture + webpack support, or is the Vite-only wording intentional for rollout/maturity reasons? |
|
Small follow-up on user-facing terminology in The current Suggested change: That would make screen-reader labels consistent with filter/docs terminology. |
- dev-server: type adapter, demote stack trace to debug - ParseResolveCache: read/parse failures use debug log - ResolverFactory: shorter regex-alias warn, list moved to debug - builder-vite adapter: simplify event filter with Set + type guard
CI orchestrates the dev server via "yarn task dev -s dev" (selectedTask='dev')
and runs e2e-tests-dev as a separate downstream step. The earlier guard
limiting git init to selectedTask === 'e2e-tests-dev' therefore never matched
on CI, leaving the dev sandbox without a git repo and disabling change
detection ("Change detection unavailable: not a git repository").
Drop the guard and always init the dev sandbox's git repo. The init is
idempotent and Chromatic / publishing flows run in dedicated jobs with
their own checkout, so they are unaffected.
Closes #
What I did
This PR rewrites Storybook's file-change detection pipeline from scratch. When you edit a file in dev mode, Storybook needs to figure out which stories are affected and mark them as modified in the sidebar. Previously this logic lived inside the Vite builder and was tightly coupled to Vite's internal module graph — it had no real understanding of the import graph, so changes to non-story files (shared utilities, design tokens, components) often weren't picked up at all.
The new system is builder-agnostic, lives in core, and is built on a real dependency graph.
How it works
Dependency graph on startup — When the dev server starts, Storybook eagerly builds a graph of every import relationship across your project using oxc-resolver. When a file changes, it walks the graph upward to find exactly which stories are affected, rather than rescanning everything.
Parsing in worker threads — Import extraction runs in a pool of worker threads powered by oxc, a Rust-based JS/TS parser. This keeps the main event loop free during cold start and live patches. Workers are bounded to
min(4, cpus()-1)and respawn automatically on crash; a single worker failure only rejects tasks assigned to that slot.Incremental patching — File
add/change/unlinkevents are applied to the live graph one at a time through a Promise-chain mutex, so concurrent saves never corrupt the graph state.Pluggable parsers for Vue and Svelte — Vue and Svelte single-file components embed script blocks that need special handling before import edges can be extracted. Renderers register custom parsers via the new
experimental_importParserspreset key; the core graph stays format-agnostic.Builder adapter contract — Vite now implements a thin
ChangeDetectionAdapterinterface that forwards chokidar file-change events to the central service. Other builders can implement the same interface.Public surface
New experimental exports from
storybook/internal/core-server:ChangeDetectionService,ChangeDetectionAdapter,FileChangeEvent— the core wiringImportParser,ImportParserContext— for custom SFC parsers contributed by renderersexperimental_getChangeDetectionReadiness,Experimental_ChangeDetectionReadiness— readiness status observableexperimental_importParsersChecklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Unit tests cover:
add/change/unlinkacross stories, leaves, and shared helpers; cold-start cascade-failure recovery (a change to a file the cold-start walker never reached triggers a direct-importer scan).worker.exitrejection, partial-construction cleanup, per-task timeout, refcounted singleton.Manual testing
yarn nx compile coreyarn task sandbox --template react-vite/default-ts --start-from autosrc/Button.tsx) and confirm that stories importing it show amodifiedindicator within ~200ms.Documentation
Follow-up: add a MIGRATION.md paragraph for
experimental_importParsersand theChangeDetectionAdaptercontract once the surface stabilises out ofexperimental_.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 pull request has been released as version
0.0.0-pr-34625-sha-b1efbaa3. Try it out in a new sandbox by runningnpx storybook@0.0.0-pr-34625-sha-b1efbaa3 sandboxor in an existing project withnpx storybook@0.0.0-pr-34625-sha-b1efbaa3 upgrade.More information
0.0.0-pr-34625-sha-b1efbaa3valentin/change-detection-perfb1efbaa31777534100)To request a new release of this pull request, mention the
@storybookjs/coreteam.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=34625Summary by CodeRabbit
New Features
Documentation