-
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
"Did you mean" spelling suggestions can offer the same thing being complained about #25564
Comments
The immediate problem is that spelling suggestions get suggestion targets from I'm not sure whether spelling suggestions should be changed, except perhaps to add an assert that the result is not equal to the missing name. (The assertion that the missing name isn't in the candidate set is too expensive.) |
So then a real misspelling suggests the correct spelling, which is then an error? Sounds fun: return {
test: 'test',
requiredWhenNotVoid: a
//~~~~~~~~~~~~~~~~~~~ Error:
//Object literal may only specify known properties,
//but 'requiredWhenNotVoid' does not exist in
//type 'Something<A>'. Did you mean to write 'requiredWhenNonVoid'?
} Okay... return {
test: 'test',
requiredWhenNonVoid: a // fixed
//~~~~~~~~~~~~~~~~~~~ Error:
//Object literal may only specify known properties,
//but 'requiredWhenNonVoid' does not exist in
//type 'Something<A>'. Ha ha, fooled you! 😝
} I hate you, compiler! 🏃♂️ 😭 |
I figured out how to fix the excess property error at least: make excess property checks understand conditional types: /** (in isKnownProperty) */
else if (targetType.flags & TypeFlags.Conditional) {
return isKnownProperty((targetType as ConditionalType).root.trueType, name, isComparingJsxAttributes) ||
isKnownProperty((targetType as ConditionalType).root.falseType, name, isComparingJsxAttributes);
} This exposes the more inscrutable bug that @RyanCavanaugh and I encountered, which is that |
Thanks for fixing the message so quickly! Any idea why the example still doesn't compile though? It should be valid right? This came from my initial question here: https://stackoverflow.com/q/51274107/1048589 |
Basically, there's no assignability rule that directly relates object types to conditional types, and because we're inside
The assignment the other way works because of a simplification rule that applies:
So I know why the example doesn't compile. I don't know if the reason is valid: Should the simplification rule also apply when the target is a conditional type, or just when the source is a conditional type? I'm not sure. @ahejlsberg have you thought about this assignability case? |
@ahejlsberg any ideas? I'm wondering if I should create a separate issue for tracking this? This causes an issue for us currently and we have to revert to some '' trickery and it would be great to not have to do this. |
First some comments on the original example. The type The conditional type used in ({ test: string } & { requiredWhenNonVoid: string }) |
({ test: string } & { requiredWhenNonVoid: number }) I suspect something closer to the intent is type Something<T> = { test: string } &
(undefined extends T ? { requiredWhenNonVoid?: T } : { requiredWhenNonVoid: T }); which basically says that if { test: string } & { requiredWhenNonVoid: string | number } Regarding the assignment compatibility error, as @sandersn points out we don't currently have a rule that makes a non-conditional type assignable to a conditional type. But we probably should. Specifically, we should say that some type type Foo<T> = T extends true ? string : "a";
function test<T>(x: Foo<T>, s: string) {
x = "a"; // Currently an error, should be ok
x = s; // Error
} With such a rule the original example would compile if the definition of |
@ahejlsberg thanks I understand a bit better what is happening now. I think there is actually already a ticket for this: #24929 |
Actually, #24929 is a different issue in that it would require the type checker to infer conditional types from conditional statements and expressions. What we're talking about here is a simple rule that says a type |
TypeScript Version: 2.92
Search Terms: did you mean spelling suggestion same
Code
Expected behavior: Probably no error, or at least not this error suggestion
Actual behavior:
There's no spelling error here -- the
requiredWhenNonVoid
being cited is the samerequiredWhenNonVoid
being suggested 🤡Playground Link: Link
Related Issues: Nope
The text was updated successfully, but these errors were encountered: