Skip to content

Fixed issue with spreading a generic call expression into generic JSX and gather intra expression inference sites from spread expressions #53444

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 3 commits into from
May 8, 2023
Merged

Fixed issue with spreading a generic call expression into generic JSX and gather intra expression inference sites from spread expressions #53444

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
a716293
Revert "Revert PR #51580 (#53283)"
Andarist Mar 22, 2023
7d58c78
Gather intra expression inference sites in spread expressions
Andarist Mar 22, 2023
0c72dc2
update old baseline
Andarist Mar 22, 2023
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
12 changes: 6 additions & 6 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30008,7 +30008,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return links.immediateTarget;
}

function checkObjectLiteral(node: ObjectLiteralExpression, checkMode?: CheckMode): Type {
function checkObjectLiteral(node: ObjectLiteralExpression, checkMode: CheckMode = CheckMode.Normal): Type {
const inDestructuringPattern = isAssignmentTarget(node);
// Grammar checking
checkGrammarObjectLiteralExpression(node, inDestructuringPattern);
Expand Down Expand Up @@ -30110,7 +30110,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
member = prop;
allPropertiesTable?.set(prop.escapedName, prop);

if (contextualType && checkMode && checkMode & CheckMode.Inferential && !(checkMode & CheckMode.SkipContextSensitive) &&
if (contextualType && checkMode & CheckMode.Inferential && !(checkMode & CheckMode.SkipContextSensitive) &&
(memberDecl.kind === SyntaxKind.PropertyAssignment || memberDecl.kind === SyntaxKind.MethodDeclaration) && isContextSensitive(memberDecl)) {
const inferenceContext = getInferenceContext(node);
Debug.assert(inferenceContext); // In CheckMode.Inferential we should always have an inference context
Expand All @@ -30130,7 +30130,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
hasComputedNumberProperty = false;
hasComputedSymbolProperty = false;
}
const type = getReducedType(checkExpression(memberDecl.expression));
const type = getReducedType(checkExpression(memberDecl.expression, checkMode & CheckMode.Inferential));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This forwards CheckMode.Inferential as that's a requirement for addIntraExpressionInferenceSite to be called

Copy link
Member

Choose a reason for hiding this comment

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

Dub question, but, why not just pass check mode alone? (I wish we made the parameter required everywhere...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That’s a question i can’t answer. I just figured out that the original issue was caused by the fact that it was passed down and that it was inconsistent with regular calls. So I unified both paths to work the same.

that other issue with spreads and intra expression inferences was accidentally discovered - it wasn’t exactly what was reported as broken for lack of this feature in JSX. It just was an example of a workaround and when I was fixing that issue I just copy pasted all of the examples from the issue. If not that, it’s likely that we would never know about this being broken - although in context of JSX there are some people who prefer to spread object literals (which is not common with regular calls, at least not in such a simple form). @weswigham mentioned in the comment that this case looked alright to him and that we don't want to break it… so here we are :)

if (isValidSpreadType(type)) {
const mergedType = tryMergeUnionOfObjectTypeAndEmptyObject(type, inConstContext);
if (allPropertiesTable) {
Expand Down Expand Up @@ -30330,7 +30330,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
* @remarks Because this function calls getSpreadType, it needs to use the same checks as checkObjectLiteral,
* which also calls getSpreadType.
*/
function createJsxAttributesTypeFromAttributesProperty(openingLikeElement: JsxOpeningLikeElement, checkMode: CheckMode | undefined) {
function createJsxAttributesTypeFromAttributesProperty(openingLikeElement: JsxOpeningLikeElement, checkMode: CheckMode = CheckMode.Normal) {
const attributes = openingLikeElement.attributes;
const contextualType = getContextualType(attributes, ContextFlags.None);
const allAttributesTable = strictNullChecks ? createSymbolTable() : undefined;
Expand Down Expand Up @@ -30367,7 +30367,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
addDeprecatedSuggestion(attributeDecl.name, prop.declarations, attributeDecl.name.escapedText as string);
}
}
if (contextualType && checkMode && checkMode & CheckMode.Inferential && !(checkMode & CheckMode.SkipContextSensitive) && isContextSensitive(attributeDecl)) {
if (contextualType && checkMode & CheckMode.Inferential && !(checkMode & CheckMode.SkipContextSensitive) && isContextSensitive(attributeDecl)) {
const inferenceContext = getInferenceContext(attributes);
Debug.assert(inferenceContext); // In CheckMode.Inferential we should always have an inference context
const inferenceNode = (attributeDecl.initializer as JsxExpression).expression!;
Expand All @@ -30380,7 +30380,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
spread = getSpreadType(spread, createJsxAttributesType(), attributes.symbol, objectFlags, /*readonly*/ false);
attributesTable = createSymbolTable();
}
const exprType = getReducedType(checkExpressionCached(attributeDecl.expression, checkMode));
const exprType = getReducedType(checkExpression(attributeDecl.expression, checkMode & CheckMode.Inferential));
if (isTypeAny(exprType)) {
hasSpreadAnyType = true;
}
Expand Down
17 changes: 16 additions & 1 deletion tests/baselines/reference/intraExpressionInferences.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,4 +212,19 @@ tests/cases/conformance/types/typeRelationships/typeInference/intraExpressionInf
let test1: "a" = u
}
})


interface Props<T> {
a: (x: string) => T;
b: (arg: T) => void;
}

declare function Foo<T>(props: Props<T>): null;

Foo({
...{
a: (x) => 10,
b: (arg) => {
arg.toString();
},
},
});
39 changes: 38 additions & 1 deletion tests/baselines/reference/intraExpressionInferences.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,37 @@ branch({
let test1: "a" = u
}
})


interface Props<T> {
a: (x: string) => T;
b: (arg: T) => void;
}

declare function Foo<T>(props: Props<T>): null;

Foo({
...{
a: (x) => 10,
b: (arg) => {
arg.toString();
},
},
});

//// [intraExpressionInferences.js]
"use strict";
// Repros from #47599
var __assign = (this && this.__assign) || function () {
__assign = Object.assign || function(t) {
for (var s, i = 1, n = arguments.length; i < n; i++) {
s = arguments[i];
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p))
t[p] = s[p];
}
return t;
};
return __assign.apply(this, arguments);
};
callIt({
produce: function () { return 0; },
consume: function (n) { return n.toFixed(); }
Expand Down Expand Up @@ -316,6 +342,12 @@ branch({
var test1 = u;
}
});
Foo(__assign({
a: function (x) { return 10; },
b: function (arg) {
arg.toString();
},
}));


//// [intraExpressionInferences.d.ts]
Expand Down Expand Up @@ -383,3 +415,8 @@ declare const branch: <T, U extends T>(_: {
then: (u: U) => void;
}) => void;
declare const x: "a" | "b";
interface Props<T> {
a: (x: string) => T;
b: (arg: T) => void;
}
declare function Foo<T>(props: Props<T>): null;
42 changes: 42 additions & 0 deletions tests/baselines/reference/intraExpressionInferences.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -626,3 +626,45 @@ branch({
}
})

interface Props<T> {
>Props : Symbol(Props, Decl(intraExpressionInferences.ts, 197, 2))
>T : Symbol(T, Decl(intraExpressionInferences.ts, 199, 16))

a: (x: string) => T;
>a : Symbol(Props.a, Decl(intraExpressionInferences.ts, 199, 20))
>x : Symbol(x, Decl(intraExpressionInferences.ts, 200, 6))
>T : Symbol(T, Decl(intraExpressionInferences.ts, 199, 16))

b: (arg: T) => void;
>b : Symbol(Props.b, Decl(intraExpressionInferences.ts, 200, 22))
>arg : Symbol(arg, Decl(intraExpressionInferences.ts, 201, 6))
>T : Symbol(T, Decl(intraExpressionInferences.ts, 199, 16))
}

declare function Foo<T>(props: Props<T>): null;
>Foo : Symbol(Foo, Decl(intraExpressionInferences.ts, 202, 1))
>T : Symbol(T, Decl(intraExpressionInferences.ts, 204, 21))
>props : Symbol(props, Decl(intraExpressionInferences.ts, 204, 24))
>Props : Symbol(Props, Decl(intraExpressionInferences.ts, 197, 2))
>T : Symbol(T, Decl(intraExpressionInferences.ts, 204, 21))

Foo({
>Foo : Symbol(Foo, Decl(intraExpressionInferences.ts, 202, 1))

...{
a: (x) => 10,
>a : Symbol(a, Decl(intraExpressionInferences.ts, 207, 6))
>x : Symbol(x, Decl(intraExpressionInferences.ts, 208, 8))

b: (arg) => {
>b : Symbol(b, Decl(intraExpressionInferences.ts, 208, 17))
>arg : Symbol(arg, Decl(intraExpressionInferences.ts, 209, 8))

arg.toString();
>arg.toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))
>arg : Symbol(arg, Decl(intraExpressionInferences.ts, 209, 8))
>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))

},
},
});
42 changes: 42 additions & 0 deletions tests/baselines/reference/intraExpressionInferences.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -659,3 +659,45 @@ branch({
}
})

interface Props<T> {
a: (x: string) => T;
>a : (x: string) => T
>x : string

b: (arg: T) => void;
>b : (arg: T) => void
>arg : T
}

declare function Foo<T>(props: Props<T>): null;
>Foo : <T>(props: Props<T>) => null
>props : Props<T>

Foo({
>Foo({ ...{ a: (x) => 10, b: (arg) => { arg.toString(); }, },}) : null
>Foo : <T>(props: Props<T>) => null
>{ ...{ a: (x) => 10, b: (arg) => { arg.toString(); }, },} : { a: (x: string) => number; b: (arg: number) => void; }

...{
>{ a: (x) => 10, b: (arg) => { arg.toString(); }, } : { a: (x: string) => number; b: (arg: number) => void; }

a: (x) => 10,
>a : (x: string) => number
>(x) => 10 : (x: string) => number
>x : string
>10 : 10

b: (arg) => {
>b : (arg: number) => void
>(arg) => { arg.toString(); } : (arg: number) => void
>arg : number

arg.toString();
>arg.toString() : string
>arg.toString : (radix?: number | undefined) => string
>arg : number
>toString : (radix?: number | undefined) => string

},
},
});
47 changes: 47 additions & 0 deletions tests/baselines/reference/jsxGenericComponentWithSpreadingResultOfGenericFunction.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
=== tests/cases/compiler/jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx ===
/// <reference path="react16.d.ts" />

