From 07dbd8be216d1ddd057dac835de7f6ec6a12feb7 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Fri, 5 Oct 2018 15:11:12 -0700 Subject: [PATCH] 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 --- src/compiler/checker.ts | 92 +++++++++------- ...UnionSFXContextualTypeInferredCorrectly.js | 63 +++++++++++ ...SFXContextualTypeInferredCorrectly.symbols | 91 ++++++++++++++++ ...onSFXContextualTypeInferredCorrectly.types | 101 ++++++++++++++++++ ...nionSFXContextualTypeInferredCorrectly.tsx | 40 +++++++ 5 files changed, 349 insertions(+), 38 deletions(-) create mode 100644 tests/baselines/reference/checkJsxUnionSFXContextualTypeInferredCorrectly.js create mode 100644 tests/baselines/reference/checkJsxUnionSFXContextualTypeInferredCorrectly.symbols create mode 100644 tests/baselines/reference/checkJsxUnionSFXContextualTypeInferredCorrectly.types create mode 100644 tests/cases/conformance/jsx/checkJsxUnionSFXContextualTypeInferredCorrectly.tsx diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 7d15aa64b8c..ba0b180b71f 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -11640,27 +11640,14 @@ namespace ts { // Keep this up-to-date with the same logic within `getApparentTypeOfContextualType`, since they should behave similarly function findMatchingDiscriminantType(source: Type, target: UnionOrIntersectionType) { - let match: Type | undefined; const sourceProperties = getPropertiesOfObjectType(source); if (sourceProperties) { const sourcePropertiesFiltered = findDiscriminantProperties(sourceProperties, target); if (sourcePropertiesFiltered) { - for (const sourceProperty of sourcePropertiesFiltered) { - const sourceType = getTypeOfSymbol(sourceProperty); - for (const type of target.types) { - const targetType = getTypeOfPropertyOfType(type, sourceProperty.escapedName); - if (targetType && isRelatedTo(sourceType, targetType)) { - if (type === match) continue; // Finding multiple fields which discriminate to the same type is fine - if (match) { - return undefined; - } - match = type; - } - } - } + return discriminateTypeByDiscriminableItems(target, map(sourcePropertiesFiltered, p => ([() => getTypeOfSymbol(p), p.escapedName] as [() => Type, __String])), isRelatedTo); } } - return match; + return undefined; } function typeRelatedToEachType(source: Type, target: IntersectionType, reportErrors: boolean): Ternary { @@ -12475,6 +12462,25 @@ namespace ts { } } + function discriminateTypeByDiscriminableItems(target: UnionType, discriminators: [() => Type, __String][], related: (source: Type, target: Type) => boolean | Ternary): Type | undefined; + function discriminateTypeByDiscriminableItems(target: UnionType, discriminators: [() => Type, __String][], related: (source: Type, target: Type) => boolean | Ternary, defaultValue: Type): Type; + function discriminateTypeByDiscriminableItems(target: UnionType, discriminators: [() => Type, __String][], related: (source: Type, target: Type) => boolean | Ternary, defaultValue?: Type) { + let match: Type | undefined; + for (const [getDiscriminatingType, propertyName] of discriminators) { + for (const type of target.types) { + const targetType = getTypeOfPropertyOfType(type, propertyName); + if (targetType && related(getDiscriminatingType(), targetType)) { + if (match) { + if (type === match) continue; // Finding multiple fields which discriminate to the same type is fine + return defaultValue; + } + match = type; + } + } + } + return match || defaultValue; + } + /** * A type is 'weak' if it is an object type with at least one optional property * and no required properties, call/construct signatures or index signatures @@ -14187,7 +14193,7 @@ namespace ts { if ((prop).isDiscriminantProperty === undefined) { (prop).isDiscriminantProperty = !!((prop).checkFlags & CheckFlags.HasNonUniformType) && isLiteralType(getTypeOfSymbol(prop)); } - return (prop).isDiscriminantProperty; + return !!(prop).isDiscriminantProperty; } } return false; @@ -16762,43 +16768,53 @@ namespace ts { case SyntaxKind.FalseKeyword: case SyntaxKind.NullKeyword: case SyntaxKind.Identifier: + case SyntaxKind.UndefinedKeyword: return true; case SyntaxKind.PropertyAccessExpression: case SyntaxKind.ParenthesizedExpression: return isPossiblyDiscriminantValue((node).expression); + case SyntaxKind.JsxExpression: + return !(node as JsxExpression).expression || isPossiblyDiscriminantValue((node as JsxExpression).expression!); } return false; } + function discriminateContextualTypeByObjectMembers(node: ObjectLiteralExpression, contextualType: UnionType) { + return discriminateTypeByDiscriminableItems(contextualType, + map( + filter(node.properties, p => !!p.symbol && p.kind === SyntaxKind.PropertyAssignment && isPossiblyDiscriminantValue(p.initializer) && isDiscriminantProperty(contextualType, p.symbol.escapedName)), + prop => ([() => checkExpression((prop as PropertyAssignment).initializer), prop.symbol.escapedName] as [() => Type, __String]) + ), + isTypeAssignableTo, + contextualType + ); + } + + function discriminateContextualTypeByJSXAttributes(node: JsxAttributes, contextualType: UnionType) { + return discriminateTypeByDiscriminableItems(contextualType, + map( + filter(node.properties, p => !!p.symbol && p.kind === SyntaxKind.JsxAttribute && isDiscriminantProperty(contextualType, p.symbol.escapedName) && (!p.initializer || isPossiblyDiscriminantValue(p.initializer))), + prop => ([!(prop as JsxAttribute).initializer ? (() => trueType) : (() => checkExpression((prop as JsxAttribute).initializer!)), prop.symbol.escapedName] as [() => Type, __String]) + ), + isTypeAssignableTo, + contextualType + ); + } + // Return the contextual type for a given expression node. During overload resolution, a contextual type may temporarily // be "pushed" onto a node using the contextualType property. function getApparentTypeOfContextualType(node: Expression): Type | undefined { let contextualType = getContextualType(node); contextualType = contextualType && mapType(contextualType, getApparentType); - if (!(contextualType && contextualType.flags & TypeFlags.Union && isObjectLiteralExpression(node))) { - return contextualType; - } - // Keep the below up-to-date with the work done within `isRelatedTo` by `findMatchingDiscriminantType` - let match: Type | undefined; - propLoop: for (const prop of node.properties) { - if (!prop.symbol) continue; - if (prop.kind !== SyntaxKind.PropertyAssignment) continue; - if (isPossiblyDiscriminantValue(prop.initializer) && isDiscriminantProperty(contextualType, prop.symbol.escapedName)) { - const discriminatingType = checkExpression(prop.initializer); - for (const type of (contextualType as UnionType).types) { - const targetType = getTypeOfPropertyOfType(type, prop.symbol.escapedName); - if (targetType && isTypeAssignableTo(discriminatingType, targetType)) { - if (match) { - if (type === match) continue; // Finding multiple fields which discriminate to the same type is fine - match = undefined; - break propLoop; - } - match = type; - } - } + if (contextualType && contextualType.flags & TypeFlags.Union) { + if (isObjectLiteralExpression(node)) { + return discriminateContextualTypeByObjectMembers(node, contextualType as UnionType); + } + else if (isJsxAttributes(node)) { + return discriminateContextualTypeByJSXAttributes(node, contextualType as UnionType); } } - return match || contextualType; + return contextualType; } /** diff --git a/tests/baselines/reference/checkJsxUnionSFXContextualTypeInferredCorrectly.js b/tests/baselines/reference/checkJsxUnionSFXContextualTypeInferredCorrectly.js new file mode 100644 index 00000000000..44778eb69e2 --- /dev/null +++ b/tests/baselines/reference/checkJsxUnionSFXContextualTypeInferredCorrectly.js @@ -0,0 +1,63 @@ +//// [checkJsxUnionSFXContextualTypeInferredCorrectly.tsx] +/// + +import React from 'react'; + +interface PS { + multi: false + value: string | undefined + onChange: (selection: string | undefined) => void +} + +interface PM { + multi: true + value: string[] + onChange: (selection: string[]) => void +} + +export function ComponentWithUnion(props: PM | PS) { + return

; +} + +// Usage with React tsx +export function HereIsTheError() { + return ( + console.log(val)} // <- this throws an error + /> + ); +} + +// Usage with pure TypeScript +ComponentWithUnion({ + multi: false, + value: 's', + onChange: val => console.log(val) // <- this works fine +}); + + +//// [checkJsxUnionSFXContextualTypeInferredCorrectly.js] +"use strict"; +/// +var __importDefault = (this && this.__importDefault) || function (mod) { + return (mod && mod.__esModule) ? mod : { "default": mod }; +}; +exports.__esModule = true; +var react_1 = __importDefault(require("react")); +function ComponentWithUnion(props) { + return react_1["default"].createElement("h1", null); +} +exports.ComponentWithUnion = ComponentWithUnion; +// Usage with React tsx +function HereIsTheError() { + return (react_1["default"].createElement(ComponentWithUnion, { multi: false, value: 's', onChange: function (val) { return console.log(val); } })); +} +exports.HereIsTheError = HereIsTheError; +// Usage with pure TypeScript +ComponentWithUnion({ + multi: false, + value: 's', + onChange: function (val) { return console.log(val); } // <- this works fine +}); diff --git a/tests/baselines/reference/checkJsxUnionSFXContextualTypeInferredCorrectly.symbols b/tests/baselines/reference/checkJsxUnionSFXContextualTypeInferredCorrectly.symbols new file mode 100644 index 00000000000..13675eb985d --- /dev/null +++ b/tests/baselines/reference/checkJsxUnionSFXContextualTypeInferredCorrectly.symbols @@ -0,0 +1,91 @@ +=== tests/cases/conformance/jsx/checkJsxUnionSFXContextualTypeInferredCorrectly.tsx === +/// + +import React from 'react'; +>React : Symbol(React, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 2, 6)) + +interface PS { +>PS : Symbol(PS, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 2, 26)) + + multi: false +>multi : Symbol(PS.multi, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 4, 14)) + + value: string | undefined +>value : Symbol(PS.value, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 5, 16)) + + onChange: (selection: string | undefined) => void +>onChange : Symbol(PS.onChange, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 6, 29)) +>selection : Symbol(selection, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 7, 15)) +} + +interface PM { +>PM : Symbol(PM, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 8, 1)) + + multi: true +>multi : Symbol(PM.multi, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 10, 14)) + + value: string[] +>value : Symbol(PM.value, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 11, 15)) + + onChange: (selection: string[]) => void +>onChange : Symbol(PM.onChange, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 12, 19)) +>selection : Symbol(selection, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 13, 15)) +} + +export function ComponentWithUnion(props: PM | PS) { +>ComponentWithUnion : Symbol(ComponentWithUnion, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 14, 1)) +>props : Symbol(props, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 16, 35)) +>PM : Symbol(PM, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 8, 1)) +>PS : Symbol(PS, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 2, 26)) + + return

