Skip to content
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.

Sign up for GitHub

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

Introduce more caching and deferral into jsx checking #25302

Merged
merged 3 commits into from
Jun 28, 2018
Merged

Introduce more caching and deferral into jsx checking #25302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a560420
Introduce more caching and deferral into jsx checking
weswigham Jun 28, 2018
b924b1d
Merge branch 'master' into improve-jsx-perf
weswigham Jun 28, 2018
fe10aaf
Accept baseline with removed duplicated error
weswigham Jun 28, 2018
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
48 changes: 37 additions & 11 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16365,14 +16365,18 @@ namespace ts {
type.flags & TypeFlags.UnionOrIntersection && every((<UnionOrIntersectionType>type).types, isValidSpreadType));
}

function checkJsxSelfClosingElement(node: JsxSelfClosingElement, checkMode: CheckMode | undefined): Type {
checkJsxOpeningLikeElementOrOpeningFragment(node, checkMode);
function checkJsxSelfClosingElementDeferred(node: JsxSelfClosingElement) {
checkJsxOpeningLikeElementOrOpeningFragment(node, CheckMode.Normal);
}

function checkJsxSelfClosingElement(node: JsxSelfClosingElement, _checkMode: CheckMode | undefined): Type {
checkNodeDeferred(node);
return getJsxElementTypeAt(node) || anyType;
}

function checkJsxElement(node: JsxElement, checkMode: CheckMode | undefined): Type {
function checkJsxElementDeferred(node: JsxElement) {
// Check attributes
checkJsxOpeningLikeElementOrOpeningFragment(node.openingElement, checkMode);
checkJsxOpeningLikeElementOrOpeningFragment(node.openingElement, CheckMode.Normal);

// Perform resolution on the closing tag so that rename/go to definition/etc work
if (isJsxIntrinsicIdentifier(node.closingElement.tagName)) {
Expand All @@ -16381,6 +16385,10 @@ namespace ts {
else {
checkExpression(node.closingElement.tagName);
}
}

function checkJsxElement(node: JsxElement, _checkMode: CheckMode | undefined): Type {
Copy link
Member

@RyanCavanaugh RyanCavanaugh Jun 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Safe to remove _checkMode now?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but I'll need to add it back in a week or two when I add more specific return types for jsx elements and once again track down all the places I need to flow it through again. I figured I'd just leave it here for later.

Copy link
Member

Choose a reason for hiding this comment

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

Might deserve a comment in case Andy sees it and removes it 😉

checkNodeDeferred(node);

return getJsxElementTypeAt(node) || anyType;
}
Expand Down Expand Up @@ -16661,12 +16669,24 @@ namespace ts {
}

function getJsxNamespaceAt(location: Node | undefined): Symbol {
const namespaceName = getJsxNamespace(location);
const resolvedNamespace = resolveName(location, namespaceName, SymbolFlags.Namespace, /*diagnosticMessage*/ undefined, namespaceName, /*isUse*/ false);
if (resolvedNamespace) {
const candidate = getSymbol(getExportsOfSymbol(resolveSymbol(resolvedNamespace)), JsxNames.JSX, SymbolFlags.Namespace);
if (candidate) {
return candidate;
const links = location && getNodeLinks(location);
if (links && links.jsxNamespace) {
return links.jsxNamespace;
}
if (!links || links.jsxNamespace !== false) {
const namespaceName = getJsxNamespace(location);
const resolvedNamespace = resolveName(location, namespaceName, SymbolFlags.Namespace, /*diagnosticMessage*/ undefined, namespaceName, /*isUse*/ false);
if (resolvedNamespace) {
const candidate = getSymbol(getExportsOfSymbol(resolveSymbol(resolvedNamespace)), JsxNames.JSX, SymbolFlags.Namespace);
if (candidate) {
if (links) {
links.jsxNamespace = candidate;
}
return candidate;
}
if (links) {
links.jsxNamespace = false;
}
}
}
// JSX global fallback
Expand Down Expand Up @@ -17146,7 +17166,7 @@ namespace ts {
// sourceAttributesType is a type of an attributes properties.
// i.e <div attr1={10} attr2="string" />
// attr1 and attr2 are treated as JSXAttributes attached in the JsxOpeningLikeElement as "attributes".
const sourceAttributesType = createJsxAttributesTypeFromAttributesProperty(openingLikeElement, checkMode);
const sourceAttributesType = checkExpressionCached(openingLikeElement.attributes, checkMode);

// Check if sourceAttributesType assignable to targetAttributesType though this check will allow excess properties
const isSourceAttributeTypeAssignableToTarget = isTypeAssignableTo(sourceAttributesType, targetAttributesType);
Expand Down Expand Up @@ -26115,6 +26135,12 @@ namespace ts {
case SyntaxKind.ClassExpression:
checkClassExpressionDeferred(<ClassExpression>node);
break;
case SyntaxKind.JsxSelfClosingElement:
checkJsxSelfClosingElementDeferred(<JsxSelfClosingElement>node);
break;
case SyntaxKind.JsxElement:
checkJsxElementDeferred(<JsxElement>node);
break;
}
});
}
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3658,6 +3658,7 @@ namespace ts {
hasSuperCall?: boolean; // recorded result when we try to find super-call. We only try to find one if this flag is undefined, indicating that we haven't made an attempt.
superCall?: SuperCall; // Cached first super-call found in the constructor. Used in checking whether super is called before this-accessing
switchTypes?: Type[]; // Cached array of switch case expression types
jsxNamespace?: Symbol | false; // Resolved jsx namespace symbol for this node
}

export const enum TypeFlags {
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3196,6 +3196,7 @@ declare namespace ts {
hasSuperCall?: boolean;
superCall?: SuperCall;
switchTypes?: Type[];
jsxNamespace?: Symbol | false;
}
enum TypeFlags {
Any = 1,
Expand Down
5 changes: 1 addition & 4 deletions tests/baselines/reference/inlineJsxFactoryDeclarationsLocalTypes.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ tests/cases/conformance/jsx/inline/index.tsx(21,22): error TS2322: Type '{ child
Types of property 'children' are incompatible.
Type 'import("tests/cases/conformance/jsx/inline/renderer").dom.JSX.Element[]' is not assignable to type 'import("tests/cases/conformance/jsx/inline/renderer2").predom.JSX.Element[]'.
Type 'import("tests/cases/conformance/jsx/inline/renderer").dom.JSX.Element' is not assignable to type 'import("tests/cases/conformance/jsx/inline/renderer2").predom.JSX.Element'.
tests/cases/conformance/jsx/inline/index.tsx(21,40): error TS2605: JSX element type 'MyClass' is not a constructor function for JSX elements.
tests/cases/conformance/jsx/inline/index.tsx(21,40): error TS2605: JSX element type 'MyClass' is not a constructor function for JSX elements.
Property '__domBrand' is missing in type 'MyClass'.
tests/cases/conformance/jsx/inline/index.tsx(21,63): error TS2605: JSX element type 'MyClass' is not a constructor function for JSX elements.
Expand Down Expand Up @@ -77,7 +76,7 @@ tests/cases/conformance/jsx/inline/index.tsx(24,23): error TS2322: Type '{ child

export default <h></h>

==== tests/cases/conformance/jsx/inline/index.tsx (7 errors) ====
==== tests/cases/conformance/jsx/inline/index.tsx (6 errors) ====
/** @jsx dom */
import { dom } from "./renderer"
import prerendered, {MySFC, MyClass, tree} from "./component";
Expand Down Expand Up @@ -111,8 +110,6 @@ tests/cases/conformance/jsx/inline/index.tsx(24,23): error TS2322: Type '{ child
!!! error TS2322: Type 'import("tests/cases/conformance/jsx/inline/renderer").dom.JSX.Element[]' is not assignable to type 'import("tests/cases/conformance/jsx/inline/renderer2").predom.JSX.Element[]'.
!!! error TS2322: Type 'import("tests/cases/conformance/jsx/inline/renderer").dom.JSX.Element' is not assignable to type 'import("tests/cases/conformance/jsx/inline/renderer2").predom.JSX.Element'.
~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2605: JSX element type 'MyClass' is not a constructor function for JSX elements.
~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2605: JSX element type 'MyClass' is not a constructor function for JSX elements.
!!! error TS2605: Property '__domBrand' is missing in type 'MyClass'.
~~~~~~~~~~~~~~~~~~~~~~~
Expand Down