4 ms·
This is wrong: (from the article) function f(leaves: Array<Leaf>, aggregators: Array<Aggregator>): Array<MemsqlNode> { // The next line errors because yo
by funkaster 8y ago
This is wrong: (from the article)
function f(leaves: Array<Leaf>, aggregators: Array<Aggregator>): Array<MemsqlNode> {
// The next line errors because you cannot concat aggregators to leaves.
return leaves.concat(aggregators);
}
That's just wrong, of course you can't concat aggregators and leaves! and Typescript is OK to not accept it. If you want that to be ok, you could do something like
[].concat(leaves, aggregators)
- paulddraper 8y agoAlso, you may consider [...leaves, ...aggregators]
- sephoric 8y agoIf the return type was Array<Leaf | Aggregator>, wouldn't TypeScript be able to infer the right type U in Array<T>.concat => Array<U> ?
- WorldMaker 8y agoNo, because JS concat is an in place operation, Typescript models it as Array<T>.concat() (intentionally leaves it "narrow") rather than Array<T>.concat<U extends T>(). In this case it is the input type that potentially would need to change so that the left hand side was Array<Leaf | Aggregator>. Or as others point out, array spread does widen unlike concat (because it isn't an in-place operation).
- WorldMaker 8y agoApparently concat does not operate in place, my mistake. I've been coding in TS for so long I assumed its definition was correct. https://news.ycombinator.com/item?id=18907771 https://news.ycombinator.com/item?id=18907771 https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/concat https://developer.mozilla.org/en-US/docs/Web/JavaScript/Refe...
- munchor 8y agoAuthor here. I understand this example may be a little controversial. However, from my point of view, if I annotate my function as returning an `Array<MemsqlNode>` and `leaves.concat(aggregators)` can be cast to `Array<MemsqlNode>`, then I don't see why I shouldn't be able to return this in this function without verbose syntax or "tricks".
- funkaster 8y agoit'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>.
- funkaster 8y agoLooking 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[];
- whatever_dude 8y agoI like his take at the end: > We converted a codebase that was adapted to Flow to TypeScript. This means that we obviously only found things that Flow can express but TypeScript can't. If the port had been the other way around, I'm sure we would have found things that TypeScript can infer/express better than Flow. So he's aware of the conundrum, and, I hope, his readers too.
- underwater 8y agoConcat doesn't mutate the original array, so there is no reason the output has to have the same value as the original array. Typescript has chosen to implement it this way, but it's not a limitation of the underlying JS method. Saying it's wrong because it's how Typescript has typed the function is circular reasoning. Consider the same functionality exposed as a function, it would seem be fine to type this as: function concat(a: Array<A>, b: Array<B>): Array<A | B> { }