Skip to content

Commit 07dbd8b

Browse files
weswighamweswigham
authored
Discriminate jsx contextual types same as object contextual types (#27408)
* Discriminate jsx contextual types same as object contextual types * Extract core discrimination algorithm to getDiscriminationResultForProperty * Merge all discrimination implementations * Fix lints
1 parent a7b4635 commit 07dbd8b

5 files changed

+349
-38
lines changed

‎src/compiler/checker.ts

Lines changed: 54 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -11640,27 +11640,14 @@ namespace ts {
1164011640

1164111641
// Keep this up-to-date with the same logic within `getApparentTypeOfContextualType`, since they should behave similarly
1164211642
function findMatchingDiscriminantType(source: Type, target: UnionOrIntersectionType) {
11643-
let match: Type | undefined;
1164411643
const sourceProperties = getPropertiesOfObjectType(source);
1164511644
if (sourceProperties) {
1164611645
const sourcePropertiesFiltered = findDiscriminantProperties(sourceProperties, target);
1164711646
if (sourcePropertiesFiltered) {
11648-
for (const sourceProperty of sourcePropertiesFiltered) {
11649-
const sourceType = getTypeOfSymbol(sourceProperty);
11650-
for (const type of target.types) {
11651-
const targetType = getTypeOfPropertyOfType(type, sourceProperty.escapedName);
11652-
if (targetType && isRelatedTo(sourceType, targetType)) {
11653-
if (type === match) continue; // Finding multiple fields which discriminate to the same type is fine
11654-
if (match) {
11655-
return undefined;
11656-
}
11657-
match = type;
11658-
}
11659-
}
11660-
}
11647+
return discriminateTypeByDiscriminableItems(target, map(sourcePropertiesFiltered, p => ([() => getTypeOfSymbol(p), p.escapedName] as [() => Type, __String])), isRelatedTo);
1166111648
}
1166211649
}
11663-
return match;
11650+
return undefined;
1166411651
}
1166511652

1166611653
function typeRelatedToEachType(source: Type, target: IntersectionType, reportErrors: boolean): Ternary {
@@ -12475,6 +12462,25 @@ namespace ts {
1247512462
}
1247612463
}
1247712464

12465+
function discriminateTypeByDiscriminableItems(target: UnionType, discriminators: [() => Type, __String][], related: (source: Type, target: Type) => boolean | Ternary): Type | undefined;
12466+
function discriminateTypeByDiscriminableItems(target: UnionType, discriminators: [() => Type, __String][], related: (source: Type, target: Type) => boolean | Ternary, defaultValue: Type): Type;
12467+
function discriminateTypeByDiscriminableItems(target: UnionType, discriminators: [() => Type, __String][], related: (source: Type, target: Type) => boolean | Ternary, defaultValue?: Type) {
12468+
let match: Type | undefined;
12469+
for (const [getDiscriminatingType, propertyName] of discriminators) {
12470+
for (const type of target.types) {
12471+
const targetType = getTypeOfPropertyOfType(type, propertyName);
12472+
if (targetType && related(getDiscriminatingType(), targetType)) {
12473+
if (match) {
12474+
if (type === match) continue; // Finding multiple fields which discriminate to the same type is fine
12475+
return defaultValue;
12476+
}
12477+
match = type;
12478+
}
12479+
}
12480+
}
12481+
return match || defaultValue;
12482+
}
12483+
1247812484
/**
1247912485
* A type is 'weak' if it is an object type with at least one optional property
1248012486
* and no required properties, call/construct signatures or index signatures
@@ -14187,7 +14193,7 @@ namespace ts {
1418714193
if ((<TransientSymbol>prop).isDiscriminantProperty === undefined) {
1418814194
(<TransientSymbol>prop).isDiscriminantProperty = !!((<TransientSymbol>prop).checkFlags & CheckFlags.HasNonUniformType) && isLiteralType(getTypeOfSymbol(prop));
1418914195
}
14190-
return (<TransientSymbol>prop).isDiscriminantProperty;
14196+
return !!(<TransientSymbol>prop).isDiscriminantProperty;
1419114197
}
1419214198
}
1419314199
return false;
@@ -16762,43 +16768,53 @@ namespace ts {
1676216768
case SyntaxKind.FalseKeyword:
1676316769
case SyntaxKind.NullKeyword:
1676416770
case SyntaxKind.Identifier:
16771+
case SyntaxKind.UndefinedKeyword:
1676516772
return true;
1676616773
case SyntaxKind.PropertyAccessExpression:
1676716774
case SyntaxKind.ParenthesizedExpression:
1676816775
return isPossiblyDiscriminantValue((<PropertyAccessExpression | ParenthesizedExpression>node).expression);
16776+
case SyntaxKind.JsxExpression:
16777+
return !(node as JsxExpression).expression || isPossiblyDiscriminantValue((node as JsxExpression).expression!);
1676916778
}
1677016779
return false;
1677116780
}
1677216781

16782+
function discriminateContextualTypeByObjectMembers(node: ObjectLiteralExpression, contextualType: UnionType) {
16783+
return discriminateTypeByDiscriminableItems(contextualType,
16784+
map(
16785+
filter(node.properties, p => !!p.symbol && p.kind === SyntaxKind.PropertyAssignment && isPossiblyDiscriminantValue(p.initializer) && isDiscriminantProperty(contextualType, p.symbol.escapedName)),
16786+
prop => ([() => checkExpression((prop as PropertyAssignment).initializer), prop.symbol.escapedName] as [() => Type, __String])
16787+
),
16788+
isTypeAssignableTo,
16789+
contextualType
16790+
);
16791+
}
16792+
16793+
function discriminateContextualTypeByJSXAttributes(node: JsxAttributes, contextualType: UnionType) {
16794+
return discriminateTypeByDiscriminableItems(contextualType,
16795+
map(
16796+
filter(node.properties, p => !!p.symbol && p.kind === SyntaxKind.JsxAttribute && isDiscriminantProperty(contextualType, p.symbol.escapedName) && (!p.initializer || isPossiblyDiscriminantValue(p.initializer))),
16797+
prop => ([!(prop as JsxAttribute).initializer ? (() => trueType) : (() => checkExpression((prop as JsxAttribute).initializer!)), prop.symbol.escapedName] as [() => Type, __String])
16798+
),
16799+
isTypeAssignableTo,
16800+
contextualType
16801+
);
16802+
}
16803+
1677316804
// Return the contextual type for a given expression node. During overload resolution, a contextual type may temporarily
1677416805
// be "pushed" onto a node using the contextualType property.
1677516806
function getApparentTypeOfContextualType(node: Expression): Type | undefined {
1677616807
let contextualType = getContextualType(node);
1677716808
contextualType = contextualType && mapType(contextualType, getApparentType);
16778-
if (!(contextualType && contextualType.flags & TypeFlags.Union && isObjectLiteralExpression(node))) {
16779-
return contextualType;
16780-
}
16781-
// Keep the below up-to-date with the work done within `isRelatedTo` by `findMatchingDiscriminantType`
16782-
let match: Type | undefined;
16783-
propLoop: for (const prop of node.properties) {
16784-
if (!prop.symbol) continue;
16785-
if (prop.kind !== SyntaxKind.PropertyAssignment) continue;
16786-
if (isPossiblyDiscriminantValue(prop.initializer) && isDiscriminantProperty(contextualType, prop.symbol.escapedName)) {
16787-
const discriminatingType = checkExpression(prop.initializer);
16788-
for (const type of (contextualType as UnionType).types) {
16789-
const targetType = getTypeOfPropertyOfType(type, prop.symbol.escapedName);
16790-
if (targetType && isTypeAssignableTo(discriminatingType, targetType)) {
16791-
if (match) {
16792-
if (type === match) continue; // Finding multiple fields which discriminate to the same type is fine
16793-
match = undefined;
16794-
break propLoop;
16795-
}
16796-
match = type;
16797-
}
16798-
}
16809+
if (contextualType && contextualType.flags & TypeFlags.Union) {
16810+
if (isObjectLiteralExpression(node)) {
16811+
return discriminateContextualTypeByObjectMembers(node, contextualType as UnionType);
16812+
}
16813+
else if (isJsxAttributes(node)) {
16814+
return discriminateContextualTypeByJSXAttributes(node, contextualType as UnionType);
1679916815
}
1680016816
}
16801-
return match || contextualType;
16817+
return contextualType;
1680216818
}
1680316819

1680416820
/**

‎tests/baselines/reference/checkJsxUnionSFXContextualTypeInferredCorrectly.js

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
//// [checkJsxUnionSFXContextualTypeInferredCorrectly.tsx]
2+
/// <reference path="/.lib/react16.d.ts" />
3+
4+
import React from 'react';
5+
6+
interface PS {
7+
multi: false
8+
value: string | undefined
9+
onChange: (selection: string | undefined) => void
10+
}
11+
12+
interface PM {
13+
multi: true
14+
value: string[]
15+
onChange: (selection: string[]) => void
16+
}
17+
18+
export function ComponentWithUnion(props: PM | PS) {
19+
return <h1></h1>;
20+
}
21+
22+
// Usage with React tsx
23+
export function HereIsTheError() {
24+
return (
25+
<ComponentWithUnion
26+
multi={false}
27+
value={'s'}
28+
onChange={val => console.log(val)} // <- this throws an error
29+
/>
30+
);
31+
}
32+
33+
// Usage with pure TypeScript
34+
ComponentWithUnion({
35+
multi: false,
36+
value: 's',
37+
onChange: val => console.log(val) // <- this works fine
38+
});
39+
40+
41+
//// [checkJsxUnionSFXContextualTypeInferredCorrectly.js]
42+
"use strict";
43+
/// <reference path="react16.d.ts" />
44+
var __importDefault = (this && this.__importDefault) || function (mod) {
45+
return (mod && mod.__esModule) ? mod : { "default": mod };
46+
};
47+
exports.__esModule = true;
48+
var react_1 = __importDefault(require("react"));
49+
function ComponentWithUnion(props) {
50+
return react_1["default"].createElement("h1", null);
51+
}
52+
exports.ComponentWithUnion = ComponentWithUnion;
53+
// Usage with React tsx
54+
function HereIsTheError() {
55+
return (react_1["default"].createElement(ComponentWithUnion, { multi: false, value: 's', onChange: function (val) { return console.log(val); } }));
56+
}
57+
exports.HereIsTheError = HereIsTheError;
58+
// Usage with pure TypeScript
59+
ComponentWithUnion({
60+
multi: false,
61+
value: 's',
62+
onChange: function (val) { return console.log(val); } // <- this works fine
63+
});

‎tests/baselines/reference/checkJsxUnionSFXContextualTypeInferredCorrectly.symbols

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
=== tests/cases/conformance/jsx/checkJsxUnionSFXContextualTypeInferredCorrectly.tsx ===
2+
/// <reference path="react16.d.ts" />
3+
4+
import React from 'react';
5+
>React : Symbol(React, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 2, 6))
6+
7+
interface PS {
8+
>PS : Symbol(PS, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 2, 26))
9+
10+
multi: false
11+
>multi : Symbol(PS.multi, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 4, 14))
12+
13+
value: string | undefined
14+
>value : Symbol(PS.value, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 5, 16))
15+
16+
onChange: (selection: string | undefined) => void
17+
>onChange : Symbol(PS.onChange, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 6, 29))
18+
>selection : Symbol(selection, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 7, 15))
19+
}
20+
21+
interface PM {
22+
>PM : Symbol(PM, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 8, 1))
23+
24+
multi: true
25+
>multi : Symbol(PM.multi, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 10, 14))
26+
27+
value: string[]
28+
>value : Symbol(PM.value, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 11, 15))
29+
30+
onChange: (selection: string[]) => void
31+
>onChange : Symbol(PM.onChange, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 12, 19))
32+
>selection : Symbol(selection, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 13, 15))
33+
}
34+
35+
export function ComponentWithUnion(props: PM | PS) {
36+
>ComponentWithUnion : Symbol(ComponentWithUnion, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 14, 1))
37+
>props : Symbol(props, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 16, 35))
38+
>PM : Symbol(PM, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 8, 1))
39+
>PS : Symbol(PS, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 2, 26))
40+
41+
return <h1></h1>;
42+
>h1 : Symbol(JSX.IntrinsicElements.h1, Decl(react16.d.ts, 2430, 106))
43+
>h1 : Symbol(JSX.IntrinsicElements.h1, Decl(react16.d.ts, 2430, 106))
44+
}
45+
46+
// Usage with React tsx
47+
export function HereIsTheError() {
48+
>HereIsTheError : Symbol(HereIsTheError, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 18, 1))
49+
50+
return (
51+
<ComponentWithUnion
52+
>ComponentWithUnion : Symbol(ComponentWithUnion, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 14, 1))
53+
54+
multi={false}
55+
>multi : Symbol(multi, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 23, 27))
56+
57+
value={'s'}
58+
>value : Symbol(value, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 24, 25))
59+
60+
onChange={val => console.log(val)} // <- this throws an error
61+
>onChange : Symbol(onChange, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 25, 23))
62+
>val : Symbol(val, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 26, 22))
63+
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
64+
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
65+
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
66+
>val : Symbol(val, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 26, 22))
67+
68+
/>
69+
);
70+
}
71+
72+
// Usage with pure TypeScript
73+
ComponentWithUnion({
74+
>ComponentWithUnion : Symbol(ComponentWithUnion, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 14, 1))
75+
76+
multi: false,
77+
>multi : Symbol(multi, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 32, 20))
78+
79+
value: 's',
80+
>value : Symbol(value, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 33, 17))
81+
82+
onChange: val => console.log(val) // <- this works fine
83+
>onChange : Symbol(onChange, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 34, 15))
84+
>val : Symbol(val, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 35, 13))
85+
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
86+
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
87+
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
88+
>val : Symbol(val, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 35, 13))
89+
90+
});
91+

‎tests/baselines/reference/checkJsxUnionSFXContextualTypeInferredCorrectly.types

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
=== tests/cases/conformance/jsx/checkJsxUnionSFXContextualTypeInferredCorrectly.tsx ===
2+
/// <reference path="react16.d.ts" />
3+
4+
import React from 'react';
5+
>React : typeof React
6+
7+
interface PS {
8+
multi: false
9+
>multi : false
10+
>false : false
11+
12+
value: string | undefined
13+
>value : string | undefined
14+
15+
onChange: (selection: string | undefined) => void
16+
>onChange : (selection: string | undefined) => void
17+
>selection : string | undefined
18+
}
19+
20+
interface PM {
21+
multi: true
22+
>multi : true
23+
>true : true
24+
25+
value: string[]
26+
>value : string[]
27+
28+
onChange: (selection: string[]) => void
29+
>onChange : (selection: string[]) => void
30+
>selection : string[]
31+
}
32+
33+
export function ComponentWithUnion(props: PM | PS) {
34+
>ComponentWithUnion : (props: PS | PM) => JSX.Element
35+
>props : PS | PM
36+
37+
return <h1></h1>;
38+
><h1></h1> : JSX.Element
39+
>h1 : any
40+
>h1 : any
41+
}
42+
43+
// Usage with React tsx
44+
export function HereIsTheError() {
45+
>HereIsTheError : () => JSX.Element
46+
47+
return (
48+
>( <ComponentWithUnion multi={false} value={'s'} onChange={val => console.log(val)} // <- this throws an error /> ) : JSX.Element
49+
50+
<ComponentWithUnion
51+
><ComponentWithUnion multi={false} value={'s'} onChange={val => console.log(val)} // <- this throws an error /> : JSX.Element
52+
>ComponentWithUnion : (props: PS | PM) => JSX.Element
53+
54+
multi={false}
55+
>multi : false
56+
>false : false
57+
58+
value={'s'}
59+
>value : string
60+
>'s' : "s"
61+
62+
onChange={val => console.log(val)} // <- this throws an error
63+
>onChange : (val: string | undefined) => void
64+
>val => console.log(val) : (val: string | undefined) => void
65+
>val : string | undefined
66+
>console.log(val) : void
67+
>console.log : (message?: any, ...optionalParams: any[]) => void
68+
>console : Console
69+
>log : (message?: any, ...optionalParams: any[]) => void
70+
>val : string | undefined
71+
72+
/>
73+
);
74+
}
75+
76+
// Usage with pure TypeScript
77+
ComponentWithUnion({
78+
>ComponentWithUnion({ multi: false, value: 's', onChange: val => console.log(val) // <- this works fine}) : JSX.Element
79+
>ComponentWithUnion : (props: PS | PM) => JSX.Element
80+
>{ multi: false, value: 's', onChange: val => console.log(val) // <- this works fine} : { multi: false; value: string; onChange: (val: string | undefined) => void; }
81+
82+
multi: false,
83+
>multi : false
84+
>false : false
85+
86+
value: 's',
87+
>value : string
88+
>'s' : "s"
89+
90+
onChange: val => console.log(val) // <- this works fine
91+
>onChange : (val: string | undefined) => void
92+
>val => console.log(val) : (val: string | undefined) => void
93+
>val : string | undefined
94+
>console.log(val) : void
95+
>console.log : (message?: any, ...optionalParams: any[]) => void
96+
>console : Console
97+
>log : (message?: any, ...optionalParams: any[]) => void
98+
>val : string | undefined
99+
100+
});
101+

0 commit comments

Comments
 (0)