You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I am writing a helper function that provides functionality often associated with the traditional switch statement, but in a more functional style. In doing so, I have come across a bug.
Consider the following code, which is a simplification of the helper I'm writing:
When hovering the first on call, we see the expected information:
However, when hovering the second on call, which is a call to the on property of the result of either match or matched, we get:
As we can see, the types are just fine, but the argument names are replaced with 'arg0' and 'arg1'. This in itself could be intentional for some reason, and hence not a bug, but here's what leads me to consider this a bug:
When removing the last argument to the second on call, IntelliSense gives us an error as expected, but with an error text containing one of the previously replaced argument names, 'outcome':
The same goes for 'prediction' if both arguments are removed.
I suspect this might be a result of trying to derive the parameter names from the second definition of on (as a result of the matched function), which would be inconsistent as the rest of the call signature is clearly derived from the first definition (as a result of the match function). Or I am completely wrong, which is also very much possible.
Anyway, this is misleading. What I expect is for both the call signature and error text to contain 'prediction' and 'outcome' as the parameter names, or at the very least for these to be consistent.
The issue is that when the signatures in the union are combined, instead of using the only available name (ex: a1) the fallback for when parameter names don't match is used (arg${i}). This does not manifest for non-generic functions as there the signature is just cloned.
Possible Solutions:
Targeted fix for just this case: Fix the combination algorithm to use the available name when one of the combined signatures has fewer parameters. This should be a quick easy fix.
Improve on arg${i}: arg${i} is not particularly useful or friendly, and this is what is used when signatures have conflicting parameter names. Perhaps we could concatenate parameter names, ideally to something meaningful like p1 & p2 (where p1 and p2 come from the original signatures). While this is not normally a valid name for a parameter, it would make clear to the user that this is an amalgamation of the two original parameter and will probably not have any negative impact in other places since this is just a signature used for type checking (but I have not investigated this).
For my part I kind of like the second idea, when you have a union of functions it ends up requiring an intersection of arguments (by way of contravariance) which tends to throw people off especially if they're not used to seeing intersections in everyday code. So it might be good to provide a hint that the signature on display was in fact synthesized from two (or more) different functions.
Activity
korkje commentedon Jun 9, 2019
Also, weirdly, if you hover
.onbefore typing the parentheses, the correct parameter names are shown.mjbvz commentedon Jun 10, 2019
Confirmed with TS 3.6.0-dev.20190608
fatcerberus commentedon Jun 13, 2019
Possibly related: Playground link
Parameter names are correct while typing out the call, but...
once everything is totally filled out it reverts to generic
arg_0etc.dragomirtitian commentedon Jun 13, 2019
Ryan Cavanaugh (@RyanCavanaugh) can I have a look at this ? I dug around previously in the area (#30084)
RyanCavanaugh commentedon Jun 13, 2019
Sure!
dragomirtitian commentedon Jun 22, 2019
Ryan Cavanaugh (@RyanCavanaugh) A smaller repro of this would be:
The issue is that when the signatures in the union are combined, instead of using the only available name (ex:
a1) the fallback for when parameter names don't match is used (arg${i}). This does not manifest for non-generic functions as there the signature is just cloned.Possible Solutions:
arg${i}:arg${i}is not particularly useful or friendly, and this is what is used when signatures have conflicting parameter names. Perhaps we could concatenate parameter names, ideally to something meaningful likep1 & p2(wherep1andp2come from the original signatures). While this is not normally a valid name for a parameter, it would make clear to the user that this is an amalgamation of the two original parameter and will probably not have any negative impact in other places since this is just a signature used for type checking (but I have not investigated this).Ex:
This is a bit of a whacky idea but it might be useful 🤷♀️
Should I just proceed with 1 ? Or experiment with 2 ?
Bruce Pascoe (@fatcerberus) The issue you found has a different root cause to the one in the original post, it might merit a ticket of its own.
fatcerberus commentedon Jun 22, 2019
For my part I kind of like the second idea, when you have a union of functions it ends up requiring an intersection of arguments (by way of contravariance) which tends to throw people off especially if they're not used to seeing intersections in everyday code. So it might be good to provide a hint that the signature on display was in fact synthesized from two (or more) different functions.