; +>h1 : Symbol(JSX.IntrinsicElements.h1, Decl(react16.d.ts, 2430, 106)) +>h1 : Symbol(JSX.IntrinsicElements.h1, Decl(react16.d.ts, 2430, 106)) +} + +// Usage with React tsx +export function HereIsTheError() { +>HereIsTheError : Symbol(HereIsTheError, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 18, 1)) + + return ( + ComponentWithUnion : Symbol(ComponentWithUnion, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 14, 1)) + + multi={false} +>multi : Symbol(multi, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 23, 27)) + + value={'s'} +>value : Symbol(value, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 24, 25)) + + onChange={val => console.log(val)} // <- this throws an error +>onChange : Symbol(onChange, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 25, 23)) +>val : Symbol(val, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 26, 22)) +>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --)) +>console : Symbol(console, Decl(lib.dom.d.ts, --, --)) +>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --)) +>val : Symbol(val, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 26, 22)) + + /> + ); +} + +// Usage with pure TypeScript +ComponentWithUnion({ +>ComponentWithUnion : Symbol(ComponentWithUnion, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 14, 1)) + + multi: false, +>multi : Symbol(multi, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 32, 20)) + + value: 's', +>value : Symbol(value, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 33, 17)) + + onChange: val => console.log(val) // <- this works fine +>onChange : Symbol(onChange, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 34, 15)) +>val : Symbol(val, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 35, 13)) +>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --)) +>console : Symbol(console, Decl(lib.dom.d.ts, --, --)) +>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --)) +>val : Symbol(val, Decl(checkJsxUnionSFXContextualTypeInferredCorrectly.tsx, 35, 13)) + +}); + diff --git a/tests/baselines/reference/checkJsxUnionSFXContextualTypeInferredCorrectly.types b/tests/baselines/reference/checkJsxUnionSFXContextualTypeInferredCorrectly.types new file mode 100644 index 00000000000..a5318a5c195 --- /dev/null +++ b/tests/baselines/reference/checkJsxUnionSFXContextualTypeInferredCorrectly.types @@ -0,0 +1,101 @@ +=== tests/cases/conformance/jsx/checkJsxUnionSFXContextualTypeInferredCorrectly.tsx === +/// + +import React from 'react'; +>React : typeof React + +interface PS { + multi: false +>multi : false +>false : false + + value: string | undefined +>value : string | undefined + + onChange: (selection: string | undefined) => void +>onChange : (selection: string | undefined) => void +>selection : string | undefined +} + +interface PM { + multi: true +>multi : true +>true : true + + value: string[] +>value : string[] + + onChange: (selection: string[]) => void +>onChange : (selection: string[]) => void +>selection : string[] +} + +export function ComponentWithUnion(props: PM | PS) { +>ComponentWithUnion : (props: PS | PM) => JSX.Element +>props : PS | PM + + return

