-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Support relations and inference between template literal types #43361
Conversation
const matches: Type[] = []; | ||
let seg = 0; | ||
let pos = targetStartText.length; | ||
for (let i = 1; i < lastTargetIndex; i++) { |
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.
Some sort of comment (an overview of the algorithm / what it's trying to do, or the name of it if this is a well-known one?) seems justified here
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.
Yes, I think if you could just explain with examples too, that'd be helpful.
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.
Ok, I'll add some comments.
} | ||
|
||
function getStringLikeTypeForType(type: Type) { | ||
return type.flags & (TypeFlags.Any | TypeFlags.StringLike) ? type : getTemplateLiteralType(["", ""], [type]); |
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.
Would it be useful to have a singleton emptyTemplateType
?
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.
Well, the single-element types
array varies here, so we can't use a singleton. We could possibly have a singleton for the ["", ""]
array, but it's hardly worth the effort as this code doesn't run that often.
return type.flags & (TypeFlags.Any | TypeFlags.StringLike) ? type : getTemplateLiteralType(["", ""], [type]); | ||
} | ||
|
||
function inferFromLiteralPartsToTemplateLiteral(sourceTexts: readonly string[], sourceTypes: readonly Type[], target: TemplateLiteralType): Type[] | undefined { |
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.
I don't like calling these sourceTexts
because it immediately signals to me "BUG" even though you're not actually grabbing out the source text. You're grabbing the normalized text content which is the correct thing to do.
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.
Hmm, I don't think the current names are that bad. After all the properties are called texts
and types
in template literals and we want the parameters to reflect the relation.
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.
Right, I think that even texts
would be a better name - I may send a follow-up PR but I'm going to merge this for now.
With this PR we support relations and inference between template literal types. Specifically, when the target side in a type relation is a template literal type, the source side is now permitted to be a compatible (i.e. more specific) template literal type. Likewise, when inferring to a template literal target type, we now permit the source type to also be a template literal type.
Some examples of improved assignment relations:
With this PR all of the above assignments are permitted, where previously only the first assignment was permitted.
Some examples of inference between template literal types:
Fixes #43060.
Fixes #43243.