Repository navigation
Angular: Record two compodoc signal gaps as harness fixtures - #35628
Closed
valentinpalkovic wants to merge 2 commits into
Closed
valentinpalkovic wants to merge 2 commits into
valentinpalkovic wants to merge 2 commits into
Conversation
Compodoc does not read signals through the type checker. It matches the
initializer's source text against the literal names input, output and model,
so two shapes get lost.
An aliased import is the first. Write `import { input as ngInput }` and the
name no longer matches, so the member is demoted to an untyped property.
Angular resolves it as a normal input, so only the docs are wrong.
A non-literal default is the second. An array or a call keeps its raw text but
records no type, a sole object literal is read as the options bag and records
neither, and `input.required()` records no type either.
Both are recorded as fixtures with a red marker in angular-legacy-gaps.test.ts.
The captures are real: compodoc 2.0.0, the version we pin, for the docs, and
ngc for the AOT input maps.
5 of 9 tasks
valentinpalkovic
added
the
ci:normal
Contributor
Package BenchmarksCommit: No significant changes detected, all good. 👏 |
valentinpalkovic
added
the
qa:skip
valentinpalkovic
marked this pull request as ready for review
August 3, 2026 10:40
Contributor
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (19)
WalkthroughAdded Angular harness fixtures for aliased signal imports and non-literal signal-input defaults. Added AOT, Compodoc, Storybook, snapshot, and TypeScript metadata. Added legacy-gap tests and documentation. ChangesAngular signal legacy gaps
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 (1)
code/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/aot-cmp.ts (1)
1-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace provenance and parser-detail comments with maintenance rationale.
These comments expose capture provenance or internal parser mechanics. Keep only the stable reason that the fixture or test exists.
code/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/aot-cmp.ts#L1-L5: State that signal inputs require AOT metadata because JIT does not populate the maps read by the recorder.code/lib/docgen-harness/src/angular/angular-legacy-gaps.test.ts#L74-L76: State that legacy Compodoc does not recognize aliased signal input imports.code/lib/docgen-harness/src/angular/angular-legacy-gaps.test.ts#L81-L84: State that legacy Compodoc loses type and default metadata for non-literal signal input defaults.As per coding guidelines, comments should explain maintenance rationale for future maintainers, not investigation transcripts, and must not commit provenance claims.
🤖 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/__testfixtures__/signal-non-literal-defaults/aot-cmp.ts` around lines 1 - 5, Replace the provenance and parser-detail comments with concise maintenance rationale: in code/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/aot-cmp.ts lines 1-5, state that signal inputs require AOT metadata because JIT does not populate the maps read by the recorder; in code/lib/docgen-harness/src/angular/angular-legacy-gaps.test.ts lines 74-76, state that legacy Compodoc does not recognize aliased signal input imports; and in lines 81-84, state that legacy Compodoc loses type and default metadata for non-literal signal input defaults.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/lib/docgen-harness/README.md`:
- Around line 139-140: Clarify the README entries so the
`signal-aliased-import/` case explicitly refers to an aliased Angular import
such as `import { input as ngInput }`, while the existing entry around the
aliased signal inputs identifies Angular input metadata aliases. Ensure the two
documented behaviors are clearly distinguished rather than appearing
contradictory.
In
`@code/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/signal-non-literal-defaults.component.ts`:
- Line 18: Update requiredValue to call input.required with an explicit string
type parameter so type metadata is generated, then update
angular-legacy-gaps.test.ts to assert string type metadata and no default value
for requiredValue.
---
Nitpick comments:
In
`@code/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/aot-cmp.ts`:
- Around line 1-5: Replace the provenance and parser-detail comments with
concise maintenance rationale: in
code/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/aot-cmp.ts
lines 1-5, state that signal inputs require AOT metadata because JIT does not
populate the maps read by the recorder; in
code/lib/docgen-harness/src/angular/angular-legacy-gaps.test.ts lines 74-76,
state that legacy Compodoc does not recognize aliased signal input imports; and
in lines 81-84, state that legacy Compodoc loses type and default metadata for
non-literal signal input defaults.
🪄 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: 5d3ce6ab-4739-40ae-96f8-60145ad41f9a
📒 Files selected for processing (19)
code/lib/docgen-harness/README.mdcode/lib/docgen-harness/src/angular/__testfixtures__/signal-aliased-import/aot-cmp.tscode/lib/docgen-harness/src/angular/__testfixtures__/signal-aliased-import/argtypes-filtered.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/signal-aliased-import/argtypes.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/signal-aliased-import/compodoc-input.jsoncode/lib/docgen-harness/src/angular/__testfixtures__/signal-aliased-import/input.stories.tscode/lib/docgen-harness/src/angular/__testfixtures__/signal-aliased-import/signal-aliased-import.component.tscode/lib/docgen-harness/src/angular/__testfixtures__/signal-aliased-import/snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/signal-aliased-import/tsconfig.jsoncode/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/aot-cmp.tscode/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/argtypes-filtered.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/argtypes.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/compodoc-input.jsoncode/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/input.stories.tscode/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/signal-non-literal-defaults.component.tscode/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/snippet-PropsAsWritten.snapshotcode/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/tsconfig.jsoncode/lib/docgen-harness/src/angular/angular-legacy-gaps.test.tscode/lib/docgen-harness/src/angular/angular-render.test.ts
|
|
||
| config = input({ a: 1 }); | ||
|
|
||
| requiredValue = input.required(); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'input\.required' code/lib/docgen-harness/src/angular
rg -n -C 5 'requiredValue|nonLiteralDefaultsArgTypes' \
code/lib/docgen-harness/src/angular/angular-legacy-gaps.test.ts \
code/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaultsRepository: storybookjs/storybook
Length of output: 20036
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- fixture and test ---'
cat -n code/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/signal-non-literal-defaults.component.ts
sed -n '70,92p' code/lib/docgen-harness/src/angular/angular-legacy-gaps.test.ts
printf '%s\n' '--- requiredValue snapshot entries ---'
for f in \
code/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/argtypes.snapshot \
code/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/argtypes-filtered.snapshot \
code/lib/docgen-harness/src/angular/__testfixtures__/signal-io/argtypes.snapshot \
code/lib/docgen-harness/src/angular/__testfixtures__/signal-io/argtypes-filtered.snapshot
do
echo "### $f"
awk '/"requiredValue"|"count"/{p=1} p{print} p && /^ },$/{p=0}' "$f"
done
printf '%s\n' '--- Angular input.required declarations and project versions ---'
rg -n -C 3 'required\s*[:=].*input|input\.required|InputSignal' code/lib/docgen-harness package.json code/package.json 2>/dev/null | head -220
rg -n '"`@angular/core`"|"angular"' package.json code/package.json code/lib/docgen-harness/package.json 2>/dev/null | head -80Repository: storybookjs/storybook
Length of output: 9889
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
fixture = Path("code/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/signal-non-literal-defaults.component.ts").read_text()
snapshot = Path("code/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/argtypes.snapshot").read_text()
tests = Path("code/lib/docgen-harness/src/angular/angular-legacy-gaps.test.ts").read_text()
decl = re.search(r"requiredValue\s*=\s*input\.required(?:<([^>]+)>)?\(\s*\)", fixture)
assert decl, "requiredValue declaration not found"
print("requiredValue generic:", decl.group(1) or "<none>")
entry = re.search(r' "requiredValue": \{(.*?)\n \},', snapshot, re.S)
assert entry, "requiredValue snapshot entry not found"
body = entry.group(1)
summary = re.search(r'"summary":\s*([^,\n]+)', body)
print("requiredValue snapshot summaries:", summary.group(1) if summary else "<none>")
print("required flag present:", '"required": true' in body)
print("requiredValue asserted in gap test:", "requiredValue" in tests)
assert decl.group(1) is None
assert summary is None or summary.group(1).strip() == "undefined"
assert '"required": true' in body
assert "requiredValue" not in tests
PY
printf '%s\n' '--- exact snapshot entry ---'
sed -n '57,76p' code/lib/docgen-harness/src/angular/__testfixtures__/signal-non-literal-defaults/argtypes.snapshotRepository: storybookjs/storybook
Length of output: 673
Give requiredValue a concrete source type.
input.required() produces no type metadata here. The snapshot records an undefined type summary. Use input.required<string>(), then assert requiredValue has string type metadata and no default value in angular-legacy-gaps.test.ts.
Proposed fix
- requiredValue = input.required();
+ requiredValue = input.required<string>();📝 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
| requiredValue = input.required(); | |
| requiredValue = input.required<string>(); |
🤖 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/__testfixtures__/signal-non-literal-defaults/signal-non-literal-defaults.component.ts`
at line 18, Update requiredValue to call input.required with an explicit string
type parameter so type metadata is generated, then update
angular-legacy-gaps.test.ts to assert string type metadata and no default value
for requiredValue.
huang-julien
approved these changes
Aug 3, 2026
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.
What I did
code/lib/docgen-harnessrecords what today's Angular docgen actually produces, so that when we replace it we can see exactly what changed.This adds two more recordings, both about signal inputs.
Compodoc does not read signals through the TypeScript type checker.
It matches the initializer's source text against the literal names
input,outputandmodel.Two shapes therefore get lost:
import { input as ngInput }andaliased = ngInput('hello'), and compodoc stops seeing an input at all — the member is demoted to an untyped property. Angular itself resolves it as a normal working input, so only the docs are wrong.input([1, 2, 3])andinput(makeDefault())keep their raw default text but record no type.input({ a: 1 })is read as the options bag, so it records neither a type nor a default.input.required()records no type either.Both fixtures are real captures, not hand-written ones: compodoc 2.0.0, the version we pin, for the docs side, and real
ngcoutput for theɵcmpinput maps.Each gap gets a red marker in
angular-legacy-gaps.test.ts, so it turns green on its own once a replacement resolves these correctly.Test fixtures only — no production code is touched.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Expect 11 files passing, 167 passed and 26 expected failures.
The two new expected failures are
a signal input behind an aliased import is still recognized as an inputandnon-literal signal input defaults still resolve real type/default info.They are red on purpose: they describe what a correct engine should say, and today's engine does not say it.
Documentation
The two gaps are listed under "Known legacy gaps (angular)" in
code/lib/docgen-harness/README.md.Checklist for Maintainers
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.tsqa:neededorqa:skipbuild