; +>

: JSX.Element +>h1 : any +>h1 : any +} + +// Usage with React tsx +export function HereIsTheError() { +>HereIsTheError : () => JSX.Element + + return ( +>( console.log(val)} // <- this throws an error /> ) : JSX.Element + + console.log(val)} // <- this throws an error /> : JSX.Element +>ComponentWithUnion : (props: PS | PM) => JSX.Element + + multi={false} +>multi : false +>false : false + + value={'s'} +>value : string +>'s' : "s" + + onChange={val => console.log(val)} // <- this throws an error +>onChange : (val: string | undefined) => void +>val => console.log(val) : (val: string | undefined) => void +>val : string | undefined +>console.log(val) : void +>console.log : (message?: any, ...optionalParams: any[]) => void +>console : Console +>log : (message?: any, ...optionalParams: any[]) => void +>val : string | undefined + + /> + ); +} + +// Usage with pure TypeScript +ComponentWithUnion({ +>ComponentWithUnion({ multi: false, value: 's', onChange: val => console.log(val) // <- this works fine}) : JSX.Element +>ComponentWithUnion : (props: PS | PM) => JSX.Element +>{ multi: false, value: 's', onChange: val => console.log(val) // <- this works fine} : { multi: false; value: string; onChange: (val: string | undefined) => void; } + + multi: false, +>multi : false +>false : false + + value: 's', +>value : string +>'s' : "s" + + onChange: val => console.log(val) // <- this works fine +>onChange : (val: string | undefined) => void +>val => console.log(val) : (val: string | undefined) => void +>val : string | undefined +>console.log(val) : void +>console.log : (message?: any, ...optionalParams: any[]) => void +>console : Console +>log : (message?: any, ...optionalParams: any[]) => void +>val : string | undefined + +}); + diff --git a/tests/cases/conformance/jsx/checkJsxUnionSFXContextualTypeInferredCorrectly.tsx b/tests/cases/conformance/jsx/checkJsxUnionSFXContextualTypeInferredCorrectly.tsx new file mode 100644 index 00000000000..7d4f6071e81 --- /dev/null +++ b/tests/cases/conformance/jsx/checkJsxUnionSFXContextualTypeInferredCorrectly.tsx @@ -0,0 +1,40 @@ +// @jsx: react +// @strict: true +// @esModuleInterop: true +/// + +import React from 'react'; + +interface PS { + multi: false + value: string | undefined + onChange: (selection: string | undefined) => void +} + +interface PM { + multi: true + value: string[] + onChange: (selection: string[]) => void +} + +export function ComponentWithUnion(props: PM | PS) { + return

; +} + +// Usage with React tsx +export function HereIsTheError() { + return ( + console.log(val)} // <- this throws an error + /> + ); +} + +// Usage with pure TypeScript +ComponentWithUnion({ + multi: false, + value: 's', + onChange: val => console.log(val) // <- this works fine +});