Typescript: Spread operator is not correctly translated into JS

[tnikodym]

TypeScript Version: 1.8
Target: ES5

The following code:

[...(new Array(5))]

translates into:

(new Array(5)).slice();

However, the ES6 meaning is not the same. See the output below:

> [...(new Array(5))]
[ undefined, undefined, undefined, undefined, undefined ]
> (new Array(5)).slice();
[ , , , ,  ]

Expected behavior:

Array.apply(null, Array(5))

[icfantv]

My target is ES5 so not sure if this is related, but instances of Set are incorrectly converted to slice when using the spread operator. According to the MDN docs (scroll down to the _Relation with Array objects_ section) - I should be able to convert a Set instance to an array by simply doing:

let foo = [1,2,3];
let bar = new Set(foo);
[...bar] instanceof Array; // results in `true` in the Chrome console

But the TypeScript compiler converts [...bar] to bar.slice() which is not correct.

[basarat]

Perhaps the emit should use concat instead of slice :-/ :rose:

[icfantv]

I've found that Array.from(Set) works, but you need to make sure you have the typings installed.

[Arnavion]

Regarding your second question: @icfantv's question:

The spread operator on Set, etc is only supported when targeting ES6, since it requires the type to implement [Symbol.iterator]() which only exists in ES6. When targeting ES5, the spread operator is only allowed for arrays (and maybe for strings and other array-likes in the future).

So the compiler emits a call to .slice() because it expects to be emitting it for arrays. Note that it also emits an error complaining about using the spread operator on a non-array, so there's not really a problem.

[icfantv]

@Arnavion, if the compiler had spit out an error, I wouldn't have researched for a GH issue or a work-around as it would have been clear what the problem was. I didn't get an error until the code was actually executed in my application. Whether or not this is due to some TSC setting, I don't know, I just know it's an issue in our environment.

[Arnavion]

Playground

[kitsonk]

Of course that depends on how @icfantv acquired the types for Set in the first place... Some polyfills would have easily have tricked TypeScript into thinking it could do what it couldn't do.

For example:

interface Set<T> extends Array<T> {
}

interface SetConstructor {
    new <T>(): Set<T>;
}

declare var Set: SetConstructor;

[...new Set()]; // no errors

[icfantv]

@kitsonk excellent point. I am upgrading an Angular 1 app to Angular 2 and have to manage the typings myself to some extent and am at the mercy of those typings. While I would like to believe that they are solid, I have run into issues as noted above where the compiler will not complain but the generated code is not valid.

[mhegazy]

The compiler has no way to know if a definition is correct or not. if the definition says a Set is an Array, then it will be treated like one.

If you are using typescript@next consider using the --lib flag to specify the ES6-compliant set definition in --lib es2015.collection. see https://github.com/Microsoft/TypeScript/issues/6974 for more information.

[icfantv]

@mhegazy, can you please elaborate on what you mean when you say the compiler has no way of knowing if a definition is correct?

If I say [...Set], the compiler should not be generating invalid JavaScript. It's almost like you're saying the compiler uses the typings files to not only determine what is right syntax, but the actual generated code as well. Surely that can't be right.

[kitsonk]

Surely that can't be right.

It is correct. When figuring out how to down emit something, the compiler has to understand what it can and cannot do. When looking at the spread operator and a target of ES5, it says to itself "I can spread things that look like Arrays". The only way it knows if something is an Array or not is using the types it can reason out from the code. If it is passed something that allegedly is an Array, it will do what its down emit code says, which is to do a .slice(). If it doesn't match one of the patterns it doesn't know how to down-emit, it throws.

How else could it be done?

[Arnavion]

@mhegazy I feel the issue got side-tracked. Can you reopen for the original issue?

[simonwiles]

+1 for this original issue -- a straightforward case, I think?

[LPGhatguy]

I ran into this issue today writing some JSX using the spread operator; the way that TS handles the simplest spread case is strange!

Here's a specific StackOverflow answer recommending [...Array(N)] syntax for repeating an element N times: http://stackoverflow.com/a/29629588

[brettjurgens]

nvm, see @Arnavion's reply below

[Arnavion]

@brettjurgens This issue is about spread operator downlevel emit for arrays with holes. Your issue is #2696

[Zzzen]

