Skip to content

Strongly typecheck unions of intrinsic tag names #28557

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 22 additions & 20 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18345,16 +18345,31 @@ namespace ts {
return getNameFromJsxElementAttributesContainer(JsxNames.ElementChildrenAttributeNameContainer, jsxNamespace);
}

function getUninstantiatedJsxSignaturesOfType(elementType: Type): ReadonlyArray<Signature> {
function getUninstantiatedJsxSignaturesOfType(elementType: Type, caller: JsxOpeningLikeElement): ReadonlyArray<Signature> {
if (elementType.flags & TypeFlags.String) {
return [anySignature];
}
else if (elementType.flags & TypeFlags.StringLiteral) {
const intrinsicType = getIntrinsicAttributesTypeFromStringLiteralType(elementType as StringLiteralType, caller);
if (!intrinsicType) {
error(caller, Diagnostics.Property_0_does_not_exist_on_type_1, (elementType as StringLiteralType).value, "JSX." + JsxNames.IntrinsicElements);
return emptyArray;
}
else {
const fakeSignature = createSignatureForJSXIntrinsic(caller, intrinsicType);
return [fakeSignature];
}
}
const apparentElemType = getApparentType(elementType);
// Resolve the signatures, preferring constructor
let signatures = getSignaturesOfType(elementType, SignatureKind.Construct);
let signatures = getSignaturesOfType(apparentElemType, SignatureKind.Construct);
if (signatures.length === 0) {
// No construct signatures, try call signatures
signatures = getSignaturesOfType(elementType, SignatureKind.Call);
signatures = getSignaturesOfType(apparentElemType, SignatureKind.Call);
}
if (signatures.length === 0 && elementType.flags & TypeFlags.Union) {
if (signatures.length === 0 && apparentElemType.flags & TypeFlags.Union) {
// If each member has some combination of new/call signatures; make a union signature list for those
signatures = getUnionSignatures(map((elementType as UnionType).types, getUninstantiatedJsxSignaturesOfType));
signatures = getUnionSignatures(map((apparentElemType as UnionType).types, t => getUninstantiatedJsxSignaturesOfType(t, caller)));
}
return signatures;
}
Expand Down Expand Up @@ -20537,21 +20552,8 @@ namespace ts {
return resolveErrorCall(node);
}

if (exprTypes.flags & TypeFlags.StringLiteral) {
const intrinsicType = getIntrinsicAttributesTypeFromStringLiteralType(exprTypes as StringLiteralType, node);
if (!intrinsicType) {
error(node, Diagnostics.Property_0_does_not_exist_on_type_1, (exprTypes as StringLiteralType).value, "JSX." + JsxNames.IntrinsicElements);
return resolveUntypedCall(node);
}
else {
const fakeSignature = createSignatureForJSXIntrinsic(node, intrinsicType);
checkTypeAssignableToAndOptionallyElaborate(checkExpressionWithContextualType(node.attributes, getEffectiveFirstArgumentForJsxSignature(fakeSignature, node), /*mapper*/ undefined), intrinsicType, node.tagName, node.attributes);
return fakeSignature;
}
}

const signatures = getUninstantiatedJsxSignaturesOfType(apparentType);
if (exprTypes.flags & TypeFlags.String || isUntypedFunctionCall(exprTypes, apparentType, signatures.length, /*constructSignatures*/ 0)) {
const signatures = getUninstantiatedJsxSignaturesOfType(exprTypes, node);
if (isUntypedFunctionCall(exprTypes, apparentType, signatures.length, /*constructSignatures*/ 0)) {
return resolveUntypedCall(node);
}

Expand Down
17 changes: 17 additions & 0 deletions tests/baselines/reference/jsxIntrinsicUnions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//// [jsxIntrinsicUnions.tsx]
/// <reference path="/.lib/react16.d.ts" />

import * as React from "react";

const El = Math.random() ? 'h1' : 'h2';

const tag = <El className="ok" key="key">{"Title"}</El>;


//// [jsxIntrinsicUnions.js]
"use strict";
/// <reference path="react16.d.ts" />
exports.__esModule = true;
var React = require("react");
var El = Math.random() ? 'h1' : 'h2';
var tag = React.createElement(El, { className: "ok", key: "key" }, "Title");
19 changes: 19 additions & 0 deletions tests/baselines/reference/jsxIntrinsicUnions.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
=== tests/cases/compiler/jsxIntrinsicUnions.tsx ===
/// <reference path="react16.d.ts" />

import * as React from "react";
>React : Symbol(React, Decl(jsxIntrinsicUnions.tsx, 2, 6))

const El = Math.random() ? 'h1' : 'h2';
>El : Symbol(El, Decl(jsxIntrinsicUnions.tsx, 4, 5))
>Math.random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --))
>Math : Symbol(Math, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
>random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --))

