Skip to content
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

Union signatures should exclude "uncallable" callbacks, so methods like .map work on union containing empty array #42558

Closed
5 tasks done
Nathan-Fenner opened this issue Jan 30, 2021 · 7 comments
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@Nathan-Fenner
Copy link
Contributor

Suggestion

When merging union signatures, skip "uncallable" callbacks: those that request a never callback argument. These functions can't be called, so they can be removed from consideration when building the function corresponding to a union signature.

💻 Use Cases

TypeScript sometimes infers empty arrays as having type never[]. Usually, this doesn't cause problems, until it gets unioned with another type, and you try to call it as a function/method:

function orDefault<T, D>(x: T | null, d: D): T | D {
    if (x === null) {
        return d;
    }
    return x;
}

const xs: string[] | null = ["a", "bc", "def"];

// y: string[] | never[]
const y = orDefault(xs, []);

// ERR: TypeScript 4.1
//  Each member of the union type 
//    (<U>(callbackfn: (value: string, index: number, array: string[]) => U, thisArg?: any) => U[])
//  | (<U>(callbackfn: (value: never, index: number, array: never[]) => U, thisArg?: any) => U[])
// has signatures, but none of those signatures are compatible with each other. (2349)

// ERR: TypeScript 4.2 (dev)
// Parameter 'item' implicitly has an 'any' type.(7006)
const yThen = y.map(item => item.length);
const yChain = orDefault(xs, []).map(item => item.length);

In this example, since the [] is typed as never[], the .map method has a union type with two incompatible types; one possibility is a string, number, string[] argument list returning a generic U, and the other is a never, number, never[] argument list returning a generic U.

In TypeScript 4.1, the two signatures were considered "incompatible", so attempting to call the function would raise an error. TypeScript 4.2 improves this situation slightly, since the function is now callable; however, it fails to infer the right types for the callback parameters, and so they get defaulted to any.

However, there's a generic rule which can be used to "fix" the union signature so that the correct typing machinery is activated! Since "legal" code can't create values of type never, legal code can't call functions with any argument having type never. For brevity, a callback where any of its required, non-rest parameters have type never is called "uncallable".

To avoid introducing too many new bugs or soundness holes, we can restrict this reasoning just to where union signatures are created. Specifically, when merging two callback types, if one of them is "uncallable" and the other is not, then we use the "callable" one for their intersection.


TypeScript currently handles a more-restrictive form of this check. It's able to see that (for example)

const mapNums: <R>(func: (item: number) => R) => R[]
    = null as any;
const mapNever: <S>(func: (item: never) => S) => S[]
    = null as any;
// mapSomething inferred: <R>(func: (item: number) => R) => R[]
const mapSomething = Math.random() < 1/2 ? mapNever : mapNums;

in this case, mapSomething has the same type as mapNums, since it can verify that the types are "compatible". The problem happens when an "irrelevant" type appears, which often occurs for "methods" since they have an implicit or explicit this parameter:

const mapNums: <R>(func: (item: number, x: string) => R) => R[]
    = null as any;
const mapNever: <S>(func: (item: never, y: number) => S) => S[]
    = null as any;
// mapSomething inferred: const mapSomething: (<S>(func: (item: never, y: number) => S) => S[]) | (<R>(func: (item: number, x: string) => R) => R[])
const mapSomething = Math.random() < 1/2 ? mapNever : mapNums;

now, the x and y parameters prevent tsc from assigning a subtyping relation between the two functions. However, since mapNever still can't be called, its type has effectively not changed from the previous example! So it's safe to merge them by treating it as a subtype still. That's what this change proposes, to handle some of the rough-cases where never doesn't aggressively "poison" the types it's wrapped in.

🔍 Search Terms

  • empty array map
  • "This expression is not callable"
  • Each member of the union type '((callbackfn: (value: Item, index: number, array: Item[]) => U, thisArg?: any) => U[]) | ((callbackfn: (value: never, index: number, array: never[]) => U, thisArg?: any) => U[])' has signatures, but none of those signatures are compatible with each other
  • ts(2349)
  • never type
  • incompatible callbacks

Vaguely related, but broader/different from this proposal: #40157

I previously opened and closed #42487 since I originally didn't know about the change in behavior in 4.2.

✅ Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@RyanCavanaugh
Copy link
Member

@weswigham what do you think?

@weswigham
Copy link
Member

I think this is morally equivalent to producing the union of the possibilities for the inferred parameter type, which iirc, we've declined to do in the past because it's a breaking change unless you're in noImplicitAny mode, and even then, there's no guarantee the types flow from the parameter to the return to make a valid signature (the typical example being two overloads with identical input and output types - you'd expect x => x to satisfy that, but it doesn't if you assign x a union of it's possible types).

@weswigham
Copy link
Member

weswigham commented Feb 1, 2021

Errr, scratch that, I'm thinking of overloads. For union's signatures our issue I'm this case is really that both signatures have generics, and those generics need to be reconciled. I actually thought I had a PR out (or already in?) that merged generics when they came from the same base signature, and thus allowed the signatures to merge...

@Nathan-Fenner
Copy link
Contributor Author

Nathan-Fenner commented Feb 1, 2021

@weswigham it shouldn't be a breaking change for any existing code - TS 4.1 and below typically rejects anything with union signatures that are complicated enough to trigger this change.

Since TS 4.2(dev) fails to infer callback parameter types in the one case this doesn't work, so you'd need type annotation to get it to compile. In that case, the type signature might be slightly refined after this change, but not enough to reject existing annotations (since e.g. string[] is a subtype of string[] | never[]).

It also goes beyond generic code; this is a non-generic example (in TS 4.2:

const f: (ask1: (x: string, y: never) => void, otherThing: string) => void = null as any;
const g: (ask2: (z: number, w: string) => void, moreThing: number) => void = null as any;

const h = Math.random() < 1/2 ? f : g;

h((a, b) => {
	return;
});

f and g ask for two different callbacks:

  • f wants (x: string, y: never) => void
  • g wants (z: number, w: string) => void

Since f can't intend to call its callback, "morally", we only need to pass a function satisfying g's requirements. Thus, we type h( ... ) as if it were g( ... ) since f is "uncallable". Specifically, we only do that for the callback argument.

So h effectively has the signature (ask2: (z: number, w: string) => void, otherThing: string | number) => void. In particular, the other arguments still get the usual "union" treatment.

Also for the record, the implementation at #42559 didn't find any breakages in existing code, and I think it would be exceedingly unlikely to find any. Union signatures aren't common, and they're much less common when they ask for callbacks that ask for arguments with type never.

@Nathan-Fenner
Copy link
Contributor Author

Nathan-Fenner commented Feb 17, 2021

@weswigham I want to ping on this issue again, since it's something I have run into in a few different ways in TS.

This is not about merging generic params on signatures - tsc already does this.

This is just about skipping callback parameters when these signatures are merged.


Effectively, it's a much-more constrained version of the following typing rule (which is sound, but TS currently does not use):

((x: never) => Whatever) & AFunctionType 
=
AFunctionType

However, this is likely to break existing code in unexpected ways, since intersections show up in a lot of places.

Instead, we only perform this reduction when combining callback parameters in union signatures. Which is a very uncommon place for them to show up, but when they do, they cause problems. This fixes them, while having very minimal fallout.

@jcalz
Copy link
Contributor

jcalz commented May 27, 2021

Is this addressed by #42620?

@Nathan-Fenner
Copy link
Contributor Author

All the cases I originally cared about seem to be solved in 4.3 beta, so looks good to me! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants