Repository navigation
Angular: Extract Compodoc parsing into its own package - #35749
Merged
Merged
Conversation
valentinpalkovic
added
ci:normal
5 of 9 tasks
valentinpalkovic
marked this pull request as ready for review
August 4, 2026 11:34
valentinpalkovic
force-pushed
the
valentin/sb11-angular-compodoc-package
branch
from
August 4, 2026 11:41
ef82271 to
4628f33
Compare
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR adds a shared ChangesAngular Compodoc centralization
Sequence Diagram(s)sequenceDiagram
participant AngularIntegration
participant BrowserAdapter
participant StorybookGlobals
participant SharedParser
AngularIntegration->>BrowserAdapter: call extractArgTypes(component)
BrowserAdapter->>StorybookGlobals: read Compodoc JSON and FEATURES
BrowserAdapter->>SharedParser: call extractArgTypesFromData
SharedParser-->>BrowserAdapter: return ArgTypes
BrowserAdapter-->>AngularIntegration: return ArgTypes
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: 2
🧹 Nitpick comments (3)
code/lib/angular-compodoc/src/extract-arg-types.test.ts (1)
39-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the
model()signal path.The three cases cover alias and enum resolution. The most intricate logic in the module is the
model()detection at lines 344-350 and the${name}Changesynthesis at lines 398-421 ofextract-arg-types.ts. That logic relies on a name appearing in bothinputsClassandoutputsClass, which no test exercises. A regression there would change rendered arg tables silently.Add a case where the same name exists in both arrays. Assert that the bare output row is suppressed and that
<name>Changeis present withcategory: 'outputs'and nodefaultValue.🤖 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/angular-compodoc/src/extract-arg-types.test.ts` around lines 39 - 74, Add a test in the extractArgTypesFromData suite that provides the same property name in both inputsClass and outputsClass, exercising model() detection. Assert the bare input/output row is omitted and the synthesized <name>Change entry exists with category 'outputs' and no defaultValue.code/lib/angular-compodoc/src/extract-arg-types.ts (1)
347-350: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueThe
as Property[]cast can let a method reach the synthesis block.
readMembers(componentData, 'outputsClass')returns(Method | Property)[]. The filter only compares names, then the result is cast toProperty[]. Line 360 excludes methods from the suppression branch, butmodelPropertieskeeps them. AMethodentry inoutputsClasswhose name also exists ininputsClasswould then producesummary: "(e: undefined) => void"at line 411, and the original output row would not be suppressed.Narrow with
isMethodinstead of casting.♻️ Proposed narrowing
- const modelProperties = readMembers(componentData, 'outputsClass').filter((item) => - inputClassNames.has(item.name) - ) as Property[]; + const modelProperties = readMembers(componentData, 'outputsClass').filter( + (item): item is Property => !isMethod(item) && inputClassNames.has(item.name) + );Also applies to: 398-414
🤖 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/angular-compodoc/src/extract-arg-types.ts` around lines 347 - 350, Update the modelProperties filter in the outputsClass handling to exclude methods using the existing isMethod type guard, rather than relying on the Property[] cast. Ensure modelPropertyNames and the later synthesis logic only receive actual properties, while preserving the existing name-matching behavior.code/lib/angular-compodoc/src/compodoc-types.ts (1)
28-54: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign
ClassandPipewith Compodoc output.
Classusestype: 'class'withoutngname.Pipeusestype: 'pipe'withngname. Swap the literals and movengnametoPipe.🤖 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/angular-compodoc/src/compodoc-types.ts` around lines 28 - 54, Update the Class and Pipe interfaces in compodoc-types.ts to match Compodoc output: set Class.type to 'class' and remove ngname, while setting Pipe.type to 'pipe' and adding ngname. Leave the remaining properties unchanged.
🤖 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/lib/angular-compodoc/README.md`:
- Around line 7-8: Update the integration contract in the README to include
unwrapHtml alongside the existing Compodoc JSON, feature flag, and logger
arguments for the root API, and state that the ./browser adapter supplies it
from the browser environment.
In `@code/lib/angular-compodoc/src/browser.ts`:
- Around line 40-41: Update unwrapHtml to return a guaranteed string by applying
a null fallback to the parsed document body's textContent, preserving the
existing HTML parsing behavior.
---
Nitpick comments:
In `@code/lib/angular-compodoc/src/compodoc-types.ts`:
- Around line 28-54: Update the Class and Pipe interfaces in compodoc-types.ts
to match Compodoc output: set Class.type to 'class' and remove ngname, while
setting Pipe.type to 'pipe' and adding ngname. Leave the remaining properties
unchanged.
In `@code/lib/angular-compodoc/src/extract-arg-types.test.ts`:
- Around line 39-74: Add a test in the extractArgTypesFromData suite that
provides the same property name in both inputsClass and outputsClass, exercising
model() detection. Assert the bare input/output row is omitted and the
synthesized <name>Change entry exists with category 'outputs' and no
defaultValue.
In `@code/lib/angular-compodoc/src/extract-arg-types.ts`:
- Around line 347-350: Update the modelProperties filter in the outputsClass
handling to exclude methods using the existing isMethod type guard, rather than
relying on the Property[] cast. Ensure modelPropertyNames and the later
synthesis logic only receive actual properties, while preserving the existing
name-matching behavior.
🪄 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: 84871fc3-3c0c-4db7-a6db-cc5f79153cb9
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (20)
code/frameworks/angular-vite/package.jsoncode/frameworks/angular-vite/src/client/compodoc-types.tscode/frameworks/angular-vite/src/client/compodoc.tscode/frameworks/angular/package.jsoncode/frameworks/angular/src/client/compodoc-types.tscode/frameworks/angular/src/client/compodoc.tscode/lib/angular-compodoc/README.mdcode/lib/angular-compodoc/build-config.tscode/lib/angular-compodoc/package.jsoncode/lib/angular-compodoc/project.jsoncode/lib/angular-compodoc/src/browser.tscode/lib/angular-compodoc/src/compodoc-types.tscode/lib/angular-compodoc/src/extract-arg-types.test.tscode/lib/angular-compodoc/src/extract-arg-types.tscode/lib/angular-compodoc/src/index.tscode/lib/angular-compodoc/src/typings.d.tscode/lib/angular-compodoc/tsconfig.jsoncode/lib/angular-compodoc/vitest.config.tsscripts/build/entry-configs.tsscripts/verdaccio.yaml
💤 Files with no reviewable changes (1)
- scripts/verdaccio.yaml
valentinpalkovic
force-pushed
the
valentin/sb11-angular-compodoc-package
branch
from
August 4, 2026 12:22
4628f33 to
fa60c5c
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/lib/angular-compodoc/src/compodoc-types.ts`:
- Line 20: Update the Property type definition in compodoc-types.ts to allow
type to be omitted, matching extractType’s existing defaultValue-based inference
path. Preserve the current string type when provided and use the project’s
existing TypeScript optional-property conventions.
- Around line 28-54: Update the Class interface to use type 'class' and remove
ngname, and update the Pipe interface to use type 'pipe' and require ngname,
matching the Compodoc payload schemas while leaving their other fields
unchanged.
🪄 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: c725ffee-58ff-4046-92ac-27efdb8f69dc
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (19)
code/frameworks/angular-vite/package.jsoncode/frameworks/angular-vite/src/client/compodoc-types.tscode/frameworks/angular-vite/src/client/compodoc.tscode/frameworks/angular/package.jsoncode/frameworks/angular/src/client/compodoc-types.tscode/frameworks/angular/src/client/compodoc.tscode/lib/angular-compodoc/README.mdcode/lib/angular-compodoc/build-config.tscode/lib/angular-compodoc/package.jsoncode/lib/angular-compodoc/project.jsoncode/lib/angular-compodoc/src/browser.tscode/lib/angular-compodoc/src/compodoc-types.tscode/lib/angular-compodoc/src/extract-arg-types.test.tscode/lib/angular-compodoc/src/extract-arg-types.tscode/lib/angular-compodoc/src/index.tscode/lib/angular-compodoc/src/typings.d.tscode/lib/angular-compodoc/tsconfig.jsoncode/lib/angular-compodoc/vitest.config.tsscripts/build/entry-configs.ts
🚧 Files skipped from review as they are similar to previous changes (17)
- code/frameworks/angular/package.json
- code/frameworks/angular-vite/package.json
- code/lib/angular-compodoc/package.json
- code/lib/angular-compodoc/vitest.config.ts
- code/lib/angular-compodoc/src/extract-arg-types.test.ts
- code/frameworks/angular-vite/src/client/compodoc-types.ts
- code/frameworks/angular-vite/src/client/compodoc.ts
- code/frameworks/angular/src/client/compodoc.ts
- scripts/build/entry-configs.ts
- code/lib/angular-compodoc/README.md
- code/lib/angular-compodoc/build-config.ts
- code/frameworks/angular/src/client/compodoc-types.ts
- code/lib/angular-compodoc/tsconfig.json
- code/lib/angular-compodoc/src/typings.d.ts
- code/lib/angular-compodoc/src/browser.ts
- code/lib/angular-compodoc/src/extract-arg-types.ts
- code/lib/angular-compodoc/project.json
huang-julien
reviewed
Aug 4, 2026
valentinpalkovic
force-pushed
the
valentin/sb11-angular-compodoc-package
branch
from
August 4, 2026 13:42
fa60c5c to
d906da3
Compare
huang-julien
reviewed
Aug 4, 2026
The code that turns Compodoc's output into argTypes existed twice, once in each of the two Angular framework packages, as byte-identical copies. Any fix applied to one had to be remembered for the other, and in practice they drifted. It now lives in one place both of them share. The extracted module no longer assumes a browser. What differs between environments is passed in rather than reached for: the Compodoc data, the feature flag, a logger, and the helper that unwraps Compodoc's HTML-rendered JSDoc. Each framework's preview supplies its own. The shared package is private and compiled into both framework packages rather than published, so nothing new appears on npm and neither package gains a runtime dependency. No behaviour changes. Both frameworks' existing test suites pass untouched, and the preview still unwraps HTML with DOMParser exactly as before.
valentinpalkovic
force-pushed
the
valentin/sb11-angular-compodoc-package
branch
2 times, most recently
from
August 4, 2026 19:22
53f1137 to
9095526
Compare
4 of 9 tasks
valentinpalkovic
force-pushed
the
valentin/sb11-angular-compodoc-package
branch
from
August 5, 2026 00:34
9095526 to
53f1137
Compare
huang-julien
approved these changes
Aug 5, 2026
2 tasks done
This was referenced Aug 13, 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
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
The code that turns Compodoc's output into argTypes existed twice, once in each of the two Angular framework packages, as byte-identical copies. Any fix applied to one had to be remembered for the other, and in practice they drifted. This moves it into a single place both packages share.
The extracted module no longer assumes it is running in a browser. The things that differ between environments are passed in rather than reached for: the Compodoc data, the feature flag, a logger, and the helper that unwraps Compodoc's HTML-rendered JSDoc comments. Each framework's preview supplies its own.
The shared package is private and is compiled into both Angular framework packages rather than published, so nothing new appears on npm and neither package gains a runtime dependency.
Nothing about the rendered output changes. Both frameworks' existing test suites pass without being touched, and the preview still unwraps HTML the same way it always has.
This is the first of a stack of two. It stands on its own; the follow-up adds a server-side Angular docgen provider that becomes the second consumer of the extracted parsing.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
yarn task --task sandbox --start-from auto --template angular-vite/default-tsnext, with the same descriptions and default values--template angular-cli/default-ts, since both Angular framework packages now share the extracted codeThe recorded Angular docgen baselines are unchanged, so a diff in those would indicate an unintended behaviour change.
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.