-
Notifications
You must be signed in to change notification settings - Fork 12.9k
Back out from reducing template literal types with no extra texts too eagerly #58703
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18087,14 +18087,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |
if (contains(types, wildcardType)) { | ||
return wildcardType; | ||
} | ||
if ( | ||
texts.length === 2 && texts[0] === "" && texts[1] === "" | ||
// literals (including string enums) are stringified below | ||
&& !(types[0].flags & TypeFlags.Literal) | ||
// infer T extends StringLike can't be unwrapped eagerly | ||
&& !types[0].symbol?.declarations?.some(d => d.parent.kind === SyntaxKind.InferType) | ||
&& isTypeAssignableTo(types[0], stringType) | ||
) { | ||
if (texts.length === 2 && texts[0] === "" && texts[1] === "" && (types[0].flags & TypeFlags.StringMapping)) { | ||
return types[0]; | ||
} | ||
const newTypes: Type[] = []; | ||
|
@@ -25580,6 +25573,14 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |
return false; | ||
} | ||
|
||
function isTemplateLiteralTypeWithSinglePlaceholderAndNoExtraTexts(type: Type): type is TemplateLiteralType { | ||
if (!(type.flags & TypeFlags.TemplateLiteral)) { | ||
return false; | ||
} | ||
const texts = (type as TemplateLiteralType).texts; | ||
return texts.length === 2 && texts[0] === "" && texts[1] === ""; | ||
} | ||
|
||
function isValidTypeForTemplateLiteralPlaceholder(source: Type, target: Type): boolean { | ||
if (target.flags & TypeFlags.Intersection) { | ||
return every((target as IntersectionType).types, t => t === emptyTypeLiteralType || isValidTypeForTemplateLiteralPlaceholder(source, t)); | ||
|
@@ -26217,6 +26218,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |
} | ||
|
||
function inferToTemplateLiteralType(source: Type, target: TemplateLiteralType) { | ||
if (isTemplateLiteralTypeWithSinglePlaceholderAndNoExtraTexts(source) && isTemplateLiteralTypeWithSinglePlaceholderAndNoExtraTexts(target)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm still considering if this shouldn't have some extra rules on top of this to be completely safe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not understanding why this needs to be here. After reverting #55371 the following works just fine: declare function bar<T extends string>(x: `${T}`): T;
function baz<U extends string>(x: `${U}`) {
return bar(x); // Infers U
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I confused myself which one of those errored after reverting #55371: interface TypeMap {
a: 'A'
b: 'B'
}
declare const f: <T extends 'a' | 'b'>(x: `${T}`) => TypeMap[T];
type F1 = <T extends 'a' | 'b'>(x: `${T}`) => TypeMap[T];
const f1: F1 = f; // Ok
type F2 = <T extends 'a' | 'b'>(x: `${T}`) => TypeMap[`${T}`];
const f2: F2 = f; // Error, T is not assignable to `${T}` Then I just knew where I could try to improve inference for this as I knew that in here i agree, this PR is completely redundant 🤦♂️ this is what I get for jumping too quickly between work, this, and the kids There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I know the feeling. Your efforts are always appreciated! |
||
inferFromTypes(source.types[0], target.types[0]); | ||
return; | ||
} | ||
const matches = inferTypesFromTemplateLiteralType(source, target); | ||
const types = target.types; | ||
// When the target template literal contains only placeholders (meaning that inference is intended to extract | ||
|
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
templateLiteralTypes5.ts(10,7): error TS2322: Type '<T0 extends "a" | "b">(x: `${T0}`) => TypeMap[T0]' is not assignable to type 'F2'. | ||
Type 'TypeMap[T2]' is not assignable to type 'TypeMap[`${T2}`]'. | ||
Type 'T2' is not assignable to type '`${T2}`'. | ||
Type '"a" | "b"' is not assignable to type '`${T2}`'. | ||
Type '"a"' is not assignable to type '`${T2}`'. | ||
templateLiteralTypes5.ts(13,11): error TS2322: Type 'T3' is not assignable to type '`${T3}`'. | ||
Type '"a" | "b"' is not assignable to type '`${T3}`'. | ||
Type '"a"' is not assignable to type '`${T3}`'. | ||
templateLiteralTypes5.ts(14,11): error TS2322: Type '`${T3}`' is not assignable to type 'T3'. | ||
'`${T3}`' is assignable to the constraint of type 'T3', but 'T3' could be instantiated with a different subtype of constraint '"a" | "b"'. | ||
Type '"a" | "b"' is not assignable to type 'T3'. | ||
'"a" | "b"' is assignable to the constraint of type 'T3', but 'T3' could be instantiated with a different subtype of constraint '"a" | "b"'. | ||
Type '"a"' is not assignable to type 'T3'. | ||
'"a"' is assignable to the constraint of type 'T3', but 'T3' could be instantiated with a different subtype of constraint '"a" | "b"'. | ||
|
||
|
||
==== templateLiteralTypes5.ts (3 errors) ==== | ||
// https://github.com/microsoft/TypeScript/issues/55364 | ||
interface TypeMap { | ||
a: "A"; | ||
b: "B"; | ||
} | ||
declare const f: <T0 extends "a" | "b">(x: `${T0}`) => TypeMap[T0]; | ||
type F1 = <T1 extends "a" | "b">(x: `${T1}`) => TypeMap[T1]; | ||
const f1: F1 = f; | ||
type F2 = <T2 extends 'a' | 'b'>(x: `${T2}`) => TypeMap[`${T2}`] | ||
const f2: F2 = f | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. before #55371 this was OK but the assignment above was not. The one above being an error was really spooky as both functions (source and the target there) were syntactically identical. This line here doesn't come from the user report per se but rather it was created in the process of analyzing the error reported previously for that first assignment (since the error mentioned template literal type at that changed position in the indexed access). I think it still might be possible to fix this case here, string like index types in the indexed accesses are stringified: enum E { a = "a" }
type Foo = { [E.a]: 1 } // type Foo = { a: 1; } So it feels like this fact could be leveraged to simplify such indexed access types like here |
||
~~ | ||
!!! error TS2322: Type '<T0 extends "a" | "b">(x: `${T0}`) => TypeMap[T0]' is not assignable to type 'F2'. | ||
!!! error TS2322: Type 'TypeMap[T2]' is not assignable to type 'TypeMap[`${T2}`]'. | ||
!!! error TS2322: Type 'T2' is not assignable to type '`${T2}`'. | ||
!!! error TS2322: Type '"a" | "b"' is not assignable to type '`${T2}`'. | ||
!!! error TS2322: Type '"a"' is not assignable to type '`${T2}`'. | ||
|
||
function f3<T3 extends "a" | "b">(x: T3) { | ||
const test1: `${T3}` = x; // error, T3 could be instantiated with a string enum | ||
~~~~~ | ||
!!! error TS2322: Type 'T3' is not assignable to type '`${T3}`'. | ||
!!! error TS2322: Type '"a" | "b"' is not assignable to type '`${T3}`'. | ||
!!! error TS2322: Type '"a"' is not assignable to type '`${T3}`'. | ||
const test2: T3 = "" as `${T3}`; // error, T3 could be instantiated with a string enum | ||
~~~~~ | ||
!!! error TS2322: Type '`${T3}`' is not assignable to type 'T3'. | ||
!!! error TS2322: '`${T3}`' is assignable to the constraint of type 'T3', but 'T3' could be instantiated with a different subtype of constraint '"a" | "b"'. | ||
!!! error TS2322: Type '"a" | "b"' is not assignable to type 'T3'. | ||
!!! error TS2322: '"a" | "b"' is assignable to the constraint of type 'T3', but 'T3' could be instantiated with a different subtype of constraint '"a" | "b"'. | ||
!!! error TS2322: Type '"a"' is not assignable to type 'T3'. | ||
!!! error TS2322: '"a"' is assignable to the constraint of type 'T3', but 'T3' could be instantiated with a different subtype of constraint '"a" | "b"'. | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
//// [tests/cases/conformance/types/literal/templateLiteralTypes8.ts] //// | ||
|
||
=== templateLiteralTypes8.ts === | ||
const enum E { | ||
>E : Symbol(E, Decl(templateLiteralTypes8.ts, 0, 0)) | ||
|
||
a = "a", | ||
>a : Symbol(E.a, Decl(templateLiteralTypes8.ts, 0, 14)) | ||
|
||
b = "b", | ||
>b : Symbol(E.b, Decl(templateLiteralTypes8.ts, 1, 10)) | ||
} | ||
|
||
type Stringify<T extends string> = `${T}`; | ||
>Stringify : Symbol(Stringify, Decl(templateLiteralTypes8.ts, 3, 1)) | ||
>T : Symbol(T, Decl(templateLiteralTypes8.ts, 5, 15)) | ||
>T : Symbol(T, Decl(templateLiteralTypes8.ts, 5, 15)) | ||
|
||
let z1: `${E}` = "a"; | ||
>z1 : Symbol(z1, Decl(templateLiteralTypes8.ts, 7, 3)) | ||
>E : Symbol(E, Decl(templateLiteralTypes8.ts, 0, 0)) | ||
|
||
let z2: Stringify<E> = "a"; | ||
>z2 : Symbol(z2, Decl(templateLiteralTypes8.ts, 8, 3)) | ||
>Stringify : Symbol(Stringify, Decl(templateLiteralTypes8.ts, 3, 1)) | ||
>E : Symbol(E, Decl(templateLiteralTypes8.ts, 0, 0)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
//// [tests/cases/conformance/types/literal/templateLiteralTypes8.ts] //// | ||
|
||
=== templateLiteralTypes8.ts === | ||
const enum E { | ||
>E : E | ||
> : ^ | ||
|
||
a = "a", | ||
>a : E.a | ||
> : ^^^ | ||
>"a" : "a" | ||
> : ^^^ | ||
|
||
b = "b", | ||
>b : E.b | ||
> : ^^^ | ||
>"b" : "b" | ||
> : ^^^ | ||
} | ||
|
||
type Stringify<T extends string> = `${T}`; | ||
>Stringify : `${T}` | ||
> : ^^^^^^ | ||
|
||
let z1: `${E}` = "a"; | ||
>z1 : "a" | "b" | ||
> : ^^^^^^^^^ | ||
>"a" : "a" | ||
> : ^^^ | ||
|
||
let z2: Stringify<E> = "a"; | ||
>z2 : "a" | "b" | ||
> : ^^^^^^^^^ | ||
>"a" : "a" | ||
> : ^^^ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
// @strict: true | ||
// @noEmit: true | ||
|
||
const enum E { | ||
a = "a", | ||
b = "b", | ||
} | ||
|
||
type Stringify<T extends string> = `${T}`; | ||
|
||
let z1: `${E}` = "a"; | ||
let z2: Stringify<E> = "a"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reasoning for keeping the reduction around for string mapping types? Does it fix a known issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, it just keeps the behavior from #55371 as this one seems like a safe reduction and i thought that it might be worth avoiding a new type identity in a case like this