From ea5179bf1577c2f175781692869e5d232877ea54 Mon Sep 17 00:00:00 2001 From: Shannen Barrameda Date: Mon, 27 Jul 2026 17:08:11 -0400 Subject: [PATCH 1/3] feat: allow multiple declarations of the same property in discriminated union types --- .../discriminated-union/list/index.tsx | 35 +++++++++++++++++++ .../discriminated-union/tsconfig.json | 4 +++ src/shared/type-utils.ts | 18 ++++++++-- test/components/discriminated-union.test.ts | 33 +++++++++++++++++ test/shared/type-utils.test.ts | 30 +++++++++++++++- 5 files changed, 116 insertions(+), 4 deletions(-) create mode 100644 fixtures/components/discriminated-union/list/index.tsx create mode 100644 fixtures/components/discriminated-union/tsconfig.json create mode 100644 test/components/discriminated-union.test.ts diff --git a/fixtures/components/discriminated-union/list/index.tsx b/fixtures/components/discriminated-union/list/index.tsx new file mode 100644 index 0000000..b0246c4 --- /dev/null +++ b/fixtures/components/discriminated-union/list/index.tsx @@ -0,0 +1,35 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +import * as React from 'react'; + +interface BaseProps { + /** The items to display. */ + items: ReadonlyArray; +} + +interface WithCustomRender { + /** Custom render function for each item. */ + renderItem: () => React.ReactNode | null; + /** The currently active item. */ + activeItem: T | undefined; +} + +interface WithDefaultRender { + /** Not applicable when using the default renderer. */ + renderItem?: never; + /** Not applicable when using the default renderer. */ + activeItem?: never; +} + +export type ListProps = BaseProps & + (WithCustomRender | WithDefaultRender); + +export namespace ListProps { + export interface Item { + id: string; + } +} + +export default function List({ items }: ListProps) { + return
{items.length}
; +} diff --git a/fixtures/components/discriminated-union/tsconfig.json b/fixtures/components/discriminated-union/tsconfig.json new file mode 100644 index 0000000..8afdfb3 --- /dev/null +++ b/fixtures/components/discriminated-union/tsconfig.json @@ -0,0 +1,4 @@ +{ + "extends": "../tsconfig.json", + "include": ["./**/*.tsx"] +} diff --git a/src/shared/type-utils.ts b/src/shared/type-utils.ts index 4137f14..718721d 100644 --- a/src/shared/type-utils.ts +++ b/src/shared/type-utils.ts @@ -77,10 +77,22 @@ export function tryExtractDeclaration(symbol: ts.Symbol): ts.Declaration | undef if (!declarations || declarations.length === 0) { return undefined; } - if (declarations.length > 1) { - throw new Error(`Multiple declarations found for symbol: ${symbol.getName()}`); + if (declarations.length === 1) { + return declarations[0]; } - return declarations[0]; + // Multiple property signature declarations occur in discriminated unions where + // the same property appears in multiple branches (e.g. `prop: SomeType` | `prop?: never`). + // Pick the first declaration whose type is not `never`. + if (declarations.every(decl => ts.isPropertySignature(decl))) { + const nonNever = declarations.find(decl => { + if (ts.isPropertySignature(decl) && decl.type) { + return decl.type.kind !== ts.SyntaxKind.NeverKeyword; + } + return true; + }); + return nonNever ?? declarations[0]; + } + throw new Error(`Multiple declarations found for symbol: ${symbol.getName()}`); } export function isOptionalSymbol(symbol: ts.Symbol): boolean { diff --git a/test/components/discriminated-union.test.ts b/test/components/discriminated-union.test.ts new file mode 100644 index 0000000..dead8c2 --- /dev/null +++ b/test/components/discriminated-union.test.ts @@ -0,0 +1,33 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +import { expect, test, beforeAll } from 'vitest'; +import { ComponentDefinition } from '../../src/components/interfaces'; +import { buildProject } from './test-helpers'; + +let list: ComponentDefinition; + +beforeAll(() => { + const result = buildProject('discriminated-union'); + expect(result).toHaveLength(1); + + list = result[0]; +}); + +test('should extract properties from discriminated union without errors', () => { + expect(list.name).toBe('List'); + const propNames = list.properties.map(p => p.name); + expect(propNames).toContain('items'); + expect(propNames).toContain('renderItem'); + expect(propNames).toContain('activeItem'); +}); + +test('should use the non-never declaration type for shared properties', () => { + const renderProp = list.properties.find(p => p.name === 'renderItem')!; + expect(renderProp).toBeDefined(); + expect(renderProp.type).not.toBe('never'); +}); + +test('should preserve description from discriminated union properties', () => { + const renderProp = list.properties.find(p => p.name === 'renderItem')!; + expect(renderProp.description).toBeDefined(); +}); diff --git a/test/shared/type-utils.test.ts b/test/shared/type-utils.test.ts index f995983..5a6d730 100644 --- a/test/shared/type-utils.test.ts +++ b/test/shared/type-utils.test.ts @@ -3,7 +3,8 @@ import ts from 'typescript'; import { test, expect } from 'vitest'; -import { printFlags } from '../../lib/shared/type-utils'; +import { printFlags, tryExtractDeclaration } from '../../src/shared/type-utils'; +import { getInMemoryProject } from '../components/test-helpers'; test('serialises node flags', () => { const flags = printFlags( @@ -20,3 +21,30 @@ test('serialises type flags', () => { expect(flags).toContain('Number'); expect(flags).toContain('Enum'); }); + +test('tryExtractDeclaration returns undefined for symbols with no declarations', () => { + const symbol = { getDeclarations: () => undefined } as unknown as ts.Symbol; + expect(tryExtractDeclaration(symbol)).toBeUndefined(); +}); + +test('tryExtractDeclaration returns the single declaration', () => { + const decl = {} as ts.Declaration; + const symbol = { getDeclarations: () => [decl] } as unknown as ts.Symbol; + expect(tryExtractDeclaration(symbol)).toBe(decl); +}); + +test('tryExtractDeclaration picks non-never property signature from multiple declarations', () => { + const { exportSymbol, checker } = getInMemoryProject(` + interface BranchA { shared: string; } + interface BranchB { shared?: never; } + export type Combined = BranchA | BranchB; + `); + const combinedType = checker.getDeclaredTypeOfSymbol(exportSymbol); + const sharedSymbol = combinedType.getProperties().find(p => p.getName() === 'shared')!; + const declaration = tryExtractDeclaration(sharedSymbol); + expect(declaration).toBeDefined(); + // Should pick the BranchA declaration (non-never) + if (declaration && ts.isPropertySignature(declaration) && declaration.type) { + expect(declaration.type.kind).not.toBe(ts.SyntaxKind.NeverKeyword); + } +}); From 797861f59f23e97aaa24198449603bb3b8b79b80 Mon Sep 17 00:00:00 2001 From: Shannen Barrameda Date: Tue, 28 Jul 2026 13:01:29 -0400 Subject: [PATCH 2/3] fix: add unit test for tryExtractDeclaration --- test/shared/type-utils.test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/shared/type-utils.test.ts b/test/shared/type-utils.test.ts index 5a6d730..6781b51 100644 --- a/test/shared/type-utils.test.ts +++ b/test/shared/type-utils.test.ts @@ -48,3 +48,18 @@ test('tryExtractDeclaration picks non-never property signature from multiple dec expect(declaration.type.kind).not.toBe(ts.SyntaxKind.NeverKeyword); } }); + +test('tryExtractDeclaration keeps a property signature that has no type annotation', () => { + const { exportSymbol, checker } = getInMemoryProject(` + interface BranchA { shared; } + interface BranchB { shared?: never; } + export type Combined = BranchA | BranchB; + `); + const combinedType = checker.getDeclaredTypeOfSymbol(exportSymbol); + const sharedSymbol = combinedType.getProperties().find(p => p.getName() === 'shared')!; + const declaration = tryExtractDeclaration(sharedSymbol); + expect(declaration).toBeDefined(); + expect(ts.isPropertySignature(declaration!)).toBe(true); + + expect((declaration as ts.PropertySignature).type).toBeUndefined(); +}); From 3ff74c8f383b8c3aa67a5c9cc7b18dae813c7fd6 Mon Sep 17 00:00:00 2001 From: Shannen Barrameda Date: Mon, 3 Aug 2026 11:06:14 -0400 Subject: [PATCH 3/3] fix: throw when the never decl are filtered out and multiple nonNever decl remain --- src/shared/type-utils.ts | 19 ++++++++----------- test/components/discriminated-union.test.ts | 2 +- test/shared/type-utils.test.ts | 14 ++++++++++++++ 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/shared/type-utils.ts b/src/shared/type-utils.ts index 718721d..e484da7 100644 --- a/src/shared/type-utils.ts +++ b/src/shared/type-utils.ts @@ -80,17 +80,14 @@ export function tryExtractDeclaration(symbol: ts.Symbol): ts.Declaration | undef if (declarations.length === 1) { return declarations[0]; } - // Multiple property signature declarations occur in discriminated unions where - // the same property appears in multiple branches (e.g. `prop: SomeType` | `prop?: never`). - // Pick the first declaration whose type is not `never`. - if (declarations.every(decl => ts.isPropertySignature(decl))) { - const nonNever = declarations.find(decl => { - if (ts.isPropertySignature(decl) && decl.type) { - return decl.type.kind !== ts.SyntaxKind.NeverKeyword; - } - return true; - }); - return nonNever ?? declarations[0]; + // Multiple property signature declarations occur in discriminated unions where the same + // property appears in multiple branches (e.g. `prop: SomeType` in one and `prop?: never` in + // another). Keep the branches that aren't `never` and use that declaration when it's unambiguous. + if (declarations.every(ts.isPropertySignature)) { + const nonNever = declarations.filter(decl => decl.type?.kind !== ts.SyntaxKind.NeverKeyword); + if (nonNever.length === 1) { + return nonNever[0]; + } } throw new Error(`Multiple declarations found for symbol: ${symbol.getName()}`); } diff --git a/test/components/discriminated-union.test.ts b/test/components/discriminated-union.test.ts index dead8c2..4d5c1c5 100644 --- a/test/components/discriminated-union.test.ts +++ b/test/components/discriminated-union.test.ts @@ -24,7 +24,7 @@ test('should extract properties from discriminated union without errors', () => test('should use the non-never declaration type for shared properties', () => { const renderProp = list.properties.find(p => p.name === 'renderItem')!; expect(renderProp).toBeDefined(); - expect(renderProp.type).not.toBe('never'); + expect(renderProp.type).toBe('() => React.ReactNode'); }); test('should preserve description from discriminated union properties', () => { diff --git a/test/shared/type-utils.test.ts b/test/shared/type-utils.test.ts index 6781b51..497906d 100644 --- a/test/shared/type-utils.test.ts +++ b/test/shared/type-utils.test.ts @@ -63,3 +63,17 @@ test('tryExtractDeclaration keeps a property signature that has no type annotati expect((declaration as ts.PropertySignature).type).toBeUndefined(); }); + +test('tryExtractDeclaration throws when multiple non-never declarations are ambiguous', () => { + // Three property signature declarations, two of which are non-never (`string` and `number`). + // The result is ambiguous, so no single declaration can be chosen and the function throws. + const { exportSymbol, checker } = getInMemoryProject(` + interface BranchA { shared: string; } + interface BranchB { shared: number; } + interface BranchC { shared?: never; } + export type Combined = BranchA | BranchB | BranchC; + `); + const combinedType = checker.getDeclaredTypeOfSymbol(exportSymbol); + const sharedSymbol = combinedType.getProperties().find(p => p.getName() === 'shared')!; + expect(() => tryExtractDeclaration(sharedSymbol)).toThrow('Multiple declarations found for symbol: shared'); +});