microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.12k stars 12.5k forks source link

Arguments cease to be assignable when parameter is broadened via conditional #52310

Open wbt opened 1 year ago

wbt commented 1 year ago

Bug Report

πŸ”Ž Search Terms

conditional parameter assignable broader

πŸ•— Version & Regression Information

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

type AttributeMap = {
    Animal: { swims: boolean; lifespan: number; };
    Robot: { swims: boolean; serial: string; };
}
type Mover = keyof AttributeMap;
type Mountain = { name: string; elevation: number;}
type MountainAttribute = keyof Mountain;
type MoverOrMountain = Mover | 'Mountain';
type BroaderAttributeName<M extends MoverOrMountain> = (
    M extends Mover ?
    keyof AttributeMap[M] :
    MountainAttribute
) & string;
declare function actOnAttribute<M extends Mover>(
    mover : M,
    attr : keyof AttributeMap[M] & string,
) : void ;
declare function actOnAttributeBroader<M extends MoverOrMountain>(
    moverOrMountain : M,
    attr : BroaderAttributeName<M>,
) : void ;
//Sometimes-possible workaround:
interface ExtendedAttributeMap extends AttributeMap {
    Mountain: { name: string; elevation: number };
}
declare function actOnAttributeBroaderWorkaround<
    M extends keyof ExtendedAttributeMap,
>(
    moverOrMountain : M,
    attr : keyof ExtendedAttributeMap[M] & string,
) : void ;
export const demoFunction = function<M extends Mover> (
    mover : M,
    attr : keyof AttributeMap[M] & string,
) {
    actOnAttribute(mover, attr); //works, as expected
    actOnAttributeBroader(mover, attr); //Err: 2nd arg not assignable
    actOnAttributeBroader<M>(mover, attr); //specifying M doesn't help
    actOnAttributeBroaderWorkaround(mover, attr); //works, as expected
}

Note: Dropping instances of β€˜& string’ just results in a more complicated error message; it doesn’t change the core issue.

πŸ™ Actual behavior

actOnAttributeBroader accepts a superset of the valid parameters of actOnAttribute. In this calling context, these two versions of the called function are equivalent, since all types in that context are limited to the more narrow set accepted by actOnAttribute. However, the TypeScript compiler doesn't even try to figure out that the one conditional type of this example code simplifies to the true branch in the context where the error is triggered, so there isn't actually a need to trigger an error. It triggers an error.

πŸ™‚ Expected behavior

Making a reusable function more broadly reusable by accepting a broader set of parameter types should (a) possibly allow some calls that wouldn't work before the broadening to work, and (b) not cause any calls that were working fine before the broadening to suddenly start throwing Typescript compilation failures. Generic, reusable code should be considered a good thing and TypeScript should support that goal (as it seems to in the marketing that convinces technical executives to trigger conversions) rather than fight against it (as it does in practice with in-practice design choices and implementation defects, where this issue is an example).

Where Typescript correctly validates all parameters to a call to actOnAttribute and has enough information to determine which side of the conditional will apply (as it does here), it should do so and simplify the broader possibility of the reusable function down to the simpler original version where it can correctly validate types. There should be no error.

RyanCavanaugh commented 1 year ago

TypeScript should support that goal (as it seems to in the marketing that convinces technical executives to trigger conversions) rather than fight against it (as it does in practice with in-practice design choices and implementation defects, where this issue is an example).

This side-swiping isn't necessary or helpful. Why bother?

wbt commented 1 year ago

It's not intended as side-swiping but rather to try to provide for a little motivation on why this should be an issue worth fixing, instead of an issue to be dismissed like this, where behavior inconsistent with documentation won't get fixed because it just doesn't matter enough.

Showing a connection to a larger goal the TypeScript project seems to make some public claims of supporting as a motivator for the project overall could be helpful and fixing this would bring the actual practice closer to the goal.

Similarly, showing a connection between any project/task proposal and the motivation/mission of an organization/larger project is generally helpful to include in a document when the objective is to secure resources for that task to be completed. In the initial report, "bug/backlog" is a much more desired outcome than "closed wontfix/design choice."

