Skip to content

Angular: Keep the function control on constructor and generic signatures - #35921

Merged
valentinpalkovic merged 1 commit into
nextfrom
valentin/sb-1809-bug-angular-constructor-and-generic-function-inputs-lose-the
Aug 17, 2026
Merged

valentinpalkovic merged 1 commit into
nextfrom
valentin/sb-1809-bug-angular-constructor-and-generic-function-inputs-lose-the

Conversation

@valentinpalkovic

Copy link
Copy Markdown
Contributor

What I did

A function-typed Angular input shows its real signature in the props table since we started rendering signatures instead of a bare function. Two shapes never got the second half of that deal, though: a constructor type (new (...) => T) and a generic signature (<T>(...) => T) both keep a correct summary but lose the function control and fall back to the other / empty-enum catch-all, which the user sees as an unusable object control.

The summary and the control are independent fields. Hence, there is no reason for getting one right to cost the other.

The cause is a drift between two places that have to agree. TypeIndex.render learned to lead a constructor type with new and a generic one with its type parameters, while the sbType predicate still only accepted a leading (:

@Input() factory!: new (value: number) => Thing
        │
        ▼
  TypeIndex.render          ──▶  "new (value: number) => Thing"
        │                          summary: correct
        ▼
  isFunctionTypeString()    ──▶  false   (matched only /^\(.*\)\s*=>/)
        │
        ▼
  extractType() fallback    ──▶  { name: 'other', value: 'empty-enum' }
                                   control: wrong

The generic half is a regression from 3ab7feccdc6, which added type-parameter rendering to fix invalid type text in generated metadata and quietly paid for it with the control. The constructor half never had a control to begin with.

Solution

Teach the predicate the two prefixes the renderer actually emits:

-const isFunctionTypeString = (type: string): boolean =>
-  type === 'function' || /^\(.*\)\s*=>/.test(type);
+const isFunctionTypeString = (type: string): boolean =>
+  type === 'function' || /^(new\s+)?(<.*>\s*)?\(.*\)\s*=>/.test(type);

The summary text is untouched - it was already right in both cases. The same predicate is duplicated verbatim in @storybook/angular-compodoc (behind the modern flag), so it is fixed in both packages to stop the two paths drifting apart again.

What changes, per declared input type:

Declared input type table.type.summary control before control after
(value: number) => string (value: number) => string function function
(a?: string, ...rest: number[]) => void (a?: string, ...rest: number[]) => void function function
<T>(value: T) => T <T>(value: T) => T empty-enum function
new (value: number) => Thing new (value: number) => Thing empty-enum function
new <T>(value: T) => Thing new <T>(value: T) => Thing empty-enum function

Only the control column moves. Array<(value: number) => string> still does not read as a function, which is pinned by a negative test in both packages.

This is what a bad run looks like, from the tests before the one-line fix:

 FAIL  code/lib/angular-cm/src/extract-arg-types.test.ts > function-typed inputs > keeps both for a generic constructor type, which leads with both
AssertionError: expected { name: 'other', value: 'empty-enum' } to deeply equal { name: 'function' }

- Expected
+ Received

  {
-   "name": "function",
+   "name": "other",
+   "value": "empty-enum",
  }

Worth noting for reviewers: the new angular-cm tests drive the real analyzer through componentIn rather than feeding the predicate hand-written type strings. That is deliberate. The defect is precisely a mismatch between what the renderer emits and what the predicate accepts, so strings picked by a test author would have sailed straight through the whole regression.

No recorded baseline moved, since no fixture contained either shape.

One deviation worth flagging

The issue asked for the two shapes to be added to the docgen-harness fixture corpus as well. I skipped that on purpose. compodoc-input.json is a committed Compodoc 2.0.0 recording with no record script, Compodoc 2.0.0 crashes under TypeScript 7 in this repo, and that corpus exists to gate ACM against Compodoc - which collapses every function type to function and therefore exercises none of this. I would argue it buys churn and a fragile regeneration step for zero signal, and that the package-level tests cover both shapes better. Happy to add it anyway if someone disagrees.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

  1. Generate an Angular sandbox on the docgen-server template: yarn task sandbox --template angular-vite/docgen-server-ts --start-from auto
  2. Open code/frameworks/angular-vite/template/stories/argTypes/doc-button/doc-button.component.ts in the sandbox and add two inputs to the component class:
    @Input() factory!: new (value: number) => Date;
    @Input() mapper!: <T>(value: T) => T;
  3. Start the sandbox and open the argTypes/doc-button story
  4. Open the Controls panel and look at the factory and mapper rows

Expected: the Type column shows the full signature (new (value: number) => Date and <T>(value: T) => T), and the Control column shows the function control placeholder. Before this PR both rows fell back to a raw JSON/object editor instead.

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

No documentation change needed - this restores intended behaviour on an existing field and adds no new API.

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Declare whether manual QA will be needed for this PR during the next release, through qa:needed or qa:skip

  • Make 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/core team 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>

TypeIndex.render leads a constructor type with `new ` and a generic
signature with its type parameters, but isFunctionTypeString only
accepted a leading `(`. Both shapes therefore got a correct
table.type.summary and lost the function control to the other/empty-enum
catch-all.

The generic case regressed in 3ab7fec, which added type-parameter
rendering to fix invalid type text and paid for it with the control. The
constructor case never had one.

The new angular-cm tests go through the real analyzer rather than
hand-written type strings, because the defect was a drift between what
the renderer emits and what the predicate accepts; strings chosen by a
test author would not have caught it.
@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4b7524d9-afca-43c1-8ad7-821b29a9ee59

📥 Commits

Reviewing files that changed from the base of the PR and between 37f57dc and 7fb4f9e.

📒 Files selected for processing (4)
  • code/lib/angular-cm/src/extract-arg-types.test.ts
  • code/lib/angular-cm/src/extract-arg-types.ts
  • code/lib/angular-compodoc/src/extract-arg-types.test.ts
  • code/lib/angular-compodoc/src/extract-arg-types.ts

Included review availability: 1 review is currently available. Based on recent review activity, included reviews refill at 3 per hour.


Walkthrough

Changes

Angular function type detection

Layer / File(s) Summary
Angular CM detection and coverage
code/lib/angular-cm/src/extract-arg-types.ts, code/lib/angular-cm/src/extract-arg-types.test.ts
Angular CM recognizes constructor and generic function signatures. Tests cover signature preservation, function controls, and non-function wrapper types.
Angular Compodoc detection and coverage
code/lib/angular-compodoc/src/extract-arg-types.ts, code/lib/angular-compodoc/src/extract-arg-types.test.ts
Angular Compodoc recognizes constructor and generic function signatures. Tests confirm nested function signatures in array types remain non-function controls.

Merge Risk: ⚪ Minimal · up to 7fb4f

This restores usable function controls for Angular constructor and generic function inputs without changing their displayed signatures; no actionable merge-blocking risk remains beyond normal checks and review.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands.

@valentinpalkovic valentinpalkovic added angular ci:normal Run our default set of CI jobs (choose this for most PRs). labels Aug 17, 2026
@valentinpalkovic valentinpalkovic added the qa:skip Pull Requests that do not need any QA. (e.g. documentation) label Aug 17, 2026
@valentinpalkovic
valentinpalkovic merged commit b8cc04e into next Aug 17, 2026
161 of 167 checks passed
@valentinpalkovic
valentinpalkovic deleted the valentin/sb-1809-bug-angular-constructor-and-generic-function-inputs-lose-the branch August 17, 2026 19:47
@github-actions github-actions Bot mentioned this pull request Aug 17, 2026
3 tasks done
huang-julien added a commit to storybookjs/sandboxes that referenced this pull request Aug 18, 2026
Check the diff here: storybookjs/storybook@f96ed20...add38a0

List of included PRs since previous version:
- storybookjs/storybook#35922 (valentin/sb-1766-angular-docgen-documentation-pass)
- storybookjs/storybook#35844 (s-robertson/u/srobertson/fix-react-component-meta-union-props)
- storybookjs/storybook#35931 (valentin/sb-1847-componentid-collision-warning)
- storybookjs/storybook#35923 (valentin/sb-1789-server-side-code-snippets-resolve-spreads-and-identifier)
- storybookjs/storybook#35940 (valentin/sb-1789-review-fixes)
- storybookjs/storybook#35900 (julien/vue-api-description)
- storybookjs/storybook#35938 (fix-publish-ansi-parsing)
- storybookjs/storybook#35929 (valentin/sb-1821-pin-oxc-resolver)
- storybookjs/storybook#35936 (chore/changelog-v10.5.9)
- storybookjs/storybook#35930 (valentin/sb-1789-review-fixes)
- storybookjs/storybook#35921 (valentin/sb-1809-bug-angular-constructor-and-generic-function-inputs-lose-the)
- storybookjs/storybook#35917 (norbert/fix-publish-staged-retries)
- storybookjs/storybook#35896 (valentin/sb-1776-angular-docs-end-to-end)
- storybookjs/storybook#35920 (julien/vue_server_docgen_options)
- storybookjs/storybook#35907 (valentin/docgen-server-arg-types)
- storybookjs/storybook#35886 (valentin/sb-1799-default-docgen-server-angular-vite)
- storybookjs/storybook#35902 (fix/vue-snippet-runtimeoverride)
- storybookjs/storybook#35825 (norbert/module-graph-skip-noop-mirror)
- storybookjs/storybook#35629 (reuben/fix-pseudo-states-cssom-rewrites)
- storybookjs/storybook#35915 (next-merge-prerelease)
- storybookjs/storybook#35906 (valentin/angular-docs-decorator-gate)
- storybookjs/storybook#35830 (version-non-patch-from-10.6.0-alpha.5)
- storybookjs/storybook#35899 (valentin/angular-required-input-with-default)
- storybookjs/storybook#35831 (norbert/spike-module-graph-hot-cold-split)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

angular bug ci:normal Run our default set of CI jobs (choose this for most PRs). qa:skip Pull Requests that do not need any QA. (e.g. documentation)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants