Skip to content

Unify JSX And Normal Call Checking Codepaths #27627

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

Merged
merged 7 commits into from
Oct 17, 2018
Merged

Unify JSX And Normal Call Checking Codepaths #27627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3240f41
Unify JSX Call Checking Codepaths
weswigham Oct 4, 2018
071b91b
Add tests for fixed issues
weswigham Oct 8, 2018
f280b6b
Fix lint, move all error checking into the only-run-once resolveSigna…
weswigham Oct 9, 2018
97df58e
Remove unused (unreachable?) code path
weswigham Oct 9, 2018
c51c496
Consolidate a little more duplicated logic into signature checking
weswigham Oct 9, 2018
1ebbaa1
Fix #19775 a bit more
weswigham Oct 9, 2018
d83f463
Cosmetic changes from CR
weswigham Oct 11, 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
787 changes: 223 additions & 564 deletions src/compiler/checker.ts
View file
Open in desktop

Large diffs are not rendered by default.

5 changes: 1 addition & 4 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3103,8 +3103,6 @@ namespace ts {
getExportsOfModule(moduleSymbol: Symbol): Symbol[];
/** Unlike `getExportsOfModule`, this includes properties of an `export =` value. */
/* @internal */ getExportsAndPropertiesOfModule(moduleSymbol: Symbol): Symbol[];

getAllAttributesTypeFromJsxOpeningLikeElement(elementNode: JsxOpeningLikeElement): Type | undefined;
getJsxIntrinsicTagNamesAt(location: Node): Symbol[];
isOptionalParameter(node: ParameterDeclaration): boolean;
getAmbientModules(): Symbol[];
Expand Down Expand Up @@ -3171,7 +3169,7 @@ namespace ts {
* True if `contextualType` should not be considered for completions because
* e.g. it specifies `kind: "a"` and obj has `kind: "b"`.
*/
/* @internal */ isTypeInvalidDueToUnionDiscriminant(contextualType: Type, obj: ObjectLiteralExpression): boolean;
/* @internal */ isTypeInvalidDueToUnionDiscriminant(contextualType: Type, obj: ObjectLiteralExpression | JsxAttributes): boolean;
/**
* For a union, will include a property if it's defined in *any* of the member types.
* So for `{ a } | { b }`, this will include both `a` and `b`.
Expand Down Expand Up @@ -3769,7 +3767,6 @@ namespace ts {
resolvedType?: Type; // Cached type of type node
resolvedEnumType?: Type; // Cached constraint type from enum jsdoc tag
resolvedSignature?: Signature; // Cached signature of signature node or call expression
resolvedSignatures?: Map<Signature[]>; // Cached signatures of jsx node
resolvedSymbol?: Symbol; // Cached name resolution result
resolvedIndexInfo?: IndexInfo; // Cached indexing info resolution result
maybeTypePredicate?: boolean; // Cached check whether call expression might reference a type predicate
Expand Down
6 changes: 3 additions & 3 deletions src/services/completions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1200,9 +1200,9 @@ namespace ts.Completions {
function tryGetJsxCompletionSymbols(): GlobalsSearch {
const jsxContainer = tryGetContainingJsxElement(contextToken);
// Cursor is inside a JSX self-closing element or opening element
const attrsType = jsxContainer && typeChecker.getAllAttributesTypeFromJsxOpeningLikeElement(jsxContainer);
const attrsType = jsxContainer && typeChecker.getContextualType(jsxContainer.attributes);
if (!attrsType) return GlobalsSearch.Continue;
symbols = filterJsxAttributes(typeChecker.getPropertiesOfType(attrsType), jsxContainer!.attributes.properties);
symbols = filterJsxAttributes(getPropertiesForObjectExpression(attrsType, jsxContainer!.attributes, typeChecker), jsxContainer!.attributes.properties);
completionKind = CompletionKind.MemberLike;
isNewIdentifierLocation = false;
return GlobalsSearch.Success;
Expand Down Expand Up @@ -2211,7 +2211,7 @@ namespace ts.Completions {
return jsdoc && jsdoc.tags && (rangeContainsPosition(jsdoc, position) ? findLast(jsdoc.tags, tag => tag.pos < position) : undefined);
}

function getPropertiesForObjectExpression(contextualType: Type, obj: ObjectLiteralExpression, checker: TypeChecker): Symbol[] {
function getPropertiesForObjectExpression(contextualType: Type, obj: ObjectLiteralExpression | JsxAttributes, checker: TypeChecker): Symbol[] {
return contextualType.isUnion()
? checker.getAllPossiblePropertiesOfTypes(contextualType.types.filter(memberType =>
// If we're providing completions for an object literal, skip primitive, array-like, or callable types since those shouldn't be implemented by object literals.
Expand Down
1 change: 0 additions & 1 deletion tests/baselines/reference/api/tsserverlibrary.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1941,7 +1941,6 @@ declare namespace ts {
/** Follow all aliases to get the original symbol. */
getAliasedSymbol(symbol: Symbol): Symbol;
getExportsOfModule(moduleSymbol: Symbol): Symbol[];
getAllAttributesTypeFromJsxOpeningLikeElement(elementNode: JsxOpeningLikeElement): Type | undefined;
getJsxIntrinsicTagNamesAt(location: Node): Symbol[];
isOptionalParameter(node: ParameterDeclaration): boolean;
getAmbientModules(): Symbol[];
Expand Down
1 change: 0 additions & 1 deletion tests/baselines/reference/api/typescript.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1941,7 +1941,6 @@ declare namespace ts {
/** Follow all aliases to get the original symbol. */
getAliasedSymbol(symbol: Symbol): Symbol;
getExportsOfModule(moduleSymbol: Symbol): Symbol[];
getAllAttributesTypeFromJsxOpeningLikeElement(elementNode: JsxOpeningLikeElement): Type | undefined;
getJsxIntrinsicTagNamesAt(location: Node): Symbol[];
isOptionalParameter(node: ParameterDeclaration): boolean;
getAmbientModules(): Symbol[];
Expand Down
18 changes: 10 additions & 8 deletions tests/baselines/reference/checkJsxChildrenProperty14.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
tests/cases/conformance/jsx/file.tsx(42,11): error TS2322: Type '{ children: Element[]; a: number; b: string; }' is not assignable to type 'SingleChildProp'.
Types of property 'children' are incompatible.
Type 'Element[]' is not assignable to type 'Element'.
Property 'type' is missing in type 'Element[]'.
tests/cases/conformance/jsx/file.tsx(42,11): error TS2322: Type '{ children: Element[]; a: number; b: string; }' is not assignable to type 'IntrinsicAttributes & SingleChildProp'.
Type '{ children: Element[]; a: number; b: string; }' is not assignable to type 'SingleChildProp'.
Types of property 'children' are incompatible.
Type 'Element[]' is not assignable to type 'Element'.
Property 'type' is missing in type 'Element[]'.


==== tests/cases/conformance/jsx/file.tsx (1 errors) ====
Expand Down Expand Up @@ -48,7 +49,8 @@ tests/cases/conformance/jsx/file.tsx(42,11): error TS2322: Type '{ children: Ele
// Error
let k5 = <SingleChildComp a={10} b="hi"><></><Button /><AnotherButton /></SingleChildComp>;
~~~~~~~~~~~~~~~
!!! error TS2322: Type '{ children: Element[]; a: number; b: string; }' is not assignable to type 'SingleChildProp'.
!!! error TS2322: Types of property 'children' are incompatible.
!!! error TS2322: Type 'Element[]' is not assignable to type 'Element'.
!!! error TS2322: Property 'type' is missing in type 'Element[]'.
!!! error TS2322: Type '{ children: Element[]; a: number; b: string; }' is not assignable to type 'IntrinsicAttributes & SingleChildProp'.
!!! error TS2322: Type '{ children: Element[]; a: number; b: string; }' is not assignable to type 'SingleChildProp'.
Copy link
Member

Choose a reason for hiding this comment

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

I think it was special-case code that got rid of this line in the first place. Is it possible that it just needs to be moved or updated to work again?

Copy link
Member Author

Choose a reason for hiding this comment

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

As I mentioned in person, this is a result of bringing SFC checking back into line and doing the right thing - there was no "special case" removing the jsx types from the errors; they just straight up weren't being checked against!

!!! error TS2322: Types of property 'children' are incompatible.
!!! error TS2322: Type 'Element[]' is not assignable to type 'Element'.
!!! error TS2322: Property 'type' is missing in type 'Element[]'.
98 changes: 54 additions & 44 deletions tests/baselines/reference/checkJsxChildrenProperty2.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,31 @@
tests/cases/conformance/jsx/file.tsx(14,10): error TS2322: Type '{ a: number; b: string; }' is not assignable to type 'Prop'.
Property 'children' is missing in type '{ a: number; b: string; }'.
tests/cases/conformance/jsx/file.tsx(14,10): error TS2322: Type '{ a: number; b: string; }' is not assignable to type 'IntrinsicAttributes & Prop'.
Type '{ a: number; b: string; }' is not assignable to type 'Prop'.
Property 'children' is missing in type '{ a: number; b: string; }'.
tests/cases/conformance/jsx/file.tsx(17,11): error TS2710: 'children' are specified twice. The attribute named 'children' will be overwritten.
tests/cases/conformance/jsx/file.tsx(31,6): error TS2322: Type '{ children: (Element | ((name: string) => Element))[]; a: number; b: string; }' is not assignable to type 'Prop'.
Types of property 'children' are incompatible.
Type '(Element | ((name: string) => Element))[]' is not assignable to type 'string | Element'.
Type '(Element | ((name: string) => Element))[]' is not assignable to type 'Element'.
Property 'type' is missing in type '(Element | ((name: string) => Element))[]'.
tests/cases/conformance/jsx/file.tsx(37,6): error TS2322: Type '{ children: (number | Element)[]; a: number; b: string; }' is not assignable to type 'Prop'.
Types of property 'children' are incompatible.
Type '(number | Element)[]' is not assignable to type 'string | Element'.
Type '(number | Element)[]' is not assignable to type 'Element'.
Property 'type' is missing in type '(number | Element)[]'.
tests/cases/conformance/jsx/file.tsx(43,6): error TS2322: Type '{ children: (string | Element)[]; a: number; b: string; }' is not assignable to type 'Prop'.
Types of property 'children' are incompatible.
Type '(string | Element)[]' is not assignable to type 'string | Element'.
Type '(string | Element)[]' is not assignable to type 'Element'.
Property 'type' is missing in type '(string | Element)[]'.
tests/cases/conformance/jsx/file.tsx(49,6): error TS2322: Type '{ children: Element[]; a: number; b: string; }' is not assignable to type 'Prop'.
Types of property 'children' are incompatible.
Type 'Element[]' is not assignable to type 'string | Element'.
Type 'Element[]' is not assignable to type 'Element'.
Property 'type' is missing in type 'Element[]'.
tests/cases/conformance/jsx/file.tsx(31,6): error TS2322: Type '{ children: (Element | ((name: string) => Element))[]; a: number; b: string; }' is not assignable to type 'IntrinsicAttributes & Prop'.
Type '{ children: (Element | ((name: string) => Element))[]; a: number; b: string; }' is not assignable to type 'Prop'.
Types of property 'children' are incompatible.
Type '(Element | ((name: string) => Element))[]' is not assignable to type 'string | Element'.
Type '(Element | ((name: string) => Element))[]' is not assignable to type 'Element'.
Property 'type' is missing in type '(Element | ((name: string) => Element))[]'.
tests/cases/conformance/jsx/file.tsx(37,6): error TS2322: Type '{ children: (number | Element)[]; a: number; b: string; }' is not assignable to type 'IntrinsicAttributes & Prop'.
Type '{ children: (number | Element)[]; a: number; b: string; }' is not assignable to type 'Prop'.
Types of property 'children' are incompatible.
Type '(number | Element)[]' is not assignable to type 'string | Element'.
Type '(number | Element)[]' is not assignable to type 'Element'.
Property 'type' is missing in type '(number | Element)[]'.
tests/cases/conformance/jsx/file.tsx(43,6): error TS2322: Type '{ children: (string | Element)[]; a: number; b: string; }' is not assignable to type 'IntrinsicAttributes & Prop'.
Type '{ children: (string | Element)[]; a: number; b: string; }' is not assignable to type 'Prop'.
Types of property 'children' are incompatible.
Type '(string | Element)[]' is not assignable to type 'string | Element'.
Type '(string | Element)[]' is not assignable to type 'Element'.
Property 'type' is missing in type '(string | Element)[]'.
tests/cases/conformance/jsx/file.tsx(49,6): error TS2322: Type '{ children: Element[]; a: number; b: string; }' is not assignable to type 'IntrinsicAttributes & Prop'.
Type '{ children: Element[]; a: number; b: string; }' is not assignable to type 'Prop'.
Types of property 'children' are incompatible.
Type 'Element[]' is not assignable to type 'string | Element'.
Type 'Element[]' is not assignable to type 'Element'.
Property 'type' is missing in type 'Element[]'.


==== tests/cases/conformance/jsx/file.tsx (6 errors) ====
Expand All @@ -39,8 +44,9 @@ tests/cases/conformance/jsx/file.tsx(49,6): error TS2322: Type '{ children: Elem
// Error: missing children
let k = <Comp a={10} b="hi" />;
~~~~
!!! error TS2322: Type '{ a: number; b: string; }' is not assignable to type 'Prop'.
!!! error TS2322: Property 'children' is missing in type '{ a: number; b: string; }'.
!!! error TS2322: Type '{ a: number; b: string; }' is not assignable to type 'IntrinsicAttributes & Prop'.
!!! error TS2322: Type '{ a: number; b: string; }' is not assignable to type 'Prop'.
!!! error TS2322: Property 'children' is missing in type '{ a: number; b: string; }'.

let k0 =
<Comp a={10} b="hi" children="Random" >
Expand All @@ -61,47 +67,51 @@ tests/cases/conformance/jsx/file.tsx(49,6): error TS2322: Type '{ children: Elem
let k2 =
<Comp a={10} b="hi">
~~~~
!!! error TS2322: Type '{ children: (Element | ((name: string) => Element))[]; a: number; b: string; }' is not assignable to type 'Prop'.
!!! error TS2322: Types of property 'children' are incompatible.
!!! error TS2322: Type '(Element | ((name: string) => Element))[]' is not assignable to type 'string | Element'.
!!! error TS2322: Type '(Element | ((name: string) => Element))[]' is not assignable to type 'Element'.
!!! error TS2322: Property 'type' is missing in type '(Element | ((name: string) => Element))[]'.
!!! error TS2322: Type '{ children: (Element | ((name: string) => Element))[]; a: number; b: string; }' is not assignable to type 'IntrinsicAttributes & Prop'.
!!! error TS2322: Type '{ children: (Element | ((name: string) => Element))[]; a: number; b: string; }' is not assignable to type 'Prop'.
!!! error TS2322: Types of property 'children' are incompatible.
!!! error TS2322: Type '(Element | ((name: string) => Element))[]' is not assignable to type 'string | Element'.
!!! error TS2322: Type '(Element | ((name: string) => Element))[]' is not assignable to type 'Element'.
!!! error TS2322: Property 'type' is missing in type '(Element | ((name: string) => Element))[]'.
<div> My Div </div>
{(name: string) => <div> My name {name} </div>}
</Comp>;

let k3 =
<Comp a={10} b="hi">
~~~~
!!! error TS2322: Type '{ children: (number | Element)[]; a: number; b: string; }' is not assignable to type 'Prop'.
!!! error TS2322: Types of property 'children' are incompatible.
!!! error TS2322: Type '(number | Element)[]' is not assignable to type 'string | Element'.
!!! error TS2322: Type '(number | Element)[]' is not assignable to type 'Element'.
!!! error TS2322: Property 'type' is missing in type '(number | Element)[]'.
!!! error TS2322: Type '{ children: (number | Element)[]; a: number; b: string; }' is not assignable to type 'IntrinsicAttributes & Prop'.
!!! error TS2322: Type '{ children: (number | Element)[]; a: number; b: string; }' is not assignable to type 'Prop'.
!!! error TS2322: Types of property 'children' are incompatible.
!!! error TS2322: Type '(number | Element)[]' is not assignable to type 'string | Element'.
!!! error TS2322: Type '(number | Element)[]' is not assignable to type 'Element'.
!!! error TS2322: Property 'type' is missing in type '(number | Element)[]'.
<div> My Div </div>
{1000000}
</Comp>;

let k4 =
<Comp a={10} b="hi" >
~~~~
!!! error TS2322: Type '{ children: (string | Element)[]; a: number; b: string; }' is not assignable to type 'Prop'.
!!! error TS2322: Types of property 'children' are incompatible.
!!! error TS2322: Type '(string | Element)[]' is not assignable to type 'string | Element'.
!!! error TS2322: Type '(string | Element)[]' is not assignable to type 'Element'.
!!! error TS2322: Property 'type' is missing in type '(string | Element)[]'.
!!! error TS2322: Type '{ children: (string | Element)[]; a: number; b: string; }' is not assignable to type 'IntrinsicAttributes & Prop'.
!!! error TS2322: Type '{ children: (string | Element)[]; a: number; b: string; }' is not assignable to type 'Prop'.
!!! error TS2322: Types of property 'children' are incompatible.
!!! error TS2322: Type '(string | Element)[]' is not assignable to type 'string | Element'.
!!! error TS2322: Type '(string | Element)[]' is not assignable to type 'Element'.
!!! error TS2322: Property 'type' is missing in type '(string | Element)[]'.
<div> My Div </div>
hi hi hi!
</Comp>;

let k5 =
<Comp a={10} b="hi" >
~~~~
!!! error TS2322: Type '{ children: Element[]; a: number; b: string; }' is not assignable to type 'Prop'.
!!! error TS2322: Types of property 'children' are incompatible.
!!! error TS2322: Type 'Element[]' is not assignable to type 'string | Element'.
!!! error TS2322: Type 'Element[]' is not assignable to type 'Element'.
!!! error TS2322: Property 'type' is missing in type 'Element[]'.
!!! error TS2322: Type '{ children: Element[]; a: number; b: string; }' is not assignable to type 'IntrinsicAttributes & Prop'.
!!! error TS2322: Type '{ children: Element[]; a: number; b: string; }' is not assignable to type 'Prop'.
!!! error TS2322: Types of property 'children' are incompatible.
!!! error TS2322: Type 'Element[]' is not assignable to type 'string | Element'.
!!! error TS2322: Type 'Element[]' is not assignable to type 'Element'.
!!! error TS2322: Property 'type' is missing in type 'Element[]'.
<div> My Div </div>
<div> My Div </div>
</Comp>;
Loading