Use intersection types to correctly infer the result of _.merge in lodash

[pimterry]

Now that intersection types have landed in TS 1.6 we can happily and correctly infer the result of _.merge. This commit adds that (along with a few extra tests to support it).

There's notably a few limitations to this, but as far as I can tell they're unavoidable:

• The varargs version of _.merge (for 5 objects or more) requires you specify the expected result type
• The overridden key types are more general than they should be, because of how intersection types work. E.g. _.merge({a: 1}, {a: "hi"}) has type {a: number & string}, not just {a: string} (although that's assignable to {a: string}, so this doesn't cause problems, it's just not maximally safe).

[dt-bot]

lodash/lodash.d.ts

to authors (@bczengel @chrootsu). Could you review this PR?
👍 or 👎?

Checklist

• pass the Travis CI test?

[chrootsu]

@pimterry I looked at the intersection types but turned them down due to the fact that less strict types can be inferred for properties existing in both interfaces at once. I think we need more opinions about it. @vvakame what do you think?

[pimterry]

You're right that it's sad that intersection isn't perfect there, but I think it is still definitively better than not doing it, in basically 100% of cases.

Without this change you get no inference at all, and the return type is always{}, which makes it useless. Thus, currently you always have to specify the return type explicitly (with a cast or generic parameters).

Once you do specify a type (as you must), you lose all safety. By casting from '{}' or specifying the generic result type you skip any validation of valid values, and you can be totally wrong without the compiler noticing. Some examples below.

Without intersection types:

// Inferring types:
var x = _.merge({a: 1}, {b: "string"}); // x has inferred '{}' type - safe but unusable
var y = _.merge({a: 1}, {a: "string"}); // y has inferred '{}' type - safe but unusable

// Specifying types:

// You can give an explicit type, but there's no validation of this all, and no nice inference either
var z: { a: string } = _.merge({a: 1}, {a: "string"}); // Won't compile - {} is not assignable, so won't infer 
var q = <{ a: string }> _.merge({a: 1}, {a: "string"}); // Compiles, with explicit (totally unvalidated) cast
var p = <{ b: number[] }> _.merge({a: 1}, {a: "string"}); // Also compiles, but this is clearly wrong

With intersection types:

// Inferring types:
var x = _.merge({a: 1}, {b: "string"}); // x has completely correct inferred type
var y = _.merge({a: 1}, {a: "string"}); // y has slightly too general inferred type ({a: string|number})

// Specifying types:

// Explicit casts can resolve ambiguity, but here are actually validated
var z: { a: string } = _.merge({a: 1}, {a: "string"}); // compiles, has correct type, through inference only
var q = <{ a: string }> _.merge({a: 1}, {a: "string"}); // compiles
var p = <{ b: number[] }> _.merge({a: 1}, {a: "string"}); // doesn't compile - not assignable to incorrect type

I've stuck this in a playground so you can test it out.

It's imperfect, but I can't find any cases where the current solution is better, and I don't think there's any 3rd solution that would resolve the above more neatly (afaik).

It's worth noting that _.merge is almost completely equivalent to the original motivating example for the intersection types implementation: microsoft/TypeScript#3622. This is exactly what they're designed for.

[chrootsu]

@pimterry I think I have found a compromise pimterry#1. You can merge those changes, and they will fall into this pull request.
Sample: http://goo.gl/eeIGHO

[pimterry]

Ah, interesting. I don't hugely object to doing that as well, if you find it useful, but it seems more verbose than necessary, for me at least. What's the benefit of duplicating all the definitions, compared to just using a type assertion?

I.e. why is:

r = _.merge<{a: number}, {a: string}, {a: string}>({a: 1}, {b: ''});

better than

r = <{ a: string }> _.merge({a: 1}, {a: ''}); 

?

Is there any case where having it as an extra generic parameter instead of a type assertion gives you an advantage?

It feels to me like adding an extra generic parameter for the return type (as in your change) is not the easiest way to explicitly specify your return type. Instead, you can assert it into the correct type, for the exact same level of safety, but with shorter, simpler and easier to read code (and half the number of type definitions).

[chrootsu]

The type assertion can suppress errors. Example http://goo.gl/BZv22i
P.S. Excuse me for brevity of answers my English is still not too good.

[pimterry]

That's the same for both though. I've been looking into it and I think generic types without constraints allow exactly the same errors that type assertions do (and provide exactly the same safety). The only real difference is that adding generics means we have to duplicate all the definitions. I don't think we should do that.

For example, you can write this incorrect code (straight from your playground) with assertions:

declare let fn: {
    <A, B>(a: A, b: B): A & B;
}

result = <{a: number, b: string}>fn({}, {}); // <- Compiles, should error

But you can also write the same broken code with your generic version:

declare let fn2: {
    <A, B, C>(a: A, b: B): C;
}

result = fn2<{}, {}, {a: number, b: string}>({}, {}); // <- Compiles, should error

Specifying the result type with a generic type (that has no constraints) and type asserting the result type are exactly the same operation, given the same function. The typescript spec backs this up:

• Section 4.23: "In a type assertion, the expression is contextually typed by the indicated type."
• Section 4.23: "return expressions are contextually typed by the type given in the return type annotation." (in this case, the generic type given)

Specifying an explicit return type (via generics or otherwise) is the same as asserting the return type.

All the happens, in both cases, is that you specify what you think the type will be, and the compiler checks it's possible that you're right.

Summary: I don't think we should be using generics for this (i.e. to specify return types only, with no constraints or relationships to parameters), here or elsewhere. They make it more complicated, add duplicate method definitions, and can reduce safety (as in the uses of the original definition for this in my previous playground).

[chrootsu]

There are three subjects for discussion:

• first is about types of arguments: "implicitly inferred" vs "explicitly specified via generics"
• second is about how a returned value type should be specified explicitly: "assertion" vs "generic"
• third is about how returned value type should be defined: "infer via intersection" vs "explicitly specifying"

The last example (http://goo.gl/BZv22i) was about types of arguments (the first subject). A variable that is passed as an argument can be defined far away from a current call and it can have inferred type {} instead of the expected by us, e.g., {a: number}. Hence I want to make sure that this won’t happen and that is why I prefer to specify argument types explicitly.

If argument types are not specified the type of returned value becomes uncertain and needs to be checked (the second subject). I think a type assertion is less safe than constraints of a generic, for example http://goo.gl/BpWFNK . In the case of value<A, B>(a: A, b: B): A & B when a and b have unexpected type {}, the return type will be {} and then a type assertion will hide the error.

declare let fn2: {
    <A, B, C>(a: A, b: B): C;
}

result = fn2<{}, {}, {a: number, b: string}>({}, {}); // <- Compiles, should error

Specifying the result type with a generic type (that has no constraints)

Why it should be without constraints?.. If you have already specified the result type, it is very strange, IMHO, not to make the same for arguments. =)

About intersection (the third subject) I just prefer explicit specifying and more strict type to inferred one but less strict. It is a matter of taste, so I suggested a compromise that allows you to do as you wish.

@vvakame @basarat could you please help us resolve this discussion?.. Do you have any advice?

[pimterry]

Specifying the result type with a generic type (that has no constraints)

Why it should be without constraints?

I mean in the specific technical sense: there are no generic constraints on the type (if you could constrain it, like function fn<A, B, C extends A & B>() { }, then it would be equally safe, but sadly you can't). Because the type is completely unrelated to the arguments, it can be totally wrong, and the compiler can't check.

Generics exist to describe (and validate) the relationship between parameters, or parameters and return values (and thereby validate constraints on that relationship). There are no relationships here (C is nothing to do with A or B). This isn't what generics are for.

I think I can express my concerns more concretely. In any case with the unconstrained signature where you write something like the below:

_.merge<A, B, C>(a, b);

it is always safer to instead do:

<C> _.merge<A, B>(a, b)

These are functionally identical, except in the below version C is checked to be a possible result (assignable from A & B), and in the first C is totally unconstrained (it will compile if it's wrong). Any time you ever write the first example, you should write the second instead, because it is uniformly better (there is no incorrect code the second will compile that the first one won't, and there is some incorrect code the first will compile that the second won't).

(There are separate points about inferring bits of this that you touched on, which I'm going to leave for the moment. I think the above becomes more true the more you leave to inference though - it only gets better)

Motivating examples (with playground):

// All totally wrong, all happily compile with unconstrained generics

let a1 = _.merge<{a: number}, {b: number}, {c: number[]}>({a: 1}, {b: 2});
let b1 = _.merge<{a: number}, {b: number}, boolean>({a: 1}, {b: 2});
let c1 = _.merge<{a: number}, {b: number}, void>({a: 1}, {b: 2});

// Identical code, transformed as above, into assertions on the intersection-based signature
// All errors correctly caught by the compiler, unlike above

let a2 = <{c: number[]}> _.merge<{a: number}, {b: number}>({a: 1}, {b: 2});
let b2 = <boolean> _.merge<{a: number}, {b: number}>({a: 1}, {b: 2});
let c2 = <void> _.merge<{a: number}, {b: number}>({a: 1}, {b: 2});

Any time you write the first code, you should be writing the second; there are no cases where transforming as above from the unconstrained generics to these assertions makes it less safe, and there are many cases where it makes it more safe. I don't think we should duplicate all these signatures to add a second option that is equally or less safe in every case. Is that a bit clearer?

If you can find an example like the above, but where the generics version catch a mistake that wouldn't be caught by the equivalently transformed intersection version then I'd be very interested to see it. They're equivalent operations in the spec though, except for the lack of any constraints on the generic version, so I'd be very surprised.

This is interesting stuff though, and I'd never delved into it quite this far before now, thanks for bringing it up! It's definitely an interesting point. I think you're right though, we're going round in circles, we should let some others weigh in. Very keen for outside opinions!

[pimterry]

Any updates on this?

[chrootsu]

@pimterry oh, I was distracted and forgot to answer. I apologise. You have persuaded me, that your variant is better.
@vvakame please merge this PR 👍

[vvakame]

👍

[lukaw3d]

Do equal arguments hold for _.assign and _.extend functions?

[pimterry]

Indeed it does! I'll update those too shortly.

[lukaw3d]

@pimterry Looks like it is already in progress: #9388

[pimterry]

Ha, ok, excellent. Guess I won't then :-).