Spread operator doesn't play well with iterators.
[...(new Map() as any)] => (new Map()).slice()

[dpogue]

This just bit us when dealing with a DOMStringList. The most common advice for converting DOMStringList to Array is to use [...stringlist] but TypeScript compiles that to stringlist.slice() and slice isn't defined on DOMStringList.

I believe Array.prototype.slice.call(stringlist) would work for this particular case

[huy-nguyen]

I ran into the same problem recently. Here's an example from the playground. The transpilation is wrong (because test.keys() is an iterator, not array) but I managed to fix it (on my machine) by setting the compiler option target to ESNext. The typescript team really needs to fix this problem.

[ghetolay]

Just had the same problem when used on Array.push()

const a = new Map();
a.set(1, 'a');

const b = [];
b.push( ...a.values() );

works with es6 but not with typescript since b.push( ...a.values() ) is transpiled to b.push.apply(b, a.values()).

[arackaf]

@kitsonk

How else could it be done?

Process it as a generic iterable, just like Babel does.

[kitsonk]

@arackaf things have changed... when targeting ES5 the compiler supports downlevel iterator which is similar to the Babel feature which allows objects to expose an iterator interface.

Technically this issue was fixed by the addition to that feature and could be closed.

[arackaf]

Oh man, that's awesome! Didn't know.

[huy-nguyen]

@kitsonk I encountered this error last month (mentioned in a comment above) while targeting ES6. Are you sure this is fixed?

[kitsonk]

@huy-nguyen the playground is always targets ES5 and does not include the optional --downlevelIteration flag.

You could not have encountered this error if you were targeting ES6, as the down emit described above would only occur when targeting ES5. I get the following when I run it through tsc:

$ cat test.ts
const test = new Map([[1, 'a'], [2, 'b'], [3, 'c']]);
console.log([...test.keys()]);

$ tsc test.ts --target es6
$ cat test.js
const test = new Map([[1, 'a'], [2, 'b'], [3, 'c']]);
console.log([...test.keys()]);

I also get no error and the following output when using the --downlevelIteration:

$ tsc test.ts --target es5 --downlevelIteration --lib es6,dom
$ cat test.js
var __read = (this && this.__read) || function (o, n) {
    var m = typeof Symbol === "function" && o[Symbol.iterator];
    if (!m) return o;
    var i = m.call(o), r, ar = [], e;
    try {
        while ((n === void 0 || n-- > 0) && !(r = i.next()).done) ar.push(r.value);
    }
    catch (error) { e = { error: error }; }
    finally {
        try {
            if (r && !r.done && (m = i["return"])) m.call(i);
        }
        finally { if (e) throw e.error; }
    }
    return ar;
};
var __spread = (this && this.__spread) || function () {
    for (var ar = [], i = 0; i < arguments.length; i++) ar = ar.concat(__read(arguments[i]));
    return ar;
};
var test = new Map([[1, 'a'], [2, 'b'], [3, 'c']]);
console.log(__spread(test.keys()));

This of course requires a global polyfill for Map() and Symbol at run-time.

[arackaf]

Seconded - if you're targeting ES6 it'll just leave all ES6 code alone. Nothing for it to possibly break.

[marvinhagemeister]

Awesome, didn't know about that flag. Perhaps --downlevelIteration can be directly included when setting the target to ES5? Without knowing the reasons behind making it a specific flag it seems like that'd be a good default setting.

[kitsonk]

@marvinhagemeister it requires that a global Symbol polyfill to be available at the runtime environment and that any iterables expose an iterable interface "symbol". Because of those pre-reqs, I think the core team would be reticent to make it a default.

[aik-jahoda]

Please read the original bug again, it is not about iterators it is about simple bug in:
[...(new Array(5))] which emit wrong code.

Let change attention to this bug or reopen #14088 where the discussion didn't get wrong direction.

[marvinhagemeister]

@aik-jahoda an array in js implements the iterable interface and is thus by definition an iterable.

[aik-jahoda]

@marvinhagemeister ypoi are right. --downlevelIteration fixes the issue however without it it still produce wrong output.

For [...(new Array(5))] should emit Array.apply(null, Array(5)) instead of (new Array(5)).slice();.

This is why I consider --downlevelIteration as a workaround instead of proper fix as the bug stil exists.

[kitsonk]

