Hacker News Reader

Comment by funkaster | Hacker News Reader

it's not a trick. Yes, it can be casted, but you should be explicit about it. AFAIK, the signature for the concat function is dependent on the array you're using it, so in this case, it's Array<Leaves>. Even though concat returns a new array, it is expecting the arguments to be of similar type. That's a "safe" behavior and something I would expect. Auto-casting to the return type (in this case) is something that I would not expect to happen, as it could be the source of errors.

Doing [].concat(Array<A>, Array<B>) is fine, because you did not specify the type of the array for the literal [], it's not a trick, in that case the type should be Array<any>, but it's casted to Array<MemsqlNode>.

View on HN

Right but isn't that effectively changing the runtime code (thus making it a trick) in order to fix a type error? I don't know if the performance of `a.concat(b)` is the same as `[].concat(a, b)`.

It's not a type error. I think the error is in your definition of a function and types like that. I would've defined your types like in this example[0]. However, I don't know enough about your codebase or why you're doing this.

[0]: http://www.typescriptlang.org/play/#src=type%20NodeType%20%3...

The argument is that you have a potential type error in `a.concat(b)` that you hadn't considered before in that code: If `a` is defined everywhere as Array<Leaf> and you push() something to it that is not a Leaf you potentially broke one of your own invariants somewhere else that uses `a`. In JS concat happens in place and is a mass push, so the same general concept holds.

`concat` doesn't happen in place, it builds a new array:

    https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/concat

True, but does not mean the type of the array that should be built should be of different types. I think the sane definition by Typescript is that it should be of the same type: that's why it's an instance method and not a static one. If this was a static method (Array.concat(a, b)), I would agree that would be a good assumption, that the type of the resulting array should be union of the types of the arguments.

Interesting. Then that should be a fixable bug in TS' lib files then.

There's an issue mentioning it with a PR Welcome sign up: https://github.com/Microsoft/TypeScript/issues/27545

Ah, and there is at least one PR specifically for it: https://github.com/Microsoft/TypeScript/pull/25716

Looking at the type definition, it is expecting the same type:

    /**                                                                                                                                                                                                                                                                                                                       
      * Combines two or more arrays.                                                                                                                                                                                                                                                                                          
      * @param items Additional items to add to the end of array1.                                                                                                                                                                                                                                                            
      */                                                                                                                                                                                                                                                                                                                      
    concat(...items: ConcatArray<T>[]): T[];                                                                                                                                                                                                                                                                                  
    /**                                                                                                                                                                                                                                                                                                                       
      * Combines two or more arrays.                                                                                                                                                                                                                                                                                          
      * @param items Additional items to add to the end of array1.                                                                                                                                                                                                                                                            
      */                                                                                                                                                                                                                                                                                                                      
    concat(...items: (T | ConcatArray<T>)[]): T[];

That's typescript's type definition.

Flow recognizes that concat returns a new array, so it specifies that calling concat on an Array<T> with an Array<S> as an argument returns an Array<T|S>. Their actual definition is:

    declare class $ReadOnlyArray<+T> {
        // concat creates a new array
        concat<S, Item: $ReadOnlyArray<S> | S>(...items: Array<Item>): Array<T | S>;

Personally, I'm ok with that and think it is useful. Although the return value could be an array of any type more general than T|S and the types of the argument arrays could vary. (i.e. Array<T>.concat(Array<S1>,Array<S2>,...): Array<? extends (T|S1|S2)>) but I don't know if that can be expressed in flow.

I don't think either gets it perfect, but flow is trying to at least capture the fact that the return value can't be an array of a type that is disjoint from T|S.

In practice, I've been impressed by the level of detail of information that is captured and propagated by flow's type checker.

Reply on news.ycombinator.com