google / google/elemental2

JsArray.concat has incorrect generics

Open
#184 3 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
170
Forks
41
PR merge metrics
No merged PRs in 30d

Description

In both JsArray and ReadonlyArray, concat() is specified as

```java
public native JsArray concat(T... items);
```

In closure's externs however, it is defined as
https://github.com/google/closure-compiler/blob/39363c2c57dcdc365c739ec3f13d4e10ef9a204c/externs/es3.js#L830-L841
```javascript
/**
* Returns a new array comprised of this array joined with other array(s)
* and/or value(s).
*
* @param {...*} var_args
* @return {!Array}
* @this {*}
* @nosideeffects
* @override
* @see http://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/concat
*/
Array.prototype.concat = function(var_args) {};
```

This is an important loss of functionality, as concat should be able to take an array and iterate its contents, rather than only accepting a varargs of the same contents.

For example, given two gigantic arrays:
```javascript
const oneMillionApples = "apples".repeat(1000000).slice(0,-1).split(",");
const oneMillionOranges = "orange".repeat(1000000).slice(0,-1).split(",");
```
In plain JS, you can concat these - the arrays are the same type, and we don't need to do something like array destructuring, which would blow the stack
```javascript
const tooManyItems = oneMillionApples.concat(oneMillionOranges);
```
but given the current type signature of concat() in Java, this can't work - concat's args must be varargs of T. Trying to treat the JsArray as String[] will mean we have to translate java varargs semantics to js varargs semantics, and blow the stack.

Contrast this with the closure types, which seem to allow any args at all (since an array can be heterogeneous).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.