Repository navigation
Angular: Generate static template snippets on the server - #35796
Closed
valentinpalkovic wants to merge 4 commits into
Closed
valentinpalkovic wants to merge 4 commits into
valentinpalkovic wants to merge 4 commits into
Conversation
Contributor
|
This was referenced Aug 7, 2026
valentinpalkovic
marked this pull request as ready for review
August 7, 2026 11:38
valentinpalkovic
marked this pull request as draft
August 7, 2026 11:38
valentinpalkovic
added
feature request
ci:normal
valentinpalkovic
commented
Aug 7, 2026
valentinpalkovic
commented
Aug 7, 2026
valentinpalkovic
commented
Aug 7, 2026
valentinpalkovic
commented
Aug 7, 2026
valentinpalkovic
marked this pull request as ready for review
August 7, 2026 12:15
Contributor
Package BenchmarksCommit: No significant changes detected, all good. 👏 |
![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: 2
🧹 Nitpick comments (7)
code/frameworks/angular-vite/src/docgen/build-story-docs.ts (1)
61-70: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider caching the parsed CSF file per story path.
buildStoryDocsPayloadruns once per index entry. Each call reads and Babel-parses the whole story file again. A component with many stories parses the same file repeatedly on the request path. A small per-path cache keyed on the resolvedstoryPathremoves that repeated work.🤖 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/build-story-docs.ts` around lines 61 - 70, Cache the parsed CSF result used by buildStoryDocsPayload, keyed by the resolved storyPath, so repeated index entries reuse the same parse instead of rereading and reparsing the file. Update the existing story-loading flow around loadCsf and preserve its current debug logging and undefined return behavior when parsing fails.code/frameworks/angular-vite/src/docgen/compodoc-component-resolver.ts (1)
44-50: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winContain
extractArgTypesFromDatafailures like the other two failure paths.The resolver already degrades to
undefinedwhenreadMetadatathrows and whenfindCompodocEntryfinds nothing.extractArgTypesFromDataruns unguarded. If it throws on malformed Compodoc metadata, the error propagates throughbuildStoryDocsPayloadinto the provider handler instory-docs-preset.ts, which has no catch, so the story-docs request rejects instead of falling back to the next provider.♻️ Proposed guard
- const argTypes = extractArgTypesFromData(entry, { - compodocJson, - // Snippets bind inputs and outputs, which this flag never filters. - filterNonInputControls: false, - logger: options.logger, - unwrapHtml: htmlToText, - }); + let argTypes; + try { + argTypes = extractArgTypesFromData(entry, { + compodocJson, + // Snippets bind inputs and outputs, which this flag never filters. + filterNonInputControls: false, + logger: options.logger, + unwrapHtml: htmlToText, + }); + } catch (error) { + options.logger.debug( + `Could not extract Compodoc arg types for "${component.exportName}": ${error instanceof Error ? error.message : String(error)}.` + ); + return undefined; + }🤖 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/compodoc-component-resolver.ts` around lines 44 - 50, Wrap the extractArgTypesFromData call in the resolver’s existing failure-handling pattern so exceptions from malformed Compodoc metadata are caught and the resolver returns undefined, matching the readMetadata and missing-entry paths. Keep the current extraction options unchanged and ensure the error is handled locally before buildStoryDocsPayload propagates it.code/lib/docgen-harness/src/angular/angular-osa-snippets.test.ts (2)
16-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the historical precedent from this comment.
State only why these snapshots remain the baseline. The
vue3precedent does not provide a maintenance instruction.As per coding guidelines, comments must explain maintenance-relevant rationale and not investigation history.
🤖 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/lib/docgen-harness/src/angular/angular-osa-snippets.test.ts` around lines 16 - 18, Update the comment above the recorder setup to remove the reference to the vue3 precedent and historical investigation context. State only that the committed snippet-*.snapshot files remain the baseline and BASELINE_PATH is intentionally left untouched.Source: Coding guidelines
5-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftUse
memfsfor these filesystem tests.Both tests read repository fixture files through real
node:fsAPIs. This makes the tests depend on the checkout state and bypasses the required Vitest spy-mocking pattern. Mocknode:fswithspy: trueat the top of each test file. Reset and seedvolinbeforeEach. Redirect requiredvi.mocked()filesystem spies tomemfs.
code/lib/docgen-harness/src/angular/angular-osa-snippets.test.ts#L5-L7: redirectreadFileSyncandreaddirSyncto seededmemfsdata for every Angular fixture.code/frameworks/angular-vite/src/docgen/build-story-docs.test.ts#L3-L5: redirectreadFileSyncto seeded story and Compodoc fixture data.As per coding guidelines, filesystem tests using
node:fsmust usememfs, resetvolinbeforeEach, and use Vitest spy redirection rather than real filesystem access.🤖 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/lib/docgen-harness/src/angular/angular-osa-snippets.test.ts` around lines 5 - 7, Update code/lib/docgen-harness/src/angular/angular-osa-snippets.test.ts lines 5-7 and code/frameworks/angular-vite/src/docgen/build-story-docs.test.ts lines 3-5 to mock node:fs with spy: true, reset and seed memfs vol in each file’s beforeEach, and redirect the required vi.mocked() filesystem spies to the seeded fixtures. In angular-osa-snippets.test.ts redirect readFileSync and readdirSync for Angular fixtures; in build-story-docs.test.ts redirect readFileSync for story and Compodoc fixtures, eliminating real filesystem access.Source: Coding guidelines
code/lib/docgen-harness/src/angular/angular-provider-seam.test.ts (1)
35-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace this syntax-only assertion with a registration behavior test.
typeof experimental_storyDocsProvider === 'function'only verifies the direct module export. TypeScript and the import already cover that contract. Test preset resolution or provider invocation through the public registration path instead.As per coding guidelines, “Test public contracts and externally observable side effects rather than private implementation details” and “avoid syntax-only tests.”
🤖 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/lib/docgen-harness/src/angular/angular-provider-seam.test.ts` around lines 35 - 37, Replace the syntax-only assertion in the angular-vite registration test with a behavioral test through the public registration path, such as resolving the preset or invoking the registered provider. Assert the externally observable provider registration result rather than the typeof value of experimental_storyDocsProvider.Source: Coding guidelines
code/frameworks/angular-vite/src/docgen/story-docs-preset.test.ts (2)
26-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove test-double behavior into typed Vitest mocks.
Lines 30-34, 47, 69, and 78-84 define mock behavior outside
beforeEachor inline in test cases. Use shared typed spies, access them throughvi.mocked(), and configure their resolved values inbeforeEach.As per coding guidelines, “Use
vi.mocked()to type and access the mocked functions in Vitest tests,” “Implement mock behaviors inbeforeEachblocks,” and “Avoid inline mock implementations within test cases in Vitest tests.”Also applies to: 47-47, 69-69, 77-86
🤖 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/story-docs-preset.test.ts` around lines 26 - 36, Refactor the test doubles in the story-docs preset tests, including the options helper and the mocks at the referenced test cases, to use shared typed Vitest spies accessed through vi.mocked(). Move all mock implementations and resolved-value configuration into beforeEach, leaving test bodies to invoke the already-configured spies without inline behavior.Source: Coding guidelines
50-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the acceptance-criteria code from this comment.
NFR3is not maintenance rationale. Keep the explanation of the Node execution requirement without the internal code.As per coding guidelines, “Comments should explain maintenance-relevant rationale, not investigation history, internal ticket or acceptance-criteria codes, provenance claims, or cross-file line references.”
🤖 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/story-docs-preset.test.ts` around lines 50 - 51, Update the comment above the relevant test to remove the “NFR3” acceptance-criteria label and the internal assertion/provenance wording, while retaining only the maintenance-relevant explanation that the preset must be callable as a plain Node function.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/frameworks/angular-vite/src/client/docs/config.test.ts`:
- Around line 18-20: Update the `registers the source decorator by default` test
to stub the global `FEATURES` with `experimentalDocgenServer: false` via
`vi.stubGlobal` before loading `config.ts` or calling `loadDecorators()`, and
restore the ambient global state after the test using the existing Vitest
cleanup mechanism.
In
`@code/frameworks/angular-vite/src/docgen/__testfixtures__/story-docs.stories.ts`:
- Line 3: Update the ButtonComponent import in the story fixture to include the
explicit .ts extension, changing the module specifier from ./button.component to
./button.component.ts.
---
Nitpick comments:
In `@code/frameworks/angular-vite/src/docgen/build-story-docs.ts`:
- Around line 61-70: Cache the parsed CSF result used by buildStoryDocsPayload,
keyed by the resolved storyPath, so repeated index entries reuse the same parse
instead of rereading and reparsing the file. Update the existing story-loading
flow around loadCsf and preserve its current debug logging and undefined return
behavior when parsing fails.
In `@code/frameworks/angular-vite/src/docgen/compodoc-component-resolver.ts`:
- Around line 44-50: Wrap the extractArgTypesFromData call in the resolver’s
existing failure-handling pattern so exceptions from malformed Compodoc metadata
are caught and the resolver returns undefined, matching the readMetadata and
missing-entry paths. Keep the current extraction options unchanged and ensure
the error is handled locally before buildStoryDocsPayload propagates it.
In `@code/frameworks/angular-vite/src/docgen/story-docs-preset.test.ts`:
- Around line 26-36: Refactor the test doubles in the story-docs preset tests,
including the options helper and the mocks at the referenced test cases, to use
shared typed Vitest spies accessed through vi.mocked(). Move all mock
implementations and resolved-value configuration into beforeEach, leaving test
bodies to invoke the already-configured spies without inline behavior.
- Around line 50-51: Update the comment above the relevant test to remove the
“NFR3” acceptance-criteria label and the internal assertion/provenance wording,
while retaining only the maintenance-relevant explanation that the preset must
be callable as a plain Node function.
In `@code/lib/docgen-harness/src/angular/angular-osa-snippets.test.ts`:
- Around line 16-18: Update the comment above the recorder setup to remove the
reference to the vue3 precedent and historical investigation context. State only
that the committed snippet-*.snapshot files remain the baseline and
BASELINE_PATH is intentionally left untouched.
- Around line 5-7: Update
code/lib/docgen-harness/src/angular/angular-osa-snippets.test.ts lines 5-7 and
code/frameworks/angular-vite/src/docgen/build-story-docs.test.ts lines 3-5 to
mock node:fs with spy: true, reset and seed memfs vol in each file’s beforeEach,
and redirect the required vi.mocked() filesystem spies to the seeded fixtures.
In angular-osa-snippets.test.ts redirect readFileSync and readdirSync for
Angular fixtures; in build-story-docs.test.ts redirect readFileSync for story
and Compodoc fixtures, eliminating real filesystem access.
In `@code/lib/docgen-harness/src/angular/angular-provider-seam.test.ts`:
- Around line 35-37: Replace the syntax-only assertion in the angular-vite
registration test with a behavioral test through the public registration path,
such as resolving the preset or invoking the registered provider. Assert the
externally observable provider registration result rather than the typeof value
of experimental_storyDocsProvider.
🪄 Autofix
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: 9e63efcc-fd3f-474d-a656-abeccd20aa25
📒 Files selected for processing (37)
code/frameworks/angular-vite/src/client/docs/config.test.tscode/frameworks/angular-vite/src/client/docs/config.tscode/frameworks/angular-vite/src/docgen/__testfixtures__/documentation.jsoncode/frameworks/angular-vite/src/docgen/__testfixtures__/no-component.stories.tscode/frameworks/angular-vite/src/docgen/__testfixtures__/story-docs.stories.tscode/frameworks/angular-vite/src/docgen/build-docgen.tscode/frameworks/angular-vite/src/docgen/build-story-docs.test.tscode/frameworks/angular-vite/src/docgen/build-story-docs.tscode/frameworks/angular-vite/src/docgen/compodoc-component-resolver.tscode/frameworks/angular-vite/src/docgen/docgen-worker.tscode/frameworks/angular-vite/src/docgen/documentation-json.tscode/frameworks/angular-vite/src/docgen/logger.tscode/frameworks/angular-vite/src/docgen/resolve-component.tscode/frameworks/angular-vite/src/docgen/story-docs-limitations.mdcode/frameworks/angular-vite/src/docgen/story-docs-preset.test.tscode/frameworks/angular-vite/src/docgen/story-docs-preset.tscode/frameworks/angular-vite/src/docgen/template-snippet.test.tscode/frameworks/angular-vite/src/docgen/template-snippet.tscode/frameworks/angular-vite/src/preset.tscode/lib/angular-compodoc/src/compodoc-types.tscode/lib/docgen-harness/src/angular/__testfixtures__/complex-selector/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/cross-file-inheritance/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/decorator-generic/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/decorator-getter-setter/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/decorator-io-basics/osa-snippet-EventHandlerArg.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/decorator-io-basics/osa-snippet-ExplicitUndefinedArg.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/decorator-io-basics/osa-snippet-ObjectAndArrayArgs.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/decorator-io-basics/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/decorator-union-enum/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/expression-defaults/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/jsdoc-tags/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/signal-io/osa-snippet-EventHandlerArg.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/signal-io/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/signal-model/osa-snippet-TwoWayBinding.snapshotcode/lib/docgen-harness/src/angular/angular-osa-snippets.test.tscode/lib/docgen-harness/src/angular/angular-provider-seam.test.ts
Comment on lines
+18
to
+20
| it('registers the source decorator by default', async () => { | ||
| await expect(loadDecorators()).resolves.toHaveLength(1); | ||
| }); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the default feature-flag state explicit.
At Line 18, the test imports config.ts without setting FEATURES. A Vitest setup with experimentalDocgenServer: true would make this test load an empty decorator list instead of the default list. Stub FEATURES with experimentalDocgenServer: false before loading the module.
As per coding guidelines, use vi.stubGlobal and restore ambient globals in tests.
Suggested fix
it('registers the source decorator by default', async () => {
+ vi.stubGlobal('FEATURES', { experimentalDocgenServer: false });
await expect(loadDecorators()).resolves.toHaveLength(1);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('registers the source decorator by default', async () => { | |
| await expect(loadDecorators()).resolves.toHaveLength(1); | |
| }); | |
| it('registers the source decorator by default', async () => { | |
| vi.stubGlobal('FEATURES', { experimentalDocgenServer: false }); | |
| await expect(loadDecorators()).resolves.toHaveLength(1); | |
| }); |
🤖 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/client/docs/config.test.ts` around lines 18
- 20, Update the `registers the source decorator by default` test to stub the
global `FEATURES` with `experimentalDocgenServer: false` via `vi.stubGlobal`
before loading `config.ts` or calling `loadDecorators()`, and restore the
ambient global state after the test using the existing Vitest cleanup mechanism.
Source: Coding guidelines
| @@ -0,0 +1,20 @@ | |||
| // Fixture: the story shapes the Angular story-docs provider has to tell apart. | |||
| // Excluded from the package tsconfig, so the loose typing here is deliberate. | |||
| import { ButtonComponent } from './button.component'; | |||
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -HI -t f 'tsconfig.*\.json$' . -x sh -c '
echo "--- $1"
rg -n "allowImportingTsExtensions|moduleResolution|noEmit" "$1" || true
' sh {}
rg -n -C 2 "button\.component" \
code/frameworks/angular-vite/src/docgen/__testfixtures__/story-docs.stories.tsRepository: storybookjs/storybook
Length of output: 27693
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- relevant fixture files'
fd -HI -t f . code/frameworks/angular-vite/src/docgen/__testfixtures__ | sort | sed -n '1,120p'
echo '--- fixture tsconfig files'
for f in code/frameworks/angular-vite/src/docgen/__testfixtures__/tsconfig.json \
code/frameworks/angular-vite/tsconfig.json \
code/tsconfig.json; do
if [ -f "$f" ]; then
echo "--- $f"
cat -n "$f"
fi
done
echo '--- package and workspace configuration references'
rg -n -C 3 \
'story-docs\.stories|__testfixtures__|angular-vite/src/docgen|docgen.*fixture|allowImportingTsExtensions|verbatimModuleSyntax' \
code/frameworks/angular-vite code/package.json package.json .storybook 2>/dev/null | sed -n '1,240p'
echo '--- import-extension patterns in neighboring TypeScript fixtures'
rg -n \
"^import .* from ['\"]\./[^'\"]+['\"];" \
code/frameworks/angular-vite/src/docgen/__testfixtures__ \
code/lib/docgen-harness/src/angular/__testfixtures__ 2>/dev/null | sed -n '1,160p'
echo '--- referenced component path'
python3 - <<'PY'
from pathlib import Path
p = Path("code/frameworks/angular-vite/src/docgen/__testfixtures__/story-docs.stories.ts")
for spec in ["./button.component", "./button.component.ts"]:
target = p.parent / spec.removeprefix("./")
print(f"{spec}: {target} exists={target.exists()} is_file={target.is_file()}")
PYRepository: storybookjs/storybook
Length of output: 29244
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- fixture source files'
for f in \
code/frameworks/angular-vite/src/docgen/__testfixtures__/story-docs.stories.ts \
code/frameworks/angular-vite/src/docgen/__testfixtures__/button.stories.ts \
code/frameworks/angular-vite/src/docgen/__testfixtures__/no-component.stories.ts \
code/frameworks/angular-vite/src/docgen/build-story-docs.test.ts \
code/frameworks/angular-vite/src/docgen/build-story-docs.ts; do
if [ -f "$f" ]; then
echo "--- $f"
cat -n "$f" | sed -n '1,240p'
fi
done
echo '--- implementation and loader references'
rg -n -C 4 \
'loadCsf|jiti|tsx|typescript|resolve.*import|import.*path|storyPath|readFileSync|createRequire' \
code/frameworks/angular-vite/src/docgen code/lib 2>/dev/null | \
sed -n '1,320p'
echo '--- extensionless relative imports in the target fixture subtree'
rg -n \
"^import .* from ['\"]\./[^'\"]+['\"];" \
code/frameworks/angular-vite/src/docgen/__testfixtures__ || true
echo '--- extension-bearing relative imports in nearby Angular docgen tests'
rg -n \
"^import .* from ['\"]\./[^'\"]+\.(ts|tsx|js|jsx)['\"];" \
code/frameworks/angular-vite/src/docgen | sed -n '1,160p'Repository: storybookjs/storybook
Length of output: 46008
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- Angular component resolver'
cat -n code/frameworks/angular-vite/src/docgen/resolve-component.ts | sed -n '1,180p'
echo '--- resolver implementation and extension handling'
rg -n -C 5 \
'createMetaComponentResolver|importId|extensions:|tsconfig: .auto.|resolve.*extension|\.component' \
code/core code/lib code/frameworks 2>/dev/null | \
sed -n '1,360p'
echo '--- exact resolver test expectations'
cat -n code/frameworks/angular-vite/src/docgen/resolve-component.test.ts | sed -n '115,145p'
echo '--- TypeScript extension policy configuration'
rg -n -C 4 \
'explicit.*extension|import.*extension|allowImportingTsExtensions|noRelative.*Extension|extension' \
code/.eslintrc* code/eslint* .eslintrc* eslint.config.* package.json code/package.json 2>/dev/null | \
sed -n '1,240p'Repository: storybookjs/storybook
Length of output: 37439
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- all tracked and installed definitions of createMetaComponentResolver'
rg -uu -n -C 8 \
'createMetaComponentResolver' \
code node_modules 2>/dev/null | sed -n '1,360p'
echo '--- resolver-related module resolution helpers'
rg -uu -n -C 6 \
'resolveImport|resolve.*module|extensions.*js|extensions.*ts|\.tsx.*\.jsx|tsconfig.*auto' \
code/core code/lib node_modules/storybook 2>/dev/null | sed -n '1,360p'Repository: storybookjs/storybook
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
log=/tmp/coderabbit-shell-logs/shell-output-0vCwg3
echo '--- relevant match locations'
rg -n -m 30 'createMetaComponentResolver|resolveImport|extensions.*(js|ts)|tsconfig.*auto' "$log" || trueRepository: storybookjs/storybook
Length of output: 3741
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n code/core/src/common/utils/resolve-meta-component.ts | sed -n '1,150p'
cat -n code/core/src/shared/constants/extensions.ts | sed -n '1,100p'Repository: storybookjs/storybook
Length of output: 7318
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n code/core/src/common/utils/module-resolver.ts | sed -n '1,220p'Repository: storybookjs/storybook
Length of output: 1939
Add the .ts extension to the fixture import. Change ./button.component to ./button.component.ts.
🤖 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/__testfixtures__/story-docs.stories.ts`
at line 3, Update the ButtonComponent import in the story fixture to include the
explicit .ts extension, changing the module specifier from ./button.component to
./button.component.ts.
Source: Coding guidelines
valentinpalkovic
force-pushed
the
valentin/angular-story-docs-1-template-snippets
branch
from
August 7, 2026 16:43
7901b02 to
79d7bff
Compare
React's snippet generator grew several helpers that are not React-specific:
resolving a `render` identifier to the function it names, telling an absent
`render` apart from an unreadable one, and reading CSF2's `Story.args = {}`
assignment form. The Angular and Vue providers need the same answers.
They move into `csf-tools/story-shape` and React imports them back, so its
snapshots are what proves the move is behaviour-preserving. `propertyValue`
and `returnedObjectExpression` join them for the providers stacked on top.
1 of 9 tasks
An Angular story's Source block is filled in by the browser today: Storybook loads the component class, reads its inputs and outputs through Angular's reflection APIs, and builds the template in the preview. That needs a compiler, so nothing outside the browser can produce a snippet. Behind `experimentalDocgenServer`, this builds the same snippet in Node from the component's declared selector and bindings plus the args written in the story file. Property and event bindings only. With the flag off nothing changes. Metadata reaches the generator through a resolver rather than being read from Compodoc directly, so the engine behind it can be replaced without touching snippet generation.
Co-authored-by: Valentin Palkovic <dev@valentinpalkovic.dev>
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)
WalkthroughAdded Angular server-side story documentation generation, Compodoc component resolution, static Angular snippet rendering, provider registration, and feature-flagged client decorator selection. Added fixtures, unit tests, OSA snapshots, provider seam coverage, and limitations documentation. ChangesAngular static snippet generation
Angular story documentation
Story-docs provider integration
Sequence Diagram(s)sequenceDiagram
participant StoryDocsRequest
participant experimental_storyDocsProvider
participant buildStoryDocsPayload
participant createCompodocComponentResolver
participant documentationJsonReader
StoryDocsRequest->>experimental_storyDocsProvider: request story documentation
experimental_storyDocsProvider->>buildStoryDocsPayload: build matching story payload
buildStoryDocsPayload->>createCompodocComponentResolver: resolve Angular component
createCompodocComponentResolver->>documentationJsonReader: read Compodoc metadata
documentationJsonReader-->>createCompodocComponentResolver: return CompodocJson
createCompodocComponentResolver-->>buildStoryDocsPayload: return component metadata
buildStoryDocsPayload-->>experimental_storyDocsProvider: return generated payload
experimental_storyDocsProvider-->>StoryDocsRequest: merge or delegate payload
Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
code/frameworks/angular-vite/src/client/docs/config.test.ts (1)
18-20: 🎯 Functional Correctness | 🟡 MinorMake the default feature-flag state explicit.
If the Vitest setup defines
globalThis.FEATURES.experimentalDocgenServerastrue, this test expects one decorator but receives none. StubFEATURESwith{ experimentalDocgenServer: false }before callingloadDecorators().Suggested fix
it('registers the source decorator by default', async () => { + vi.stubGlobal('FEATURES', { experimentalDocgenServer: false }); await expect(loadDecorators()).resolves.toHaveLength(1); });As per coding guidelines, use
vi.stubGlobalfor ambient globals and restore them withvi.unstubAllGlobals().🤖 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/client/docs/config.test.ts` around lines 18 - 20, Update the “registers the source decorator by default” test to stub the ambient FEATURES global with experimentalDocgenServer set to false before calling loadDecorators(), and restore the stub with vi.unstubAllGlobals() after the test.Source: Coding guidelines
🧹 Nitpick comments (3)
code/frameworks/angular-vite/src/docgen/story-docs-preset.test.ts (1)
94-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConfigure the
process.cwdbehavior inbeforeEach.Line 94 changes mock behavior inside the test case. Keep the mock implementation in
beforeEach, then update a shared test value in this case.Proposed fix
const FIXTURES = join(dirname(fileURLToPath(import.meta.url)), '__testfixtures__'); +let currentWorkingDirectory = FIXTURES; // `storyRoot` and `workspaceRoot` differ here on purpose. beforeEach(() => { - vi.spyOn(process, 'cwd').mockReturnValue(FIXTURES); + currentWorkingDirectory = FIXTURES; + vi.spyOn(process, 'cwd').mockImplementation(() => currentWorkingDirectory); }); @@ it('falls through when the story path resolves against cwd but Compodoc lives elsewhere', async () => { - vi.mocked(process.cwd).mockReturnValue(join(FIXTURES, 'aliased')); + currentWorkingDirectory = join(FIXTURES, 'aliased'); const provider = await experimental_storyDocsProvider(noDownstream, options());🤖 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/story-docs-preset.test.ts` at line 94, Move the vi.mocked(process.cwd) configuration into the test suite’s beforeEach setup, and in the affected test update the shared fixture value instead of changing the mock behavior there. Preserve the test’s aliased fixture behavior through that shared value.Source: Coding guidelines
code/lib/docgen-harness/src/angular/angular-osa-snippets.test.ts (1)
5-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftUse
memfsfor filesystem access in these tests.Both tests read checked-out fixture files through
node:fs. Seed the required files involduringbeforeEach, resetvolbetween cases, and redirect filesystem calls with Vitest spies.
code/lib/docgen-harness/src/angular/angular-osa-snippets.test.ts#L5-L7: Seed story, Compodoc, baseline, and OSA snapshot files inmemfs. Read expected snapshots through the redirected filesystem path.code/frameworks/angular-vite/src/docgen/build-story-docs.test.ts#L3-L5: Seeddocumentation.jsonand story fixtures inmemfsbefore each test.As per coding guidelines, “For filesystem tests involving
node:fsornode:fs/promises, usememfs; resetvolinbeforeEach[and] seed virtual files.”🤖 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/lib/docgen-harness/src/angular/angular-osa-snippets.test.ts` around lines 5 - 7, Replace direct fixture filesystem access in code/lib/docgen-harness/src/angular/angular-osa-snippets.test.ts (lines 5-7) with memfs: reset vol in beforeEach, seed the story, Compodoc, baseline, and OSA snapshot files, and redirect filesystem calls with Vitest spies so expected snapshots are read virtually. Apply the same setup in code/frameworks/angular-vite/src/docgen/build-story-docs.test.ts (lines 3-5), resetting vol and seeding documentation.json and story fixtures before each test.Source: Coding guidelines
code/lib/docgen-harness/src/angular/angular-provider-seam.test.ts (1)
33-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove or replace this syntax-only test.
typeof experimental_storyDocsProvider === 'function'only repeats module-export validation. It does not test preset registration or provider behavior. Test the provider through its public preset contract, or rely onstory-docs-preset.test.tsfor provider behavior.As per coding guidelines, “Test public contracts and externally observable side effects rather than private implementation details” and “test real behavior and avoid syntax-only tests.”
🤖 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/lib/docgen-harness/src/angular/angular-provider-seam.test.ts` around lines 33 - 37, Remove the syntax-only test for experimental_storyDocsProvider in the angular-vite registration suite. Rely on story-docs-preset.test.ts for provider behavior, or replace this assertion with coverage of the public preset contract and an externally observable registration/provider effect.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/csf-tools/story-shape/args.ts`:
- Around line 55-74: Update the assignment search around the program traversal
to inspect only direct Program.body expression statements, excluding assignments
inside callbacks, functions, or other nested scopes. Preserve matching for
module-level storyName.args assignments, and add a regression test covering a
nested or shadowed A.args assignment so unrelated assignments cannot set found.
In `@code/frameworks/angular-vite/src/docgen/story-docs-preset.test.ts`:
- Around line 50-51: Remove the internal “NFR3” acceptance-criteria identifier
from the comment while preserving its explanation that the test is callable as a
plain Node function and validates the actual snippet output.
In `@code/lib/docgen-harness/src/angular/angular-osa-snippets.test.ts`:
- Around line 16-18: Update the comment above the recorder setup to remove the
“following the vue3 `cm-` precedent” provenance reference, while retaining the
explanation that committed `snippet-*.snapshot` files remain the measurement
oracle and `BASELINE_PATH` is intentionally untouched.
---
Duplicate comments:
In `@code/frameworks/angular-vite/src/client/docs/config.test.ts`:
- Around line 18-20: Update the “registers the source decorator by default” test
to stub the ambient FEATURES global with experimentalDocgenServer set to false
before calling loadDecorators(), and restore the stub with vi.unstubAllGlobals()
after the test.
---
Nitpick comments:
In `@code/frameworks/angular-vite/src/docgen/story-docs-preset.test.ts`:
- Line 94: Move the vi.mocked(process.cwd) configuration into the test suite’s
beforeEach setup, and in the affected test update the shared fixture value
instead of changing the mock behavior there. Preserve the test’s aliased fixture
behavior through that shared value.
In `@code/lib/docgen-harness/src/angular/angular-osa-snippets.test.ts`:
- Around line 5-7: Replace direct fixture filesystem access in
code/lib/docgen-harness/src/angular/angular-osa-snippets.test.ts (lines 5-7)
with memfs: reset vol in beforeEach, seed the story, Compodoc, baseline, and OSA
snapshot files, and redirect filesystem calls with Vitest spies so expected
snapshots are read virtually. Apply the same setup in
code/frameworks/angular-vite/src/docgen/build-story-docs.test.ts (lines 3-5),
resetting vol and seeding documentation.json and story fixtures before each
test.
In `@code/lib/docgen-harness/src/angular/angular-provider-seam.test.ts`:
- Around line 33-37: Remove the syntax-only test for
experimental_storyDocsProvider in the angular-vite registration suite. Rely on
story-docs-preset.test.ts for provider behavior, or replace this assertion with
coverage of the public preset contract and an externally observable
registration/provider effect.
🪄 Autofix
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: 8e2f9c5f-7654-4eb4-bc33-095ef76c96e9
📒 Files selected for processing (47)
code/core/src/core-server/utils/manifests/components-ref-manifest.test.tscode/core/src/csf-tools/story-shape/args.test.tscode/core/src/csf-tools/story-shape/args.tscode/core/src/csf-tools/story-shape/index.tscode/core/src/csf-tools/story-shape/render.test.tscode/core/src/csf-tools/story-shape/render.tscode/core/src/csf-tools/story-shape/utils.tscode/core/src/shared/open-service/services/story-docs/definition.tscode/core/src/shared/open-service/services/story-docs/types.tscode/frameworks/angular-vite/src/client/docs/config.test.tscode/frameworks/angular-vite/src/client/docs/config.tscode/frameworks/angular-vite/src/docgen/__testfixtures__/documentation.jsoncode/frameworks/angular-vite/src/docgen/__testfixtures__/no-component.stories.tscode/frameworks/angular-vite/src/docgen/__testfixtures__/story-docs.stories.tscode/frameworks/angular-vite/src/docgen/build-docgen.tscode/frameworks/angular-vite/src/docgen/build-story-docs.test.tscode/frameworks/angular-vite/src/docgen/build-story-docs.tscode/frameworks/angular-vite/src/docgen/compodoc-component-resolver.tscode/frameworks/angular-vite/src/docgen/docgen-worker.tscode/frameworks/angular-vite/src/docgen/documentation-json.tscode/frameworks/angular-vite/src/docgen/logger.tscode/frameworks/angular-vite/src/docgen/resolve-component.tscode/frameworks/angular-vite/src/docgen/story-docs-limitations.mdcode/frameworks/angular-vite/src/docgen/story-docs-preset.test.tscode/frameworks/angular-vite/src/docgen/story-docs-preset.tscode/frameworks/angular-vite/src/docgen/template-snippet.test.tscode/frameworks/angular-vite/src/docgen/template-snippet.tscode/frameworks/angular-vite/src/preset.tscode/lib/angular-compodoc/src/compodoc-types.tscode/lib/docgen-harness/src/angular/__testfixtures__/complex-selector/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/cross-file-inheritance/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/decorator-generic/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/decorator-getter-setter/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/decorator-io-basics/osa-snippet-EventHandlerArg.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/decorator-io-basics/osa-snippet-ExplicitUndefinedArg.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/decorator-io-basics/osa-snippet-ObjectAndArrayArgs.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/decorator-io-basics/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/decorator-union-enum/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/expression-defaults/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/jsdoc-tags/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/signal-io/osa-snippet-EventHandlerArg.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/signal-io/osa-snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/signal-model/osa-snippet-TwoWayBinding.snapshotcode/lib/docgen-harness/src/angular/angular-osa-snippets.test.tscode/lib/docgen-harness/src/angular/angular-provider-seam.test.tscode/renderers/react/src/componentManifest/generateCodeSnippet.ts
Comment on lines
+55
to
+74
| program.traverse({ | ||
| AssignmentExpression(assignment) { | ||
| const left = assignment.get('left'); | ||
| const right = assignment.get('right'); | ||
| if (!left.isMemberExpression() || !right.isObjectExpression()) { | ||
| return; | ||
| } | ||
|
|
||
| const object = left.get('object'); | ||
| const property = left.get('property'); | ||
| const isStory = object.isIdentifier() && object.node.name === storyName; | ||
| const isArgs = | ||
| (property.isIdentifier() && property.node.name === 'args' && !left.node.computed) || | ||
| (t.isStringLiteral(property.node) && left.node.computed && property.node.value === 'args'); | ||
|
|
||
| if (isStory && isArgs) { | ||
| found = right; | ||
| } | ||
| }, | ||
| }); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Restrict args assignments to module-level statements.
Line 55 traverses nested scopes. A callback parameter named A can assign A.args, and this function will use that unrelated assignment for story A.
Scan only direct Program.body expression statements. Add a regression test for a nested or shadowed A.args assignment. This prevents incorrect React and static-doc snippets.
Proposed fix
- program.traverse({
- AssignmentExpression(assignment) {
+ for (const statement of program.get('body')) {
+ if (!statement.isExpressionStatement()) {
+ continue;
+ }
+ const assignment = statement.get('expression');
+ if (!assignment.isAssignmentExpression()) {
+ continue;
+ }
const left = assignment.get('left');
const right = assignment.get('right');
if (!left.isMemberExpression() || !right.isObjectExpression()) {
- return;
+ continue;
}
// existing member checks
if (isStory && isArgs) {
found = right;
}
- },
- });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| program.traverse({ | |
| AssignmentExpression(assignment) { | |
| const left = assignment.get('left'); | |
| const right = assignment.get('right'); | |
| if (!left.isMemberExpression() || !right.isObjectExpression()) { | |
| return; | |
| } | |
| const object = left.get('object'); | |
| const property = left.get('property'); | |
| const isStory = object.isIdentifier() && object.node.name === storyName; | |
| const isArgs = | |
| (property.isIdentifier() && property.node.name === 'args' && !left.node.computed) || | |
| (t.isStringLiteral(property.node) && left.node.computed && property.node.value === 'args'); | |
| if (isStory && isArgs) { | |
| found = right; | |
| } | |
| }, | |
| }); | |
| for (const statement of program.get('body')) { | |
| if (!statement.isExpressionStatement()) { | |
| continue; | |
| } | |
| const assignment = statement.get('expression'); | |
| if (!assignment.isAssignmentExpression()) { | |
| continue; | |
| } | |
| const left = assignment.get('left'); | |
| const right = assignment.get('right'); | |
| if (!left.isMemberExpression() || !right.isObjectExpression()) { | |
| continue; | |
| } | |
| const object = left.get('object'); | |
| const property = left.get('property'); | |
| const isStory = object.isIdentifier() && object.node.name === storyName; | |
| const isArgs = | |
| (property.isIdentifier() && property.node.name === 'args' && !left.node.computed) || | |
| (t.isStringLiteral(property.node) && left.node.computed && property.node.value === 'args'); | |
| if (isStory && isArgs) { | |
| found = right; | |
| } | |
| } |
🤖 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/csf-tools/story-shape/args.ts` around lines 55 - 74, Update the
assignment search around the program traversal to inspect only direct
Program.body expression statements, excluding assignments inside callbacks,
functions, or other nested scopes. Preserve matching for module-level
storyName.args assignments, and add a regression test covering a nested or
shadowed A.args assignment so unrelated assignments cannot set found.
Comment on lines
+50
to
+51
| // NFR3: cold-callable as a plain Node function. Everything it needs comes from the preset | ||
| // options and the filesystem, and the assertion is the real snippet rather than "it returned". |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the acceptance-criteria code.
NFR3 is an internal acceptance-criteria code. Keep the test rationale without that identifier.
Proposed fix
- // NFR3: cold-callable as a plain Node function. Everything it needs comes from the preset
+ // The provider is cold-callable as a plain Node function. Everything it needs comes from the preset📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // NFR3: cold-callable as a plain Node function. Everything it needs comes from the preset | |
| // options and the filesystem, and the assertion is the real snippet rather than "it returned". | |
| // The provider is cold-callable as a plain Node function. Everything it needs comes from the preset | |
| // options and the filesystem, and the assertion is the real snippet rather than "it returned". |
🤖 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/story-docs-preset.test.ts` around
lines 50 - 51, Remove the internal “NFR3” acceptance-criteria identifier from
the comment while preserving its explanation that the test is callable as a
plain Node function and validates the actual snippet output.
Source: Coding guidelines
Comment on lines
+16
to
+18
| // A second recorder alongside the legacy one, following the vue3 `cm-` precedent: the committed | ||
| // `snippet-*.snapshot` files stay the oracle this path is measured against instead of being | ||
| // overwritten by it, so `BASELINE_PATH` is deliberately untouched. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the precedent reference from this comment.
“following the vue3 cm- precedent” is a provenance claim. Keep only the rationale for preserving the committed baseline files.
Proposed fix
-// A second recorder alongside the legacy one, following the vue3 `cm-` precedent: the committed
+// A second recorder alongside the legacy one: the committed📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // A second recorder alongside the legacy one, following the vue3 `cm-` precedent: the committed | |
| // `snippet-*.snapshot` files stay the oracle this path is measured against instead of being | |
| // overwritten by it, so `BASELINE_PATH` is deliberately untouched. | |
| // A second recorder alongside the legacy one: the committed | |
| // `snippet-*.snapshot` files stay the oracle this path is measured against instead of being | |
| // overwritten by it, so `BASELINE_PATH` is deliberately untouched. |
🤖 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/lib/docgen-harness/src/angular/angular-osa-snippets.test.ts` around
lines 16 - 18, Update the comment above the recorder setup to remove the
“following the vue3 `cm-` precedent” provenance reference, while retaining the
explanation that committed `snippet-*.snapshot` files remain the measurement
oracle and `BASELINE_PATH` is intentionally untouched.
Source: Coding guidelines
valentinpalkovic
force-pushed
the
valentin/angular-story-docs-1-template-snippets
branch
from
August 7, 2026 16:49
79d7bff to
3341953
Compare
valentinpalkovic
changed the base branch from
next
to
valentin/csf-tools-story-shape-helpers
August 7, 2026 16:49
valentinpalkovic
force-pushed
the
valentin/csf-tools-story-shape-helpers
branch
2 times, most recently
from
August 7, 2026 18:30
69eca6c to
c8b1834
Compare
Under the experimental docgen flag the snippets read the in-process analyzer, not Compodoc's documentation.json.
valentinpalkovic
force-pushed
the
valentin/csf-tools-story-shape-helpers
branch
from
August 7, 2026 19:59
c8b1834 to
7780a27
Compare
Contributor
Author
|
Closing this in favour of #35805, which already ships the same vertical through the in-process analyzer rather than Compodoc. Both branches ended up implementing server-side snippets independently. Scored against the browser snapshots the parity harness treats as truth, across the same 15 fixtures:
The two places it shows: <!-- browser -->
<sb-decorator-union-enum [size]="'large'" [tone]="'warn'" [kind]="'secondary'"></sb-decorator-union-enum>
<!-- #35805 -->
<sb-decorator-union-enum [size]="'large'" [tone]="'warn'" [kind]="'secondary'"></sb-decorator-union-enum>
<!-- this PR -->
<sb-decorator-union-enum [kind]="ButtonKind.Secondary" [size]="'large'" [tone]="'warn'"></sb-decorator-union-enum>#35805 resolves the enum member to its value and keeps declaration order; this one leaves The deciding factor is not the score, though. The resolver here reads Compodoc's Nothing is lost. The three slices stacked on top of this one are genuinely additive and are being re-pointed at #35805's |
This was referenced Aug 7, 2026
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
1 participant
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.
Note
First of four Angular story-docs slices. This one is the whole vertical: a story file goes in, a snippet comes out of the Docs Source block. Later slices add story shapes beyond plain args (#2), incompleteness reporting (#3), and a full-component snippet format (#4).
What I did
Today an Angular story's Source block is filled in by the browser: Storybook loads the component class, reads its inputs and outputs through Angular's reflection APIs, and builds the template string in the preview. That only works with a compiler present, so nothing outside the browser can produce a snippet - the CLI and MCP can't, and neither can anything that wants the docs without rendering the story.
This builds the same kind of snippet in Node, behind
experimentalDocgenServer. Property and event bindings only. With the flag off nothing changes at all.How this is measured
Fifteen snippets are recorded across eleven Angular fixtures and compared against the ones the browser path produces, so no binding may go missing. The recorded output for a two-way
model(), which stays an input plus aChangeoutput rather than banana-in-a-box:Attribute order and formatting are free to differ; a lost binding is not. Dropping the outputs a story never declared looks like this:
yarn vitest run code/lib/docgen-harness/src/angularyarn vitest run code/frameworks/angular-vite/src/docgenyarn vitest run code/lib/docgen-harness/src/angular -uKnown limitations, all documented next to the generator
Structural directives, banana-in-a-box and content projection are out of scope. Values are read from the source rather than evaluated, so
args: { kind: ButtonKind.Secondary }prints as written.The one accepted regression: the snippet is static, so it no longer follows the Controls. That was agreed for the server-side docs path and already applies to Vue.
A story that supplies its own
template, a CSF2 function story, or a re-exported story is not read yet and falls back to generated bindings. That is the next slice, and no fixture in the parity harness exercises those shapes, which is why they split cleanly.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
yarn task sandbox --template angular-vite/default-ts --start-from auto.storybook/main.ts, setfeatures: { experimentalDocgenServer: true }yarn storybookExample/Button→Docsand press Show code. You should see a real Angular template, not the story's source:primarycontrol toFalse. The story re-renders, the snippet does not change - that is the static-snippet trade-off, and it is also how you can tell the server generator is the one driving the block.next.Warning
With
experimentalDocgenServeron, Compodoc does not run at all, so a sandbox where Compodoc fails no longer affects the path this PR adds. Step 6, which turns the flag back off, does still hit it: Compodoc 2 crashes on TypeScript 7, and a freshly generated Angular sandbox now resolves TypeScript 7 because the template'stypescript@^6pin sits independencieswhile the Angular CLI scaffold's^7.0.2sits indevDependencies. Pinningtypescriptback to^6in the sandbox works around it. This breaks the existing browser path too, so it is not caused by this PR.Documentation
The v1 limitations are written up in the framework package as source material for the user-facing docs page, which a later story owns.
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.