Skip to content

Back out from reducing template literal types with no extra texts too eagerly - #58703

Closed
Mateusz Burzyński (Andarist) wants to merge 1 commit into
microsoft:mainfrom
Andarist:fix/58687
Closed

Back out from reducing template literal types with no extra texts too eagerly#58703
Mateusz Burzyński (Andarist) wants to merge 1 commit into
microsoft:mainfrom
Andarist:fix/58687

Conversation

@Andarist

Copy link
Copy Markdown
Contributor

fixes#58687

@typescript-botTypeScript Bot (typescript-bot) added the For Milestone Bug PRs that fix a bug with a specific milestone label May 29, 2024
}

function inferToTemplateLiteralType(source: Type, target: TemplateLiteralType) {
if (isTemplateLiteralTypeWithSinglePlaceholderAndNoExtraTexts(source) && isTemplateLiteralTypeWithSinglePlaceholderAndNoExtraTexts(target)) {

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The 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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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:

declarefunctionbar<Textendsstring>(x: `${T}`): T;functionbaz<Uextendsstring>(x: `${U}`){returnbar(x);// Infers U}

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Hmm, I confused myself which one of those errored after reverting #55371:

interfaceTypeMap{a: 'A'b: 'B'}declareconstf: <Textends'a'|'b'>(x: `${T}`)=>TypeMap[T];typeF1=<Textends'a'|'b'>(x: `${T}`)=>TypeMap[T];constf1: F1=f;// OktypeF2=<Textends'a'|'b'>(x: `${T}`)=>TypeMap[`${T}`];constf2: 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 `${T}` was being inferred for T based on `${T}` source and `${T}` target as that originally created an issue further down the road.

i agree, this PR is completely redundant 🤦‍♂️ this is what I get for jumping too quickly between work, this, and the kids

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh, I know the feeling. Your efforts are always appreciated!

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

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The 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:

enumE{a="a"}typeFoo={[E.a]: 1}// type Foo = { a: 1; }

So it feels like this fact could be leveraged to simplify such indexed access types like here

&& !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)) {

Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
ContributorAuthor

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

@gabritto

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) pack this

@typescript-bot

TypeScript Bot (typescript-bot) commented May 29, 2024

Copy link
Copy Markdown
Contributor

Starting jobs; this comment will be updated as builds start and complete.

CommandStatusResults
pack this✅ Started✅ Results

@typescript-bot

TypeScript Bot (typescript-bot) commented May 29, 2024

Copy link
Copy Markdown
Contributor

Hey Gabriela Araujo Britto (@gabritto), I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
"devDependencies": {
"typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/161980/artifacts?artifactName=tgz&fileId=86CFC79DB1B3075B303FC2ECF3E214166144F62CDFF52640B4CAF8573A9BE41502&fileName=/typescript-5.5.0-insiders.20240529.tgz"
}
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@5.5.0-pr-58703-2".;

@microsoftMicrosoft (microsoft) locked as resolved and limited conversation to collaborators Oct 16, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

For Milestone BugPRs that fix a bug with a specific milestone

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Template literal type reduction is inconsistent (regression in 5.5-beta)

4 participants

@Andarist@gabritto@typescript-bot@ahejlsberg