Repository navigation
Docgen: Run experimentalDocgenServer extraction in a worker thread - #35324
Merged
Merged
Conversation
Move experimentalDocgenServer's React docgen off the main thread into a single long-lived worker_threads worker owned by core, so the CPU-bound TypeScript program build never blocks Vite's dev server. Providers are described as serializable descriptors (a module specifier) and composed middleware-style inside the worker; the worker spawns lazily on the first extract. There is no in-process fallback — when the compiled worker script is absent, docgen registration is skipped. Co-authored-by: Cursor <cursoragent@cursor.com>
The React renderer contributes a DocgenProviderDescriptor pointing at a worker-target module that runs react-component-meta extraction inside core's docgen worker. The renderer owns no threading code. Extraction builds its own ComponentMetaManager in the worker, so the manifest generator's shared singleton is now scoped to the manifest path only. Co-authored-by: Cursor <cursoragent@cursor.com>
In dev, hold the Controls panel's docgen query until the story reaches a safe point in its lifecycle (STORY_FINISHED by default, or STORY_PREPARED via the STORYBOOK_DOCGEN_STORY_PREPARED env var) so extraction doesn't contend with first render. Static builds fetch precomputed docgen immediately. Also stop the panel from flashing the "No controls" empty state before docgen resolves. Co-authored-by: Cursor <cursoragent@cursor.com>
The addon-docs docgen provider only existed for debugging; remove it now that the worker-based docgen is production-bound. Co-authored-by: Cursor <cursoragent@cursor.com>
JReinhold
added
maintenance
JReinhold
changed the title
Reconciles next's docgen service-registration refactor with the docgen worker: - docgen/server.ts: take next's split (registerDocgenService + extracted extraction-service.server.ts); our only prior delta there was a comment. - common-preset.ts: feed registerDocgenService from the worker client, gated so docgen is skipped when the built worker script is absent, and register story-docs via next's separate registerStoryDocsService. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR moves experimental docgen to a worker-descriptor model, updates server and renderer wiring for the new worker entrypoint, and adds a development-only ControlsPanel gate that waits for story lifecycle events before querying docgen. ChangesDocgen Worker Architecture
ControlsPanel Docgen Gate
Sequence Diagram(s)sequenceDiagram
participant ReactPreset
participant common_preset as common-preset
participant DocgenWorkerClient
participant DocgenWorker as docgen-worker
participant ControlsPanel
ReactPreset->>common_preset: experimental_docgenProvider -> descriptors
common_preset->>DocgenWorkerClient: createDocgenWorkerClient(descriptors)
DocgenWorkerClient->>DocgenWorker: init(descriptors)
DocgenWorker->>DocgenWorkerClient: init ack
ControlsPanel->>ControlsPanel: useStoryDocgenGateReady(storyId)
ControlsPanel->>DocgenWorkerClient: extract(entry)
DocgenWorkerClient->>DocgenWorker: extract(entry)
DocgenWorker->>DocgenWorkerClient: payload or error
DocgenWorkerClient->>ControlsPanel: resolve docgen result
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
code/core/src/builder-manager/utils/template.ts (1)
68-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFix the default lifecycle event name in the comment.
The hook defaults to
STORY_FINISHED, notSTORY_RENDERED; this comment currently documents the wrong cross-layer contract.Proposed fix
- // Opt-in via STORYBOOK_DOCGEN_STORY_PREPARED: request server docgen for the Controls panel at - // STORY_PREPARED instead of the default STORY_RENDERED. See useStoryDocgenGateReady. + // Opt-in via STORYBOOK_DOCGEN_STORY_PREPARED: request server docgen for the Controls panel at + // STORY_PREPARED instead of the default STORY_FINISHED. See useStoryDocgenGateReady.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/core/src/builder-manager/utils/template.ts` around lines 68 - 70, The inline comment in template.ts documents the wrong default lifecycle event for the docgen gate. Update the wording around DOCGEN_STORY_PREPARED to say the hook defaults to STORY_FINISHED instead of STORY_RENDERED, keeping the rest of the explanation about opting into STORYBOOK_DOCGEN_STORY_PREPARED and useStoryDocgenGateReady intact.code/core/src/controls/components/ControlsPanel.stories.tsx (1)
355-376: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the build-mode UI, not only the subscription.
This story says docgen loads immediately in build mode, but it only checks the service call. Add a rendered-control assertion so the story catches regressions where the panel subscribes but remains stuck in the loading state.
Proposed assertion
- play: async () => { + play: async ({ canvas }) => { await waitFor(() => expect(serviceGetDocgen).toHaveBeenCalledWith({ id: 'example-button' })); + await expect(await canvas.findByRole('radio', { name: 'primary' })).toBeInTheDocument(); },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/core/src/controls/components/ControlsPanel.stories.tsx` around lines 355 - 376, The story ServiceDocgenLoadsImmediatelyInBuild only verifies that serviceGetDocgen is called, so it can miss cases where ControlsPanel still renders a loading state. Update the play function to also assert the rendered UI after the docgen request, using the existing ControlsPanel/story setup and the example control id, so the story confirms the build-mode panel actually displays the loaded docgen content instead of just subscribing.code/core/src/shared/open-service/services/docgen/worker/docgen-worker-client.test.ts (1)
19-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the repo’s Vitest spy-mocking pattern here.
These mocks are declared with custom factories instead of
vi.mock(..., { spy: true }), and Line 74 overrides a mock inside the test body instead ofbeforeEach. That diverges from the required pattern for*.test.tsin this repo. As per coding guidelines, "Usevi.mock()with thespy: trueoption for all package and file mocks in Vitest tests" and "Implement mock behaviors inbeforeEachblocks in Vitest tests".Also applies to: 72-75
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/core/src/shared/open-service/services/docgen/worker/docgen-worker-client.test.ts` around lines 19 - 46, The test setup is using custom mock factories for node:worker_threads, node:fs, ../../../../utils/module.ts, and storybook/internal/node-logger instead of the repo’s Vitest spy-mocking pattern. Update the docgen-worker-client.test.ts mocks to use vi.mock(..., { spy: true }) for package/file imports, and move any behavior overrides currently done in the test body (such as the mock implementation around the importMetaResolve/worker behavior) into a beforeEach block. Keep the changes aligned with the existing FakeWorkerImpl, fakeWorkers, and docgen-worker-client test helpers so the test still controls worker behavior through spies.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/core/src/controls/components/ControlsPanel.stories.tsx`:
- Around line 238-242: The play function in ControlsPanel.stories.tsx uses
waitFor for a negative assertion on serviceGetDocgen.subscribe, which can pass
before effects settle. Replace this with a stable synchronization point in the
story flow, then assert synchronously that subscribe was not called; apply the
same pattern to the other affected story play functions in this file and keep
the check anchored to serviceGetDocgen.subscribe and the relevant play handlers.
In `@code/core/src/controls/components/ControlsPanel.tsx`:
- Line 206: The loading gate in ControlsPanel is still driven by prepared in
build-mode, which keeps the panel stuck loading after precomputed docgen
arrives. Update the isLoading decision in ControlsPanel so non-development mode
is treated as prepared for this check, and ensure the call site around the
prepared prop passed from the render logic no longer forces false to block
loading. Use the existing isStoryPrepared, isInitialLoading, hasAnyControl, and
prepared symbols to locate and adjust the logic consistently.
- Around line 222-245: The gate readiness in useStoryDocgenGateReady is being
reset in a post-render effect, which allows stale ready state to leak when
storyId changes. Make ready derive from storyId directly so each story starts
gated closed in development until its own event arrives, and update the
useEffect/useChannel logic in ControlsPanel accordingly. Keep the readiness
state keyed to the current storyId so LoadedServiceControlsPanel and
useServiceQuery cannot mount before the matching STORY_PREPARED/STORY_FINISHED
payload is received.
In
`@code/core/src/shared/open-service/services/docgen/worker/docgen-worker-client.ts`:
- Around line 69-91: `DocgenWorkerClient.ready` can stay pending if the worker
fails before sending the `init` message, because `fail()` currently only affects
the worker state and not the initialization promise. Update `DocgenWorkerClient`
so pre-init failures from the worker `'error'` and `'exit'` handlers also reject
`this.ready`, and ensure the same rejection path is used for any later failure
before init completes. Keep the init handshake in the `onMessage` handler, but
make `fail()` the single place that settles both worker failure and `ready`
rejection so `extract()` cannot hang.
In `@code/core/src/types/modules/core-common.ts`:
- Around line 808-811: The public doc comment for DocgenProviderDescriptor is
inaccurate because it mentions a nonexistent config field. Update the
description in the core-common types comment so it only refers to
moduleSpecifier and aligns with the actual contract in docgen/types.ts, keeping
the wording consistent for preset authors and the docgen worker flow.
---
Nitpick comments:
In `@code/core/src/builder-manager/utils/template.ts`:
- Around line 68-70: The inline comment in template.ts documents the wrong
default lifecycle event for the docgen gate. Update the wording around
DOCGEN_STORY_PREPARED to say the hook defaults to STORY_FINISHED instead of
STORY_RENDERED, keeping the rest of the explanation about opting into
STORYBOOK_DOCGEN_STORY_PREPARED and useStoryDocgenGateReady intact.
In `@code/core/src/controls/components/ControlsPanel.stories.tsx`:
- Around line 355-376: The story ServiceDocgenLoadsImmediatelyInBuild only
verifies that serviceGetDocgen is called, so it can miss cases where
ControlsPanel still renders a loading state. Update the play function to also
assert the rendered UI after the docgen request, using the existing
ControlsPanel/story setup and the example control id, so the story confirms the
build-mode panel actually displays the loaded docgen content instead of just
subscribing.
In
`@code/core/src/shared/open-service/services/docgen/worker/docgen-worker-client.test.ts`:
- Around line 19-46: The test setup is using custom mock factories for
node:worker_threads, node:fs, ../../../../utils/module.ts, and
storybook/internal/node-logger instead of the repo’s Vitest spy-mocking pattern.
Update the docgen-worker-client.test.ts mocks to use vi.mock(..., { spy: true })
for package/file imports, and move any behavior overrides currently done in the
test body (such as the mock implementation around the importMetaResolve/worker
behavior) into a beforeEach block. Keep the changes aligned with the existing
FakeWorkerImpl, fakeWorkers, and docgen-worker-client test helpers so the test
still controls worker behavior through spies.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 271b2dbd-0a4b-41be-bc0e-2bd0bcee6714
📒 Files selected for processing (20)
code/addons/docs/src/docgen.tscode/addons/docs/src/preset.tscode/core/build-config.tscode/core/package.jsoncode/core/src/builder-manager/utils/template.tscode/core/src/controls/components/ControlsPanel.stories.tsxcode/core/src/controls/components/ControlsPanel.tsxcode/core/src/controls/typings.d.tscode/core/src/core-server/presets/common-preset.tscode/core/src/shared/open-service/services/docgen/types.tscode/core/src/shared/open-service/services/docgen/worker/docgen-worker-client.test.tscode/core/src/shared/open-service/services/docgen/worker/docgen-worker-client.tscode/core/src/shared/open-service/services/docgen/worker/docgen-worker.tscode/core/src/shared/open-service/services/docgen/worker/protocol.tscode/core/src/types/modules/core-common.tscode/renderers/react/build-config.tscode/renderers/react/package.jsoncode/renderers/react/src/componentManifest/componentMetaManagerSingleton.tscode/renderers/react/src/docgen/docgen-worker.tscode/renderers/react/src/docgen/preset.ts
💤 Files with no reviewable changes (2)
- code/addons/docs/src/docgen.ts
- code/addons/docs/src/preset.ts
- Key the dev docgen gate by storyId (derived during render) so switching stories never renders one frame with a stale ready=true that mounts the query before the new story emits its gate event. - Treat non-development as prepared for the Controls loading decision so static builds don't keep the panel loading once precomputed docgen arrives. - Reject the worker client's `ready` promise on fatal failure so an extract awaiting init fails fast instead of hanging when the worker dies during boot. - Harden the gate story tests to anchor on a stable render signal before asserting docgen was not queried, and fix a stale doc comment. Co-authored-by: Cursor <cursoragent@cursor.com>
ndelangen
reviewed
Jun 30, 2026
Comment on lines
+233
to
+243
| useChannel( | ||
| { | ||
| [gateEvent]: (payload: { id?: StoryId; storyId?: StoryId }) => { | ||
| const eventStoryId = requestAtStoryPrepared ? payload?.id : payload?.storyId; | ||
| if (eventStoryId === storyId) { | ||
| setReadyStoryId(storyId); | ||
| } | ||
| }, | ||
| }, | ||
| [storyId, requestAtStoryPrepared, gateEvent] | ||
| ); |
Member
There was a problem hiding this comment.
@JReinhold, I'm curious: is this the preferred approach?
Wouldn't we wish to encapsulate this state into the OSA?
Contributor
Author
There was a problem hiding this comment.
I think it's debatable. Currently this is "the consumer decides when they want docgen, and the service just gives it to them".
I'm not sure how we would move this into the service. Would the Controls UI just request docgen immediately, but the service would hold off generating it until it saw a matching STORY_FINISHED event from the same story? That seems weird to me. Especially when you factor in multiple manager windows.
ndelangen
reviewed
Jun 30, 2026
ndelangen
reviewed
Jun 30, 2026
Comment on lines
+17
to
+24
| export const experimental_docgenProvider = async ( | ||
| existing: DocgenProviderDescriptor[] = [] | ||
| ): Promise<DocgenProviderDescriptor[]> => [ | ||
| ...existing, | ||
| { | ||
| moduleSpecifier: fileURLToPath(import.meta.resolve('@storybook/react/internal/docgen-worker')), | ||
| }, | ||
| ]; |
Member
There was a problem hiding this comment.
I think this method has merit, but I would have kept it as is and called loadStorybook in the worker, resolving the value there.
ndelangen
reviewed
Jun 30, 2026
Comment on lines
+124
to
+141
| private handleMessage(msg: DocgenWorkerResponse): void { | ||
| if (msg.type !== 'extract') { | ||
| return; | ||
| } | ||
| const pending = this.pending.get(msg.id); | ||
| if (!pending) { | ||
| return; | ||
| } | ||
| if (pending.timer) { | ||
| clearTimeout(pending.timer); | ||
| } | ||
| this.pending.delete(msg.id); | ||
| if (msg.error) { | ||
| pending.reject(errorLikeToError(msg.error)); | ||
| } else { | ||
| pending.resolve(msg.payload); | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
For the vitest-addon, we had a similar challenge: a child process that needed to keep up to date with what was happening in the main dev process.
It would be amazing if, at some point, the answer to that is "OSA", and its state-syncing can cross even the inter-process communication layer.
Contributor
Author
There was a problem hiding this comment.
A Worker-transport on The Channel™ doesn't sound unrealistic.
ndelangen
approved these changes
Jun 30, 2026
2 tasks done
This was referenced Jul 1, 2026
This was referenced Jul 10, 2026
This was referenced Jul 14, 2026
Merged
Open
This was referenced Jul 23, 2026
This was referenced Jul 30, 2026
Merged
4 of 9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #
What I did
Moves
experimentalDocgenServer's React docgen extraction off the main thread into a single long-livedworker_threadsworker owned by core, and defers the Controls panel's docgen request until a story has reached a safe point in its lifecycle. Together this keeps the CPU-bound TypeScript program build from contending with Vite's bundling and the preview's first render. The legacy manifest/docgen path (flag off) is untouched.Key pieces (one commit each, in dependency order):
DocgenProviderDescriptors (a module specifier); the worker imports and composes them middleware-style. Results propagate back asErrorLikesuccess/failure unions. There is no in-process fallback: if the compiled worker script is missing, docgen registration is skipped.react-component-metaextraction inside the worker with its ownComponentMetaManager.STORY_FINISHED(orSTORY_PREPAREDvia the newSTORYBOOK_DOCGEN_STORY_PREPAREDenv var); static builds fetch precomputed docgen immediately. Also fixes a "No controls for this story" flash before docgen resolves.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
These steps are for a maintainer verifying the worker-based docgen end to end. Overall behavior to confirm: Controls are populated from server-extracted docgen (no build-time
__docgenInfoin the bundle), they appear shortly after the story renders, and the panel never flashes "No controls for this story".1. Dev server — worker path
Generate a sandbox:
Enable the flag in the generated sandbox's
.storybook/main.ts:Start it (run inside the generated sandbox directory):
/?path=/story/example-button--primary).primary,size,backgroundColor,label,onClick) with types/descriptions derived from the component's TypeScript types — sourced from the docgen service, not from__docgenInfobaked into the preview bundle.2. No "No controls" flash on revisit
3.
STORYBOOK_DOCGEN_STORY_PREPAREDtoggleSTORY_PREPAREDinstead ofSTORY_FINISHED.4. Static build — precomputed docgen
yarn build-storybook && npx http-server storybook-static -p 80805. Regression — flag off (legacy path)
features.experimentalDocgenServerback tofalse(or remove it), restart, and confirm Controls still work exactly as before via the legacy__docgenInfopath.Areas most likely to regress / worth extra scrutiny:
import.meta.resolve→fileURLToPath/pathToFileURL); please verify both dev and static build on Windows specifically.Once CI publishes Chromatic, the internal Controls-panel stories can be inspected at:
https://docgen-worker-threading--635781f3500dd2c49e189caf.chromatic.com/(links only resolve after CI finishes).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.tsDeclare whether manual QA will be needed for this PR during the next release, through
qa:neededorqa:skipMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit