Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spread operator is not correctly translated into JS #8856

Closed
tnikodym opened this issue May 27, 2016 · 41 comments

Comments

Projects
None yet
@tnikodym
Copy link

commented May 27, 2016

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

This comment has been minimized.

Copy link

commented May 28, 2016

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

This comment has been minimized.

Copy link
Contributor

commented May 29, 2016

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

@icfantv

This comment has been minimized.

Copy link

commented May 29, 2016

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

@Arnavion

This comment has been minimized.

Copy link
Contributor

commented Jun 1, 2016

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

This comment has been minimized.

Copy link

commented Jun 1, 2016

@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

This comment has been minimized.

Copy link
Contributor

commented Jun 1, 2016

@kitsonk

This comment has been minimized.

Copy link
Contributor

commented Jun 1, 2016

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

This comment has been minimized.

Copy link

commented Jun 1, 2016

@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

This comment has been minimized.

Copy link

commented Jun 4, 2016

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 #6974 for more information.

@mhegazy mhegazy closed this Jun 4, 2016

@mhegazy mhegazy added the Question label Jun 4, 2016

@icfantv

This comment has been minimized.

Copy link

commented Jun 5, 2016

@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

This comment has been minimized.

Copy link
Contributor

commented Jun 5, 2016

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

This comment has been minimized.

Copy link
Contributor

commented Jun 5, 2016

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

@mhegazy mhegazy reopened this Jun 5, 2016

@mhegazy mhegazy added Bug ES6 and removed Question labels Jun 5, 2016

@mhegazy mhegazy added this to the TypeScript 2.1 milestone Jun 9, 2016

@simonwiles

This comment has been minimized.

Copy link

commented Jul 23, 2016

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

@LPGhatguy

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2016

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

This comment has been minimized.

Copy link

commented Sep 28, 2016

nvm, see @Arnavion's reply below

@Arnavion

This comment has been minimized.

Copy link
Contributor

commented Sep 28, 2016

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

@mhegazy mhegazy modified the milestones: Future, TypeScript 2.1 Sep 29, 2016

@Zzzen

This comment has been minimized.

Copy link
Contributor

commented Nov 8, 2016

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

@PandaSandStudio

This comment has been minimized.

Copy link

commented Apr 21, 2018

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

This comment has been minimized.

Copy link

commented Jan 30, 2019

Is it going to be fixed?

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

@rbuckton

This comment has been minimized.

Copy link
Member

commented Apr 29, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.