// repro #51577

declare function omit<T, K extends string>(names: readonly K[], obj: T): Omit<T, K>;
>omit : Symbol(omit, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 0, 0), Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 4, 84))
>T : Symbol(T, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 4, 22))
>K : Symbol(K, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 4, 24))
>names : Symbol(names, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 4, 43))
>K : Symbol(K, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 4, 24))
>obj : Symbol(obj, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 4, 63))
>T : Symbol(T, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 4, 22))
>Omit : Symbol(Omit, Decl(lib.es5.d.ts, --, --))
>T : Symbol(T, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 4, 22))
>K : Symbol(K, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 4, 24))

declare function omit<K extends string>(names: readonly K[]): <T>(obj: T) => Omit<T, K>;
>omit : Symbol(omit, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 0, 0), Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 4, 84))
>K : Symbol(K, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 5, 22))
>names : Symbol(names, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 5, 40))
>K : Symbol(K, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 5, 22))
>T : Symbol(T, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 5, 63))
>obj : Symbol(obj, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 5, 66))
>T : Symbol(T, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 5, 63))
>Omit : Symbol(Omit, Decl(lib.es5.d.ts, --, --))
>T : Symbol(T, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 5, 63))
>K : Symbol(K, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 5, 22))

declare const otherProps: { bar: string, qwe: boolean }
>otherProps : Symbol(otherProps, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 7, 13))
>bar : Symbol(bar, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 7, 27))
>qwe : Symbol(qwe, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 7, 40))

declare function GenericComponent<T>(props: T): null
>GenericComponent : Symbol(GenericComponent, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 7, 55))
>T : Symbol(T, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 9, 34))
>props : Symbol(props, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 9, 37))
>T : Symbol(T, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 9, 34))

<GenericComponent {...omit(['bar'], otherProps)} />; // no error
>GenericComponent : Symbol(GenericComponent, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 7, 55))
>omit : Symbol(omit, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 0, 0), Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 4, 84))
>otherProps : Symbol(otherProps, Decl(jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx, 7, 13))



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

// repro #51577

declare function omit<T, K extends string>(names: readonly K[], obj: T): Omit<T, K>;
>omit : { <T, K extends string>(names: readonly K[], obj: T): Omit<T, K>; <K extends string>(names: readonly K[]): <T>(obj: T) => Omit<T, K>; }
>names : readonly K[]
>obj : T

declare function omit<K extends string>(names: readonly K[]): <T>(obj: T) => Omit<T, K>;
>omit : { <T, K extends string>(names: readonly K[], obj: T): Omit<T, K>; <K extends string>(names: readonly K[]): <T>(obj: T) => Omit<T, K>; }
>names : readonly K[]
>obj : T

declare const otherProps: { bar: string, qwe: boolean }
>otherProps : { bar: string; qwe: boolean; }
>bar : string
>qwe : boolean

declare function GenericComponent<T>(props: T): null
>GenericComponent : <T>(props: T) => null
>props : T

<GenericComponent {...omit(['bar'], otherProps)} />; // no error
><GenericComponent {...omit(['bar'], otherProps)} /> : JSX.Element
>GenericComponent : <T>(props: T) => null
>omit(['bar'], otherProps) : Omit<{ bar: string; qwe: boolean; }, "bar">
>omit : { <T, K extends string>(names: readonly K[], obj: T): Omit<T, K>; <K extends string>(names: readonly K[]): <T>(obj: T) => Omit<T, K>; }
>['bar'] : "bar"[]
>'bar' : "bar"
>otherProps : { bar: string; qwe: boolean; }



18 changes: 18 additions & 0 deletions tests/cases/compiler/jsxGenericComponentWithSpreadingResultOfGenericFunction.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// @strict: true
// @noEmit: true
// @jsx: preserve

/// <reference path="/.lib/react16.d.ts" />

// repro #51577

declare function omit<T, K extends string>(names: readonly K[], obj: T): Omit<T, K>;
declare function omit<K extends string>(names: readonly K[]): <T>(obj: T) => Omit<T, K>;

declare const otherProps: { bar: string, qwe: boolean }

declare function GenericComponent<T>(props: T): null

<GenericComponent {...omit(['bar'], otherProps)} />; // no error


16 changes: 16 additions & 0 deletions tests/cases/conformance/types/typeRelationships/typeInference/intraExpressionInferences.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,19 @@ branch({
let test1: "a" = u
}
})

interface Props<T> {
a: (x: string) => T;
b: (arg: T) => void;
}

declare function Foo<T>(props: Props<T>): null;

Foo({
...{
a: (x) => 10,
b: (arg) => {
arg.toString();
},
},
});