@aik-jahoda when using --downlevelIteration all iterators are spec compliant, so again, this could be closed, though it requires used of --downlevelIteration:

$ cat test.ts
console.log([...(new Array(5))])
$ tsc test.ts --target es5 --downlevelIteration --lib es6,dom
$ cat test.js
var __read = (this && this.__read) || function (o, n) {
    var m = typeof Symbol === "function" && o[Symbol.iterator];
    if (!m) return o;
    var i = m.call(o), r, ar = [], e;
    try {
        while ((n === void 0 || n-- > 0) && !(r = i.next()).done) ar.push(r.value);
    }
    catch (error) { e = { error: error }; }
    finally {
        try {
            if (r && !r.done && (m = i["return"])) m.call(i);
        }
        finally { if (e) throw e.error; }
    }
    return ar;
};
var __spread = (this && this.__spread) || function () {
    for (var ar = [], i = 0; i < arguments.length; i++) ar = ar.concat(__read(arguments[i]));
    return ar;
};
console.log(__spread((new Array(5))));
$ node test.js
[ undefined, undefined, undefined, undefined, undefined ]

[aik-jahoda]

Then we are in situation when without --downlevelIteration some array (not other collections) operatin works and some simple emit wrong code. Again - wrong emitted code I consider as a bug.
Examle:
[...[1,2,3]] //emits correct code: [1,2,3]

however
[...Array(5)] // emits broken code: Array(5).slice()

How can I decide to enable --downlevelIteration in case sometimes it works without it sometimes not?

Shouldn't compilator emit error for [...Array(5)] in case --downlevelIteration is disabled instead of emitting wrong JS?

[gcnew]

Honestly, I find it more interesting why you need/rely on this behaviour? It doesn't seem the suggested code is a common usage pattern or anywhere near a best practice. It is also really easy to workaround. And at the end of the day we are talking about byte code for ageing technologies, which is already handled. Putting an ounce more of effort/discussion seems like a big waste of time.

[kitsonk]

Then we are in situation when without --downlevelIteration some array (not other collections) operatin works and some simple emit wrong code.

Without --downlevelIteration other collections simply error and emit something non-sensical.

Of course since the issue is still open and as a bug, the core team may consider changing the emit without the compiler flag.

Also, it isn't broken code, it is code that has a slightly different behaviour to the spec ES6 code. Obviously having code the behaves differently is not good, but what real world example where the spec behaviour is critical to the correct operation of the code?

[aik-jahoda]

I completely agree that --downlevelIteration is necessary for the collection types like Map.
The difference between array and other collections is:

• array is first citizen class and is in ES5 and prior.
• other collections are added in ES6 and they depend on ES6 iterators. This is why for example for...of on map can't work without --downlevelIteration.

If projec target ES5 it doesn't mean it need iterator for array operation and compiler can emit code which works well without iterator.

Lets use for...of as an example:

Currently it works fine for array, if you use Map, it will produce invalid code as it will treat Map as array. - it is fine as ES5 doesn't support iterators and you did just invalid construct (I would appreciate an error)

If you need iterate over Map, --downlevelIteration is the option and it brings iterators into the game (however only if your engine supports Symbols)

--downlevelIteration is great functionality and make perfect sense and it can be used as workaround of this bug.

[s35]

We ended up using Array.from() with the necessary typings and polyfills to make the spread operator work.

[litera]

@s35 Instead of using typings and polyfills wouldn't it be easier if you replaced your Array.from with [].concat(new Set(...))? It works as expected...

[anthonyrota]

Array.from converts the iterable into an array.

The code [...iterable] should be translated into the equivalent of Array.from(iterable) as by definition [...iterable] translates the iterable into the array, just like the Array.from method.

For example typescript (without the flag mentioned above) would translate [...'abcdefg'] into 'abcdefg'.slice() instead of Array.from('abcdefg') which both produce different outputs, and only the latter is spec-compliant

[EKashpersky]

Is it going to be fixed?

I mean hey, it's almost 3 years old.

[rbuckton]

I am not certain there is anything actionable here. We previously decided that TypeScript's default ES5 transpile for spread and for..of (which forgo precise runtime semantics over runtime performance) would remain as-is, and introduced --downlevelIteration as a way to opt-in to the slower but more correct runtime semantics.