-
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
NoInfer not working on callback parameter destructuring #60544
Comments
Note: not a TS team member That parameter is not annotated so it can't infer from there anyway, with or without |
I don't know what you mean here by "not annotated" and I'm not hoping anything about |
As for "I'm not hoping anything" I was trying to say that you have an expectation about |
In this example |
In any case, not wanting to continue the discussion above and getting back to the issue at topic. I managed to do a workaround that works. playground class Foo<T, R> {
constructor(readonly cb: <NoInferR = R>(t: T, _: { x: number; other: NoInferR }) => R) {}
}
const _1 = new Foo((name: string, { x }) => ({ name, x })); // Foo<string, { name: string; x: number }> but it breaks again as soon as I try to merge the class with an interface. It doesn't make sense. playground class Foo<T, R> {
constructor(readonly cb: <NoInferR = R>(t: T, _: { x: number; other: NoInferR }) => R) {}
}
interface Foo<T, R> {}
const _1 = new Foo((name: string, { x }) => ({ name, x })); // Foo<string, unknown> again |
Iβ¦ the βexpected behaviorβ is all I was trying to say (as per the template for the issue) I sincerely donβt mean to offend, nor do I want to seem to derail your issue. I will disengage now. Good luck. |
The behavior is a design limitation. And like @jcalz has mentioned it doesn't really have anything with Your In the
The type there might be what you expect here but I don't think it actually works like you might expect it too: TS playground |
In this example: playground class Foo<T, R> {
constructor(readonly cb: (t: T, _: { x: number; other: NoInfer<R> }) => R) {}
}
const _1 = new Foo((name: string, { x }): { name: string; x: number } => ({ name, x })); // Foo<string, unknown> I'm annotating both On the other hand, I understand that in the workaround Or is this a different discussion? I feel like there are bugs everywhere. Welp, focusing on the original example, should the title of the issue be something like "There's no way to mark parameters in callback types as |
This is an interesting case. It seems like the compiler doesn't ignore from a return type annotation. It should be fairly straightforward to add this capability, I'll take a stab at it later.
As it stands, this is a design limitation and not a bug. I understand it's frustrating to hit limitations but it's not like they exist for no reason. The type checker is a complex piece of software and its design comes with some tradeoffs here or there.
No. What you report here is not about class Foo<T, R> {
constructor(readonly cb: (t: T, other: NoInfer<R>) => R) {}
}
const _4 = new Foo((name: string, other) => ({ name })); // Foo<string, unknown> The |
I think I understand now with this wording. Although, I still think that I tried to see if the limitation can be circumvented with an overload that doesn't have
On workaround 2, I found out that if you extract the type to a auxiliary interface, it works again. playground. It really is shocking seeing how changes that don't change anything make it or break it. |
It really doesn't work here. It's the same thing I mentioned at the end of this comment. You can hover over things to see that your |
I meant on resolving the merge with the interface breaking it. Because you said you would have a stab at it. I understand that the workarounds don't really help inside the callback itself. Although that doesn't matter too much, as long as the variable outside ( |
From what was discussed, I think I discerned the best hack for it to work as intended. playground interface Aux<R> { // required so that the interface merge doesn't break it.
x: number;
other: R;
}
class Foo<R, T> {
constructor(cb: (t: T, _: Aux<unknown>) => R);
constructor(cb: (t: T, _: Aux<R>) => R);
constructor(readonly cb: (t: T, _: Aux<R>) => R) {}
}
interface Foo<R, T> {}
const _1 = new Foo((name: string, { x }) => ({ name, x })); // Foo<{ name: string; x: number }>
const _2 = new Foo((name: string, { other }: { other: { name: string } }) => {
other?.name;
return { name };
}); // Foo<{ name: string; x: number }>
In my opinion, the And the alias shouldn't be necessary either, though you already mentioned it is a pretty straightforward fix. What's concerning is that there's not a clear strategy on how to reach these "hacks", or for that matter, on how to report the usability issue here (regardless of what the compiler does behind the scene), besides throwing stuff at the wall to see what sticks. |
Is it too complex to not fix Perhaps using If this were to be a suggestion, is it considerable or is it too outlandish and out of the question? |
π Search Terms
NoInfer parameter callback destructuring
π Version & Regression Information
Latest to date.
β― Playground Link
https://www.typescriptlang.org/play/?#code/MYGwhgzhAEBiD28A8AVANNASgPmgbwCgBIYeAOwgBcAnAV2EvmoApqBTMAE3JAE9pgAIwBc0ZpVHpoAfVEA5eAEkyAMzbUkeaAA9RZWgFtB6gNzR4lABbr5S1eqQ5oAX2wBKaAF5cmD3mfEAPSBAuRUdAxMrBzcZHwCImIS0FKy+Dp6hsbUZhbW1LbKahpOzh7eWH4BRMGhFDT0jCzsXDz8QqLikhhpCkUOWrrQ+kam5lY2WC7uXj5VBAEEpPUyAIxew2wA7nCIzMxkYAZsouEAlmQA5hiDLuW4zFqHxxjad25mtQjI51cYtGQANZkeBbMjYAi1CCWeC0ECcaDGXY-GgXa7pZ4naC-S5mIYjbLTAhLMKUGQAJg2ZG2yP2mNOqKu9zETyObHenxCWyYgIgom+SBxN2GbIZ1DRRKAA
π» Code
π Actual behavior
infers
unknown
.π Expected behavior
Should ignore the parameter tagged with
NoInfer
and infer correctly.Additional information about the issue
Is this intended behavior? feels like a bug to me.
If the callback parameter is destructured partially it infers
unknown
, but it works if the parameter is omitted.I tried
NoInfer
at various locations.The text was updated successfully, but these errors were encountered: