Callback parameter type is any for union of arrays #52748
Copy link
Copy link
Closed
Labels
Working as IntendedThe behavior described is the intended behavior; this is not a bugThe behavior described is the intended behavior; this is not a bug
Description
Bug Report
🔎 Search Terms
array, union, callback, any, contextual
🕗 Version & Regression Information
- This is the behavior in every version I tried, and I reviewed all FAQ
⏯ Playground Link
Playground link with relevant code
💻 Code
let myCondition = true;
const arr1 = ["a", "b", "c"];
const arr2 = [1, 2, 3];
// Correctly inferred as string[] | number[]
const myArr = myCondition ? arr1 : arr2;
for (const val of myArr) {
// val correctly inferred to string | number
console.log(val);
}
myArr.forEach((val) => {
// val incorrectly inferred as any
console.log(val);
});🙁 Actual behavior
The for..of loop correctly infers the type, but array methods (e.g. forEach, map, filter, etc.) infer the array item type to be any.
🙂 Expected behavior
Given a type which is the union of arrays and/or tuples, T1[] | T2[] | ... Tn[], array methods which take functions should infer the item parameter's type as T1[number] | T2[number] | ... Tn[number].
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
Working as IntendedThe behavior described is the intended behavior; this is not a bugThe behavior described is the intended behavior; this is not a bug
Activity
steverep commentedon Feb 13, 2023
Note #37453 is nearly the same issue, but for a union of a set and an array. It was closed saying:
I don't claim to understand the details of that limitation, but it seems like it should not apply here since the source declaration is just the array prototype for all members of the union.
RyanCavanaugh commentedon Feb 13, 2023
myArrinstead ofarr1here?string | numberas expectedsteverep commentedon Feb 13, 2023
Sorry, pasted the wrong code. I corrected to match the playground.
However, even if I do just correct that empty array version to use
myArr, it still seems to inferany(at least in 4.9.5).fatcerberus commentedon Feb 13, 2023
In the Playground as posted,
valis inferredstring(as expected, sincearr1isstring[]). If callingmyArr.forEachwhich is what I assume you intended, then it's correctlystring | number, just like thefor..ofversion:https://www.typescriptlang.org/play?ts=4.9.5#code/MYewdgzgLgBAtgTwMLgCYEsrvDAvDKAJwFcBTAbgFgAoG0SWAQ0MIEY8YBtAIke4BoY3AEYChwbgF0q1etBjNCAJg6dWgpYIDM0mjQD0+mChalgUADYIY6MADNSp1AogxohWwHNOkmAB8YMGI4YUcfOnB5RABBFg5EFDAMLBwAfgUWdgAuDOUZGjsQQhgACjlYADdGCxgQO3gEWMIAShgAbxoYLphDGCqa0FNzKxt7R0JSZygQNyIvf0Dg0MJOmDkQC1IAOgsQTxL+5ple7tPTgD1UmgBfPWpFVi3CwgBRRmAACxKD6tbcAD52qsur1+qNBhNhtZbA4nC4FGAEKt1psdnsfhYjgYjGdcZcbljaNQYiwnkU3p9voc8ICOtRTqDquCipDLNCxnDGK5GIjkZENttdvtDsccbiLldqNdCUA
I'm not sure where you're getting
anyfrom.steverep commentedon Feb 13, 2023
Ugh... finally corrected code and playground not to be
arr1.foreach...I don't know how to see the
^?inspections in the playground. They don't seem to be accessible using a screen reader. I'm simply putting that in vscode and inspecting the types there, and it's clearlyany.fatcerberus commentedon Feb 13, 2023
It's most definitely not
any. Something must be wrong with your local configuration (screenshot taken directly from your playground):(If you can't see the screenshot - hovering over
valin your playground example shows it being typed asstring | numberin both locations.)fatcerberus commentedon Feb 13, 2023
FWIW I can reproduce the
anyinference by setting the playground tov4.2.3(which leads to an "implicit any" error). All later versions exhibit the expected typing.steverep commentedon Feb 14, 2023
So after some investigation it seems the problem only occurs when
noImplicitAnyis turned off. Here's a new playground with the loops trying to logval.length. Thefor...ofloop correctly throws an error, but theforEachdoes not since it is typed asany.RyanCavanaugh commentedon Feb 14, 2023
This is the intended behavior. The
forEachcall used to beanywithnoImplicitAnyoff, and an error otherwise (due to the implicitany). Since it'd be a breaking change to ananyto anything else, that behavior hasn't changed.steverep commentedon Feb 14, 2023
That's very confusing indeed...
noImplicitAnysuggests that inferred types would change based on the setting, only that eitheranywill be inferred when the type cannot be determined or an error will occur.string[], thenforEachgives the parameter astringtype regardless ofnoImplicitAny.I also don't understand why fixing this would be avoided as a breaking change. If anyone is relying on
anyas the parameter type in this situation, then TS should expose that as a bug to fix.Please consider altering this intention at least for version 5.
fatcerberus commentedon Feb 14, 2023
Steve Repsher (@steverep) AFAIK
noImplicitAnydoesn't change inferences, only whether an implicitanyis an error or not. Something else really weird is going on here. In the playground link from your OP,valin theforEachcall isstring | number. I've double-checked this several times. In the most recent one you posted above, it'sany. Regardless of togglingnoImplicitAny. The code is exactly the same in both playgrounds.Ryan Cavanaugh (@RyanCavanaugh) What's going on here?
RyanCavanaugh commentedon Feb 14, 2023
This was really done on purpose, not sure what else to say. See #42620
This isn't true.
Probably the problem is that you still have
strictchecked in your Playground settings? The Playground UI is confusing; unchecked means "not set", not "false".1 remaining item
steverep commentedon Feb 14, 2023
I agree this is very similar to #47294, but that issue claims things changed between 4.2 and 4.3, which doesn't seem to be the case here. I went back as far as 3.9 and get the same
anyinference whennoImplicitAnyis unchecked. That seems to also suggest the above referenced PR may have nothing to do with this.Also, that issue was closed without a suitable resolution IMHO. The author also points to the documentation not being aligned with the actual behavior. It's documented as a "Type Checking" option and says:.
Reading that, there's no way a user should glean that it should be altering inferences. In fact it says the opposite.
I guess I don't know what else to say either, except that this should be addressed one way or another. Inferring
anywhen the contextual type is clearly available is confusing, inconsistent, and has no benefit (that I can see at least).fatcerberus commentedon Feb 14, 2023
I get the sense that we're all talking past each other.
It's the same issue - prior to 4.3, it always inferred
anyin your example. #42620 implemented the inference that allows it to inferstring | numberin the OP, but this only happens whennoImplicitAnyis enabled (which you yourself observed), and #47294 is calling out the discrepancy. However, that turns out to be intentional--for whatever reason--per #42620:steverep commentedon Feb 14, 2023
Ah... I missed that subtlety - thanks for explaining. For my edification, what do you mean by "OP"?
And I don't want to seem like I'm jumping on or over anyone. I'm just trying to make a good case for this not to be written off as "intended behavior". Certainly not all intentions have only positive consequences. 😉
fatcerberus commentedon Feb 14, 2023
"OP" means Original Post, and/or Original Poster, depending on context. In this case it's referring to the main text of your issue at the top of the page, prior to all the replies.
steverep commentedon Feb 14, 2023
Wesley Wigham (@weswigham) could you comment on why #42620 only improved type inferences when
noImplicitAnyis enabled?Ryan Cavanaugh (@RyanCavanaugh) is that PR the only case where
noImplicitAnyis affecting type inferences?RyanCavanaugh commentedon Feb 14, 2023
We try not to introduce new errors into existing code, especially if that code has
strictoffsteverep commentedon Feb 14, 2023
Um, isn't that exactly what the PR in question does - strengthens contextual types to expose errors? It doesn't require
strictmode.If this is intentional only in the name of backwards compatibility, it seems to me that #42620 should have left the behavior of
noImplicitAnyalone, and introduced a new backwards compatibility option instead. To me, that seems like a suitable way to address this.Please note also that my actual use case results in a situation where
noImplicitAnywould produce no error (as expected), but turning it off does throw an error, which is just another point of confusion I guess.RyanCavanaugh commentedon Feb 14, 2023
Changing an
anyto astring | numbercould absolutely introduce new check-time errors.If every change we did had an associated back-compat flag, there'd be thousands of them instead of hundreds. The approach isn't really scalable.
But that's exactly what happened. People with
noImplicitAnyoff had the previous behavior retained, and people with it on went from having a check-time error to having the "expected" type.I don't see a check-time error in either case, so I'm not sure what you're referring to.
steverep commentedon Feb 14, 2023
See this playground code instead, which is more aligned with the current use case that prompted me to open the issue. Having
noImplicitAnyturned off results in an error, and turning it on results in no error, which is counterintuitive.I disagree. I don't see that as leaving the purpose of
noImplicitAnyalone. It went from behaving as the docs explain to changing the behavior of inferred types. Furthermore, it left users who don't usenoImplicitAnycompletely incapable of benefiting from better contextual types without probably a lot of unrelated work to turn it on.fatcerberus commentedon Feb 15, 2023
Hmm, this error probably shouldn't happen.
anyis the "bypass type checking" type and the rest of the string matches.RyanCavanaugh commentedon Feb 15, 2023
noImplicitAnyhas always had subtle effects outside the strict domain of unannotated variables/parameters. The documentation is necessarily going to not cover every single edge case in the interests of being something that provides information that you need to know rather than information that exists. A 100.0% precise definition of all behavior in TypeScript is 50,000 lines of checker.ts, which is also available for reading.Yeah, though I'm not 100% sure how to make that work. I think it'd be ridiculous for
"x-${any}"to match"y-z"for the same reason that{ start: "x", end: any }still doesn't match{ start: "y", end: "z" }. But the template string relational algorithm doesn't have any logic for relating an infinite string to a finite string (since, apart fromany, that is always going to fail). If it comes up again we can think about it more.fatcerberus commentedon Feb 15, 2023
You say that, and yet
T & anyisanyfor any T. 😉 (I actually do agree with you though)typescript-bot commentedon Feb 17, 2023
This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.