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
Unwrap NoInfer
types when narrowing
#58292
base: main
Are you sure you want to change the base?
Conversation
src/compiler/checker.ts
Outdated
declaredType = isNoInferType(declaredType) ? (declaredType as SubstitutionType).baseType : declaredType; | ||
initialType = isNoInferType(initialType) ? (initialType as SubstitutionType).baseType : initialType; |
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.
My first instinct was that I should re-wrap the result of narrowing with NoInfer
but quoting the original NoInfer
PR:
Other than blocking inference, NoInfer markers have no effect on T. Indeed, T and NoInfer are considered identical types in all other contexts.
So I don't see a reason why that would be necessary. I also tested something like this (TS playground) to see if this would be needed and I don't think it is:
declare function inner<T>(arg: T): T;
declare function outer<T>(a: T, cb: (arg: NoInfer<T>) => void): void;
outer({ foo: "bar" }, (arg) => {
arg
// ^? (parameter) arg: NoInfer<{ foo: string; }>
const result = inner(arg); // inferred despite `arg` being a `NoInfer` type!
// ^? const result: NoInfer<{ foo: string; }>
});
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 agree it's fine to erase NoInfer
in variable, parameter, and property references, but I think it would be simpler and more consistent to do it first thing in the getNarrowableTypeForReference
function. I think that's all you'd need.
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.
moved it to getNarrowableTypeForReference
, works like a charm - thanks!
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.
See my comment above.
fixes #58266