From f5ea811512ad328d1761fde2cd833b36445cd4f7 Mon Sep 17 00:00:00 2001 From: Valentin Palkovic Date: Tue, 4 Aug 2026 21:24:02 +0200 Subject: [PATCH] Angular: Derive required inputs from Compodoc's own flag Compodoc reports whether an input is required, and we never read it. We computed requiredness from the TypeScript-optional marker instead, so every signal input rendered as required regardless of how it was declared. Requiredness now comes from Compodoc's flag, with the optional marker still consulted. Both are needed: Compodoc derives its flag from the presence of the `required` key in an `@Input({...})` argument rather than its value, so `@Input({ required: false })` reports itself as required and optional at once, and only requiring the two to agree keeps that case right. A `model()` signal's synthesized change output is now always optional. It used to inherit the requiredness of the input it derives from, which disagreed with how every real output reads. Plain decorator inputs still read as required. Compodoc emits neither flag for them, which is an upstream gap; when it is fixed they correct themselves. --- .../doc-model/argtypes-filtered.snapshot | 6 +- .../doc-model/argtypes.snapshot | 6 +- .../doc-model/argtypes-filtered.snapshot | 6 +- .../doc-model/argtypes.snapshot | 6 +- .../angular-compodoc/src/compodoc-types.ts | 12 +++- .../src/extract-arg-types.test.ts | 56 +++++++++++++++++++ .../angular-compodoc/src/extract-arg-types.ts | 22 +++++++- .../signal-io/argtypes-filtered.snapshot | 6 +- .../signal-io/argtypes.snapshot | 8 +-- .../signal-model/argtypes-filtered.snapshot | 6 +- .../signal-model/argtypes.snapshot | 6 +- 11 files changed, 112 insertions(+), 28 deletions(-) diff --git a/code/frameworks/angular-vite/src/client/docs/__testfixtures__/doc-model/argtypes-filtered.snapshot b/code/frameworks/angular-vite/src/client/docs/__testfixtures__/doc-model/argtypes-filtered.snapshot index d77122d74876..6847572594bb 100644 --- a/code/frameworks/angular-vite/src/client/docs/__testfixtures__/doc-model/argtypes-filtered.snapshot +++ b/code/frameworks/angular-vite/src/client/docs/__testfixtures__/doc-model/argtypes-filtered.snapshot @@ -8,7 +8,7 @@ "summary": "#345F92", }, "type": { - "required": true, + "required": false, "summary": "string", }, }, @@ -23,7 +23,7 @@ "table": { "category": "outputs", "type": { - "required": true, + "required": false, "summary": "(e: string) => void", }, }, @@ -56,7 +56,7 @@ "table": { "category": "outputs", "type": { - "required": true, + "required": false, "summary": "(e: boolean) => void", }, }, diff --git a/code/frameworks/angular-vite/src/client/docs/__testfixtures__/doc-model/argtypes.snapshot b/code/frameworks/angular-vite/src/client/docs/__testfixtures__/doc-model/argtypes.snapshot index d77122d74876..6847572594bb 100644 --- a/code/frameworks/angular-vite/src/client/docs/__testfixtures__/doc-model/argtypes.snapshot +++ b/code/frameworks/angular-vite/src/client/docs/__testfixtures__/doc-model/argtypes.snapshot @@ -8,7 +8,7 @@ "summary": "#345F92", }, "type": { - "required": true, + "required": false, "summary": "string", }, }, @@ -23,7 +23,7 @@ "table": { "category": "outputs", "type": { - "required": true, + "required": false, "summary": "(e: string) => void", }, }, @@ -56,7 +56,7 @@ "table": { "category": "outputs", "type": { - "required": true, + "required": false, "summary": "(e: boolean) => void", }, }, diff --git a/code/frameworks/angular/src/client/docs/__testfixtures__/doc-model/argtypes-filtered.snapshot b/code/frameworks/angular/src/client/docs/__testfixtures__/doc-model/argtypes-filtered.snapshot index d77122d74876..6847572594bb 100644 --- a/code/frameworks/angular/src/client/docs/__testfixtures__/doc-model/argtypes-filtered.snapshot +++ b/code/frameworks/angular/src/client/docs/__testfixtures__/doc-model/argtypes-filtered.snapshot @@ -8,7 +8,7 @@ "summary": "#345F92", }, "type": { - "required": true, + "required": false, "summary": "string", }, }, @@ -23,7 +23,7 @@ "table": { "category": "outputs", "type": { - "required": true, + "required": false, "summary": "(e: string) => void", }, }, @@ -56,7 +56,7 @@ "table": { "category": "outputs", "type": { - "required": true, + "required": false, "summary": "(e: boolean) => void", }, }, diff --git a/code/frameworks/angular/src/client/docs/__testfixtures__/doc-model/argtypes.snapshot b/code/frameworks/angular/src/client/docs/__testfixtures__/doc-model/argtypes.snapshot index d77122d74876..6847572594bb 100644 --- a/code/frameworks/angular/src/client/docs/__testfixtures__/doc-model/argtypes.snapshot +++ b/code/frameworks/angular/src/client/docs/__testfixtures__/doc-model/argtypes.snapshot @@ -8,7 +8,7 @@ "summary": "#345F92", }, "type": { - "required": true, + "required": false, "summary": "string", }, }, @@ -23,7 +23,7 @@ "table": { "category": "outputs", "type": { - "required": true, + "required": false, "summary": "(e: string) => void", }, }, @@ -56,7 +56,7 @@ "table": { "category": "outputs", "type": { - "required": true, + "required": false, "summary": "(e: boolean) => void", }, }, diff --git a/code/lib/angular-compodoc/src/compodoc-types.ts b/code/lib/angular-compodoc/src/compodoc-types.ts index 18174cf65c17..f8fbfce51986 100644 --- a/code/lib/angular-compodoc/src/compodoc-types.ts +++ b/code/lib/angular-compodoc/src/compodoc-types.ts @@ -26,7 +26,17 @@ export interface Property { decorators?: Decorator[]; /** Omitted by Compodoc for members it cannot type, e.g. `@HostBinding`. */ type?: string; - optional: boolean; + /** + * Whether the member is TS-optional. Compodoc omits it entirely for `@Input()`-decorated + * properties while emitting it for signal inputs and plain class properties (compodoc#863, still + * open at 2.0.0), so it is absent far more often than the old non-optional declaration implied. + */ + optional?: boolean; + /** + * Compodoc's own requiredness flag, which is what Angular actually means by a required input. + * Present for signal inputs and for `@Input({ required })`; absent for a plain `@Input()`. + */ + required?: boolean; defaultValue?: string; description?: Html; rawdescription?: string; diff --git a/code/lib/angular-compodoc/src/extract-arg-types.test.ts b/code/lib/angular-compodoc/src/extract-arg-types.test.ts index 6e7bfdb43d7e..d995b56e68d2 100644 --- a/code/lib/angular-compodoc/src/extract-arg-types.test.ts +++ b/code/lib/angular-compodoc/src/extract-arg-types.test.ts @@ -72,3 +72,59 @@ describe('extractArgTypesFromData', () => { expect(() => extract('Status', { enumerations: [{ name: 'Status' }] as never })).not.toThrow(); }); }); + +describe('required', () => { + /** Extracts a single input declared with the given pair of Compodoc flags. */ + const requiredOf = (flags: { optional?: boolean; required?: boolean }) => { + const componentData = { + name: 'StatusComponent', + type: 'component', + inputsClass: [{ name: 'value', type: 'string', ...flags }], + outputsClass: [], + propertiesClass: [], + methodsClass: [], + } as never; + + const argTypes = extractArgTypesFromData(componentData, { + compodocJson: jsonWith({} as never), + filterNonInputControls: true, + logger, + unwrapHtml: (html: unknown) => String(html), + }); + + // `required` has always been written into `table.type`, which the public ArgTypes type + // declares as summary/detail only, so reading it back needs an assertion. + return (argTypes.value.table?.type as { required?: boolean } | undefined)?.required; + }; + + // One case per shape Compodoc can emit. Which declaration produces which pair is recorded here + // because the pairs are not self-explanatory, and one of them is self-contradictory. + it('is false for a signal input with a default: `input("")`', () => { + expect(requiredOf({ optional: false, required: false })).toBe(false); + }); + + it('is true for `input.required()`', () => { + expect(requiredOf({ optional: false, required: true })).toBe(true); + }); + + it('is true for `@Input({ required: true })`', () => { + expect(requiredOf({ optional: false, required: true })).toBe(true); + }); + + it('is false for `@Input({ required: false })`, which Compodoc reports as required and optional at once', () => { + // Compodoc derives `required` from the presence of the key rather than its value, so this + // declaration contradicts itself. Trusting `required` alone would call it required. + expect(requiredOf({ optional: true, required: true })).toBe(false); + }); + + it('falls back to `optional` when Compodoc omits `required`', () => { + expect(requiredOf({ optional: true })).toBe(false); + expect(requiredOf({ optional: false })).toBe(true); + }); + + it('is true for a plain `@Input()`, for which Compodoc emits neither flag (compodoc#863)', () => { + // The remaining upstream gap: with nothing to read, every plain decorator input reads as + // required. Fixing it upstream makes `optional` appear, and this case corrects itself. + expect(requiredOf({})).toBe(true); + }); +}); diff --git a/code/lib/angular-compodoc/src/extract-arg-types.ts b/code/lib/angular-compodoc/src/extract-arg-types.ts index 3dc9395c2567..b5327dc78e73 100644 --- a/code/lib/angular-compodoc/src/extract-arg-types.ts +++ b/code/lib/angular-compodoc/src/extract-arg-types.ts @@ -76,6 +76,21 @@ export const isMethod = (methodOrProp: Method | Property): methodOrProp is Metho return (methodOrProp as Method).args !== undefined; }; +/** + * Whether a member must be bound, from the two flags Compodoc emits about it. + * + * `required` is the flag that matches what Angular means, but it is only trustworthy in one + * direction: Compodoc derives it from the presence of the `required` key in an `@Input({...})` + * argument rather than from its value, so `@Input({ required: false })` reports `required: true` + * alongside `optional: true`. Requiring both to agree keeps that case correct. + * + * `required` is absent altogether for a plain `@Input()`, which falls back to `optional` - and + * Compodoc omits that too (compodoc#863), so those inputs still read as required. That is the + * upstream gap; the moment a fixed Compodoc emits `optional`, this returns the right answer with + * no change here. + */ +const isRequired = (item: Property): boolean => (item.required ?? true) && !item.optional; + export const checkValidComponentOrDirective = (component: Component | Directive) => { if (!component.name) { throw new Error(`Invalid component ${JSON.stringify(component)}`); @@ -371,7 +386,7 @@ export const extractArgTypesFromData = ( category: section, type: { summary: isMethod(item) ? displaySignature(item) : item.type, - required: isMethod(item) ? false : !item.optional, + required: isMethod(item) ? false : isRequired(item as Property), }, defaultValue: { summary: defaultValue }, }, @@ -400,7 +415,10 @@ export const extractArgTypesFromData = ( category: 'outputs', type: { summary: `(e: ${item.type}) => void`, - required: !item.optional, + // An output is never required to bind, and a real output says so via Compodoc's own + // flag. This one is synthesized, so it has to say so itself rather than inheriting the + // requiredness of the model input it derives from. + required: false, }, }, }; diff --git a/code/lib/docgen-harness/src/angular/__testfixtures__/signal-io/argtypes-filtered.snapshot b/code/lib/docgen-harness/src/angular/__testfixtures__/signal-io/argtypes-filtered.snapshot index d5fa9719674a..06ae30f4ff63 100644 --- a/code/lib/docgen-harness/src/angular/__testfixtures__/signal-io/argtypes-filtered.snapshot +++ b/code/lib/docgen-harness/src/angular/__testfixtures__/signal-io/argtypes-filtered.snapshot @@ -27,7 +27,7 @@ "summary": false, }, "type": { - "required": true, + "required": false, "summary": "boolean", }, }, @@ -45,7 +45,7 @@ "summary": 1, }, "type": { - "required": true, + "required": false, "summary": "number", }, }, @@ -63,7 +63,7 @@ Visible caption next to the control.", "summary": "", }, "type": { - "required": true, + "required": false, "summary": "string", }, }, diff --git a/code/lib/docgen-harness/src/angular/__testfixtures__/signal-io/argtypes.snapshot b/code/lib/docgen-harness/src/angular/__testfixtures__/signal-io/argtypes.snapshot index d1d2ddf3535a..be0e98ae35c2 100644 --- a/code/lib/docgen-harness/src/angular/__testfixtures__/signal-io/argtypes.snapshot +++ b/code/lib/docgen-harness/src/angular/__testfixtures__/signal-io/argtypes.snapshot @@ -27,7 +27,7 @@ "summary": false, }, "type": { - "required": true, + "required": false, "summary": "boolean", }, }, @@ -45,7 +45,7 @@ "summary": 1, }, "type": { - "required": true, + "required": false, "summary": "number", }, }, @@ -63,7 +63,7 @@ Visible caption next to the control.", "summary": "", }, "type": { - "required": true, + "required": false, "summary": "string", }, }, @@ -82,7 +82,7 @@ Visible caption next to the control.", "summary": false, }, "type": { - "required": true, + "required": false, "summary": "boolean", }, }, diff --git a/code/lib/docgen-harness/src/angular/__testfixtures__/signal-model/argtypes-filtered.snapshot b/code/lib/docgen-harness/src/angular/__testfixtures__/signal-model/argtypes-filtered.snapshot index 9fe2f82109f3..93023348f9cf 100644 --- a/code/lib/docgen-harness/src/angular/__testfixtures__/signal-model/argtypes-filtered.snapshot +++ b/code/lib/docgen-harness/src/angular/__testfixtures__/signal-model/argtypes-filtered.snapshot @@ -25,7 +25,7 @@ "table": { "category": "outputs", "type": { - "required": true, + "required": false, "summary": "(e: boolean) => void", }, }, @@ -44,7 +44,7 @@ Current text value of the field.", "summary": "start", }, "type": { - "required": true, + "required": false, "summary": "string", }, }, @@ -60,7 +60,7 @@ Current text value of the field.", "table": { "category": "outputs", "type": { - "required": true, + "required": false, "summary": "(e: string) => void", }, }, diff --git a/code/lib/docgen-harness/src/angular/__testfixtures__/signal-model/argtypes.snapshot b/code/lib/docgen-harness/src/angular/__testfixtures__/signal-model/argtypes.snapshot index 9fe2f82109f3..93023348f9cf 100644 --- a/code/lib/docgen-harness/src/angular/__testfixtures__/signal-model/argtypes.snapshot +++ b/code/lib/docgen-harness/src/angular/__testfixtures__/signal-model/argtypes.snapshot @@ -25,7 +25,7 @@ "table": { "category": "outputs", "type": { - "required": true, + "required": false, "summary": "(e: boolean) => void", }, }, @@ -44,7 +44,7 @@ Current text value of the field.", "summary": "start", }, "type": { - "required": true, + "required": false, "summary": "string", }, }, @@ -60,7 +60,7 @@ Current text value of the field.", "table": { "category": "outputs", "type": { - "required": true, + "required": false, "summary": "(e: string) => void", }, },