const tag = <El className="ok" key="key">{"Title"}</El>;
>tag : Symbol(tag, Decl(jsxIntrinsicUnions.tsx, 6, 5))
>El : Symbol(El, Decl(jsxIntrinsicUnions.tsx, 4, 5))
>className : Symbol(className, Decl(jsxIntrinsicUnions.tsx, 6, 15))
>key : Symbol(key, Decl(jsxIntrinsicUnions.tsx, 6, 30))
>El : Symbol(El, Decl(jsxIntrinsicUnions.tsx, 4, 5))

25 changes: 25 additions & 0 deletions tests/baselines/reference/jsxIntrinsicUnions.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
=== tests/cases/compiler/jsxIntrinsicUnions.tsx ===
/// <reference path="react16.d.ts" />

import * as React from "react";
>React : typeof React

const El = Math.random() ? 'h1' : 'h2';
>El : "h1" | "h2"
>Math.random() ? 'h1' : 'h2' : "h1" | "h2"
>Math.random() : number
>Math.random : () => number
>Math : Math
>random : () => number
>'h1' : "h1"
>'h2' : "h2"

const tag = <El className="ok" key="key">{"Title"}</El>;
>tag : JSX.Element
><El className="ok" key="key">{"Title"}</El> : JSX.Element
>El : "h1" | "h2"
>className : string
>key : string
>"Title" : "Title"
>El : "h1" | "h2"

7 changes: 5 additions & 2 deletions tests/baselines/reference/tsxDynamicTagName3.errors.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
tests/cases/conformance/jsx/tsxDynamicTagName3.tsx(9,1): error TS2339: Property 'h1' does not exist on type 'JSX.IntrinsicElements'.
tests/cases/conformance/jsx/tsxDynamicTagName3.tsx(9,2): error TS2604: JSX element type 'CustomTag' does not have any construct or call signatures.


==== tests/cases/conformance/jsx/tsxDynamicTagName3.tsx (1 errors) ====
==== tests/cases/conformance/jsx/tsxDynamicTagName3.tsx (2 errors) ====
declare module JSX {
interface Element { }
interface IntrinsicElements {
Expand All @@ -12,4 +13,6 @@ tests/cases/conformance/jsx/tsxDynamicTagName3.tsx(9,1): error TS2339: Property
var CustomTag: "h1" = "h1";
<CustomTag> Hello World </CustomTag> // This should be an error. we will try look up string literal type in JSX.IntrinsicElements
~~~~~~~~~~~
!!! error TS2339: Property 'h1' does not exist on type 'JSX.IntrinsicElements'.
!!! error TS2339: Property 'h1' does not exist on type 'JSX.IntrinsicElements'.
~~~~~~~~~
!!! error TS2604: JSX element type 'CustomTag' does not have any construct or call signatures.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously we'd bail early rather than issuing both messages here. I could probably contort it a bit to only issue one of the two errors; but I feel like having both is slightly more descriptive, actually.

8 changes: 8 additions & 0 deletions tests/cases/compiler/jsxIntrinsicUnions.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// @jsx: react
/// <reference path="/.lib/react16.d.ts" />

import * as React from "react";

const El = Math.random() ? 'h1' : 'h2';

const tag = <El className="ok" key="key">{"Title"}</El>;