RyanCavanaugh commented 1 year ago

Similarly, showing a connection between any project/task proposal and the motivation/mission of an organization/larger project is generally helpful

Right, and the stated implication that we're "fighting against" that goal, rather than toward it, is incredibly grating.

wbt commented 1 year ago

Looking through the issues to try to find a duplicate before filing this one, most of what came up as potentially related was CLOSED WONTFIX, and I read through lots of comments to the effect of "this compiler behavior is wrong but it's hard to get right so we're not going to try to fix it and just close the issue report instead."

That's also fairly consistent with my experience of responses to my own issue reports.
My average report represents multiple days of effort, starting with trying to debug and diagnose in the source domain, copy it out and boil it down to a more minimal demonstration that still demonstrates the issue and (the trickiest part:) likely preserves other features that don't seem important but will turn out to be especially in trying to discourage closure of an issue due to the availability of some non-fix workaround that does fine on a toy example but not more complex code. I then need to try out various possible workaround strategies on the simple example, search through StackOverflow, blogs, TypeScript learning sites, documentation and release notes, the FAQ here, and other internet resources for possible solutions, search through the Issues list for possible duplicates and parse through other issue reports and sometimes-long comment chains to try to determine if each one is a duplicate or not (mentally parsing code examples to an abstract space where they are more comparable to mine, even if superficially quite different), testing the example code including checking workarounds and error messages on all available past + current + nightly versions of TypeScript, writing up the issue description, reading through and revising it (usually several times) to be sure it clearly explains the issue, then proofreading, and after all that good-faith effort toward making TypeScript better, the formal response from the TypeScript community is usually to just blow it off as not something that should be addressed because it's hard to fix and/or not important. It's a pretty frustrating experience.

TS's implementation differs so much from its advertised capabilities and attributes that even an application of only moderately low complexity requires going at least partway down this path way more often than reaching its conclusion; I see (and feel) individual engineers getting improper blame for incompetence when unable to get TS to work as easily as advertised because of compiler bugs and design choices.

I definitely understand that some issues are very hard to fix, especially in the fully general sense that can cover 100% of all intended use cases. It seems likely that most issues available to be found in TypeScript now would be hard to fix: if they were easy to fix (and not just recently introduced), there's a pretty good chance they would already have been fixed, and wouldn't be outstanding issues. Even ones that seem like they should be easily fixable apparently have some hidden wrinkles that make them a lot harder. I also understand that some issues are not that important, though I don't think nearly as many issues where actual behavior diverges from documentation fall into that category as those who do the closing. Where I see divergences from stated goals actively dismissed as design choices, as I did when reviewing issues before this post, it does seem like there is some effort being expended to fight against those goals. One might hope (as I did) that calling some attention that, even with just a small comment buried near the end of a post, might lead to some of that effort being redirected to support the goal instead.

RyanCavanaugh commented 1 year ago

It's extremely easy to find issues that people report and get treated as bugs and fixed in a timely manner. I'm not sure what it is you think the team is doing -- you can see the PRs that go through, you can see bugs getting triaged and fixed, and you can see product improvements in every release notes.

At the end of the day I think you need to stop reading so much into Open vs Closed. If you want to fix a bug in something that's closed as Design Limitation, please go right ahead. I will not stop you; in fact I will cheer for you because you've done something we thought wasn't practical.

The thing we're trying to communicate with Open vs Closed is "can this be fixed in a reasonable timeframe". We don't leave issues open if we don't think they can be fixed in a reasonable timeframe, because that creates wrong impressions in people reading those bugs. I don't want someone to see an "Open" issue on "TypeScript does not yet solve the halting problem to correctly typecheck this program" and think that TypeScript will be solving the halting problem in a future release. Maybe we will, but probably not, and the open/closed metadata is there to communicate that judgement. What you seem to be asking is that we discard that information and leave people with no idea of whether or not something can be fixed in the reasonable future, which is bad, because people should be able to look at these things and have reasonable expectactions.

What you're trying to infer from it is "Does TypeScript care about its users". I can't stop you from taking these aggressive misreads of our planning metadata, but I'd really appreciate if you would do so on your own accord.