-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Improve comparable relation of template literal types to detect more impossible situations #57872
Comments
Bisects to #46137 |
That PR adds this code if (relation === comparableRelation) {
return templateLiteralTypesDefinitelyUnrelated(source as TemplateLiteralType, target as TemplateLiteralType) ? Ternary.False : Ternary.True;
} where function templateLiteralTypesDefinitelyUnrelated(source: TemplateLiteralType, target: TemplateLiteralType) {
// Two template literal types with diffences in their starting or ending text spans are definitely unrelated.
const sourceStart = source.texts[0];
const targetStart = target.texts[0];
const sourceEnd = source.texts[source.texts.length - 1];
const targetEnd = target.texts[target.texts.length - 1];
const startLen = Math.min(sourceStart.length, targetStart.length);
const endLen = Math.min(sourceEnd.length, targetEnd.length);
return sourceStart.slice(0, startLen) !== targetStart.slice(0, startLen) ||
sourceEnd.slice(sourceEnd.length - endLen) !== targetEnd.slice(targetEnd.length - endLen);
} I'm not sure how much further we want to take this logic; allowing more things than is technically possible in the comparable relation isn't a very big defect, and it's somewhat hard to put exact bounds on what the |
number
don't fail when compared without overlap (x::${number}
versus ${number}
) 2367
Is this area very performance-bound? I've been hashing it out and I think I could write something to relatively accurately compare two template strings for overlap, but it'd necessarily require checking a lot of different options. It would presumably take a lot more time than checking if the first and last parts are the same. This caused me to ship a bug so I'd like to fix it. Otherwise I have to use branded strings to distinguish my number strings from other strings which is kinda annoying. |
Overall codesize and complexity is a budget we have to manage. By far the most common complaint we get in this area is that "I should be able to compare anything"; we get very few "I was not allowed to compare two things that in reality didn't overlap". So I'd really need to see more feedback from other people on this before committing to consider any approach. |
Ok fair enough. Thanks for the explanation! |
🔎 Search Terms
string condition always return false
2367 template
🕗 Version & Regression Information
This is the behavior in every version after 4.4.4 (including nightly), and I reviewed the FAQ for entries about "template" or "string"
This issue claims to fix a related problem in 4.5, but 4.5.5 is the first issue that fails for me. Maybe no relation though.
#45201
In 4.4.4 it it fails correctly though, so maybe that fix is the regression of this issue?! https://www.typescriptlang.org/play?ts=4.4.4#code/C4TwDgpgBAglC8UAGBDAXGgJAbwHYFcBbAIwgCcBfJAKFEigCEFliMcCTyrrbxoANZkgAebPEVKUadaAE0h7CVxrUAxgHtcAZ2BR0sIeizYAjNw3bdrRkNbGzKizqiiogxCLEO1m5yDRQ8h443tQAlgBmUAAUwgjwiCAAlFDY1FBQTuoANhAAdNnqAObRAOTAABZhWlBaFer42QAmUBUoAG7QEShhuU2lSdQUPJExKPGIxClpGVm5BcVl3b01GmRkEKrA2SADQ9RAA
⏯ Playground Link
https://www.typescriptlang.org/play?#code/C4TwDgpgBAglC8UAGBDAXGgJAbwHYFcBbAIwgCcBfJAKFEigCEFliMcCTyrrbxoANZkgAebPEVKUadaAE0h7CVxrUAxgHtcAZ2BR0sIeizYAjNw3bdrRkNbGzKizqiiogxCLEO1m5yDRQ8h443tQAlgBmUAAUwgjwiCAAlFDY1FBQTuoANhAAdNnqAObRAOTAABZhWlBaFer42QAmUBUoAG7QEShhuU2lSdQUPJExKPGIxClpGVm5BcVl3b01GmRkEKrA2SADQ9RAA
💻 Code
🙁 Actual behavior
Comparing x and y generates no complaints from TS for
"x::1"
and"1"
, but does for"a::1"
and"b::1"
. There is no way for a number to evaluate to"x::
so there is no overlap between these template types.🙂 Expected behavior
I would expect that Typescript shows that error consistently whenever types don't match.
There is no overlap between a string that must start with "x::" and one that must start with a number.
Additional information about the issue
No response
The text was updated successfully, but these errors were encountered: