Hacker News Reader

Comment by munchor | Hacker News Reader

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)`.

View on HN

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

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

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.

Reply on news.ycombinator.com