Repository navigation
Angular: share one component-meta analyzer between docgen and story-docs - #35841
Conversation
story-docs built its own AngularComponentMetaManager alongside docgen's, so the dev server ran two warm TypeScript language services for the same files and story-docs's copy never called startWatching(), so it went stale after edits while docgen's stayed live. Extends the existing docgen worker protocol with a query message so a DocgenWorkerModule can expose queryComponentMeta alongside createDocgenProvider. angular-vite's worker-entry module now shares one module-scoped manager between both exports, and story-docs-preset.ts proxies through options.docgenWorker instead of constructing its own analyzer.
|
|
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 (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe docgen worker now supports component metadata queries. Angular Story Docs receives the shared worker, uses asynchronous metadata lookup, and reuses the worker’s analyzer manager. ChangesDocgen and Story Docs integration
Sequence Diagram(s)sequenceDiagram
participant CommonPreset
participant StoryDocsProvider
participant DocgenWorkerClient
participant AngularDocgenWorker
participant AngularAnalyzer
CommonPreset->>DocgenWorkerClient: create docgen worker
CommonPreset->>StoryDocsProvider: pass docgenWorker in options
StoryDocsProvider->>DocgenWorkerClient: query component metadata
DocgenWorkerClient->>AngularDocgenWorker: forward query request
AngularDocgenWorker->>AngularAnalyzer: extract component metadata
AngularAnalyzer-->>AngularDocgenWorker: metadata result
AngularDocgenWorker-->>DocgenWorkerClient: query response
DocgenWorkerClient-->>StoryDocsProvider: metadata result
StoryDocsProvider-->>CommonPreset: asynchronous Story Docs payload
Possibly related PRs
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
code/frameworks/angular-vite/src/docgen/docgen-worker.test.ts (1)
133-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep Vitest mock behavior in setup hooks.
The changed tests configure mock behavior inside test cases. Move each setup into an applicable
beforeEachblock.
code/frameworks/angular-vite/src/docgen/docgen-worker.test.ts#L133-L133: move thebuildDocgenPayloadreturn setup into test setup.code/frameworks/angular-vite/src/docgen/docgen-worker.test.ts#L213-L213: move the new test'sbuildDocgenPayloadsetup into test setup.code/frameworks/angular-vite/src/docgen/story-docs-build.test.ts#L86-L90: configure the rejecting query-source mock in setup.As per coding guidelines, Vitest mock behaviors must be implemented in
beforeEachblocks, and inline mock implementations must not be used in test cases.🤖 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/frameworks/angular-vite/src/docgen/docgen-worker.test.ts` at line 133, Move the Vitest mock behavior into applicable beforeEach hooks: in code/frameworks/angular-vite/src/docgen/docgen-worker.test.ts at lines 133-133 and 213-213, configure buildDocgenPayload’s return values during test setup; in code/frameworks/angular-vite/src/docgen/story-docs-build.test.ts at lines 86-90, configure the rejecting query-source mock in setup. Remove inline mock implementations from the test cases while preserving each test’s behavior.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.
Nitpick comments:
In `@code/frameworks/angular-vite/src/docgen/docgen-worker.test.ts`:
- Line 133: Move the Vitest mock behavior into applicable beforeEach hooks: in
code/frameworks/angular-vite/src/docgen/docgen-worker.test.ts at lines 133-133
and 213-213, configure buildDocgenPayload’s return values during test setup; in
code/frameworks/angular-vite/src/docgen/story-docs-build.test.ts at lines 86-90,
configure the rejecting query-source mock in setup. Remove inline mock
implementations from the test cases while preserving each test’s behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a179f406-9ba0-43a0-898e-0d506d1f983e
📒 Files selected for processing (14)
code/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.tscode/core/src/shared/open-service/services/docgen/worker/docgen-worker.tscode/core/src/shared/open-service/services/docgen/worker/protocol.tscode/core/src/shared/open-service/services/story-docs/types.tscode/core/src/types/modules/core-common.tscode/frameworks/angular-vite/src/docgen/docgen-worker.test.tscode/frameworks/angular-vite/src/docgen/docgen-worker.tscode/frameworks/angular-vite/src/docgen/story-docs-build.test.tscode/frameworks/angular-vite/src/docgen/story-docs-build.tscode/frameworks/angular-vite/src/docgen/story-docs-preset.tscode/lib/docgen-harness/src/angular/angular-story-docs-snippets.test.tscode/lib/docgen-harness/src/angular/story-docs/build-story-docs.test.ts
Co-authored-by: Valentin Palkovic <dev@valentinpalkovic.dev>
Closes #
What I did
Follow-up on #35807.
story-docs-preset.tsbuilt its ownAngularComponentMetaManagerright next to the onedocgen-worker.tsalready owns, so the dev server ends up running two warm TypeScript language services over the same files, per default, wheneverexperimentalDocgenServeris on.The docgen worker already runs one persistent
node:worker_threadsWorkerand imports eachDocgenProviderDescriptor's module bymoduleSpecifierinto it. Node caches ES module evaluation per resolved URL within a thread, so colocating both exports in the same worker-entry module gets them the same manager instance for free. No second worker, no new descriptor system, just a third message kind on a protocol that's already there:story-docs-preset.tsnow proxies throughoptions.docgenWorker.query(...)instead of constructing its own analyzer:options.docgenWorkeris threaded in fromcommon-preset.ts, which now resolves the docgen descriptors and builds the worker client before composingexperimental_storyDocsProvider, passing the client throughpresets.apply'sargsparameter (merged into the callee'soptions):One wrinkle worth flagging: since
buildStoryDocsPayload's manager call now crosses apostMessageboundary,AngularComponentMetaQuerySource.extractComponentMetareturns aPromise(a realAngularComponentMetaManagerstill resolves synchronously, and awaiting a non-Promise value is a no-op), sobuildStoryDocsPayloaditself is nowasync. I would argue that's a fair trade for one shared, watched analyzer instead of two.One manager, one
startWatching(), onerecycleIfHeapPressured(). Docgen and story-docs now invalidate together instead of story-docs going stale on its own after edits, and the dev server holds one warm TypeScript program for Angular analysis instead of two.What a bad run looks like
Reverting
docgen-worker.ts's module-scopedmanagerPromiseback to a per-call local variable (soqueryComponentMetabuilds its own manager again) breaks the new coverage indocgen-worker.test.ts:Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
yarn && yarn nx run-many -t compile.yarn task sandbox --template angular-vite/docgen-server-ts --start-from auto(the sandbox withexperimentalDocgenServerandcomponentsManifeston). Open a story's Docs page - argTypes and the Source block snippet should render exactly as they did before this PR.@Input()). Both the argTypes table and the Source block snippet should pick up the change on the next load - previously only argTypes updated live.yarn test docgen-worker story-docs-build angular-story-docs-snippets build-story-docs common-preset docgenshould all pass.Documentation
MIGRATION.MD
Internal worker plumbing behind an experimental flag, so nothing user-facing to document here.
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>