Unwrap NoInfer types when narrowing#58292
Merged
sandersn merged 8 commits intomicrosoft:mainfrom May 31, 2024
Merged
Conversation
typescript-bot
added
the
For Uncommitted Bug
Andarist
commented
Apr 23, 2024
src/compiler/checker.ts
Outdated
Comment on lines
27684
to
27685
| declaredType = isNoInferType(declaredType) ? (declaredType as SubstitutionType).baseType : declaredType; | ||
| initialType = isNoInferType(initialType) ? (initialType as SubstitutionType).baseType : initialType; |
Contributor
Author
There was a problem hiding this comment.
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; }>
});
Member
There was a problem hiding this comment.
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.
Contributor
Author
There was a problem hiding this comment.
moved it to getNarrowableTypeForReference, works like a charm - thanks!
typescript-bot
added
For Backlog Bug
ahejlsberg
approved these changes
May 28, 2024
Contributor
Author
|
@sandersn @RyanCavanaugh could this one land before the upcoming RC? |
Andarist
force-pushed
the
fix/noinfer-flow-type
branch
from
8e672ba to
34a6d3d
Compare
skeate
added a commit
to skeate/TypeScript
that referenced
this pull request
Jun 1, 2024
* upstream/main: (37 commits) Added NoTruncation flag to completions (microsoft#58719) Clone node to remove location even when it has been modified if needed (microsoft#58706) Properly account for `this` argument in intersection apparent type caching (microsoft#58677) Fix: Include Values of Script Extensions for Unicode Property Value Expressions in Regular Expressions (microsoft#58615) In `reScanSlashToken` use `charCodeChecked` not `codePointChecked` (microsoft#58727) Shorten error spans for errors reported on constructor declarations (microsoft#58061) Mark file as skips typechecking if it contains ts-nocheck (microsoft#58593) Fixed an issue with broken `await using` declarations in `for of` loops (microsoft#56466) Do not expand type references in keyof and index access (microsoft#58715) Improve the performance of isolatedDeclarations quickfix (microsoft#58722) Unwrap `NoInfer` types when narrowing (microsoft#58292) Recover from type reuse errors by falling back to inferred type printing (microsoft#58720) Fixing self import (microsoft#58718) Enable JS emit for noCheck and noCheck for transpileModule (microsoft#58364) Revert PR 55371 (microsoft#58702) Update dependencies (microsoft#58639) Fix baselines after PR 58621 (microsoft#58705) Do not infer `yield*` type from contextual `TReturn` (microsoft#58621) `await using` normative changes (microsoft#58624) Handling statements from a known source file (microsoft#58679) ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #58266