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 · Fixed by #31166
Closed

Spread operator is not correctly translated into JS #8856

tnikodym opened this issue May 27, 2016 · 41 comments · Fixed by #31166
Assignees
Labels
Bug A bug in TypeScript ES6 Relates to the ES6 Spec

Comments

@tnikodym
Copy link

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
Copy link

icfantv 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
Copy link
Contributor

basarat commented May 29, 2016

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

@icfantv
Copy link

icfantv commented May 29, 2016

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

@Arnavion
Copy link
Contributor

Arnavion 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
Copy link

icfantv 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
Copy link
Contributor

Arnavion commented Jun 1, 2016

untitled

Playground

@kitsonk
Copy link
Contributor

kitsonk 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
Copy link

icfantv 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
Copy link
Contributor

mhegazy 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 as completed Jun 4, 2016
@mhegazy mhegazy added the Question An issue which isn't directly actionable in code label Jun 4, 2016
@icfantv
Copy link

icfantv 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
Copy link
Contributor

kitsonk 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
Copy link
Contributor

Arnavion 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 A bug in TypeScript ES6 Relates to the ES6 Spec and removed Question An issue which isn't directly actionable in code labels Jun 5, 2016
@mhegazy mhegazy added this to the TypeScript 2.1 milestone Jun 9, 2016
@simonwiles
Copy link

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

@LPGhatguy
Copy link
Contributor

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
Copy link

brettjurgens commented Sep 28, 2016

nvm, see @Arnavion's reply below

@Arnavion
Copy link
Contributor

@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
Copy link
Contributor

Zzzen commented Nov 8, 2016

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

@EKashpersky
Copy link

Is it going to be fixed?

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

@rbuckton
Copy link
Member

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.

@DanielRosenwasser DanielRosenwasser removed Good First Issue Well scoped, documented and has the green light Help Wanted You can do this labels May 24, 2019
bluebill1049 pushed a commit to react-hook-form/react-hook-form that referenced this issue Jul 23, 2019
* Change target to es2015 to support ie11

* Change tsconfig build target to es5

* Add downlevelIteration to compilerOptions to support ES5. Read more here: microsoft/TypeScript#8856
Gnuxie added a commit to the-draupnir-project/Draupnir that referenced this issue Sep 4, 2023
I will consult a friend to verify that I am not missing something obvious.
Apparently it was probably a result of microsoft/TypeScript#8856
Gnuxie added a commit to the-draupnir-project/Draupnir that referenced this issue Sep 5, 2023
I will consult a friend to verify that I am not missing something obvious.
Apparently it was probably a result of microsoft/TypeScript#8856
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript ES6 Relates to the ES6 Spec
Projects
None yet
Development

Successfully merging a pull request may close this issue.