Error transpiling spread Array factory

[VitorLuizC]

Bublé parses [ ...Array(length) ] as [].concat( Array(length) ) witch is not equivalent.

Spreading Array factory returns an array with undefined values.

[ ...Array(2) ] === [undefined, undefined]
[ ...Array(2) ].length === 2

Concating Array factory returns an empty array with defined length.

[].concat( Array(2) ) === []
[].concat( Array(2) ).length === 2

What I get?

[].concat( Array(length) )

What I expected?

  Array.apply( null, Array(length) )

OR

  Array.prototype.concat.apply( [], Array(length) )

OR

  [].concat.apply( [], Array(length) )

[adrianheine]

Another example for this (not correctly transpiled currently):

var a = Array(...[,,]);
return "0" in a && "1" in a && '' + a[0] + a[1] === "undefinedundefined";

[m59peacemaker]

Similarly, I just found that trying to spread a Set instance just results in an array containing the set. It's the same problem. [].concat(aSet) doesn't do the same as the spread operator.

[ ...new Set([ 1, 2, 3 ]) ]
// expected => [ 1, 2, 3 ]
// actual => [ Set{1, 2, 3} ]

For spreading iterables, would an acceptable solution be to check if the value being spread is an iterable, and if so, call .entries on it?

[adrianheine]

And then there are strings, which aren't split by our spread implementation either. The same issues also apply to destructuring:

const [a, b] = new Set([1, 2, 3]);

Apparently these buggy behaviors are rarely used in the wild, otherwise I would have expected some bug reports about it.

I think an acceptable solution should

1. Not require a lot of code
2. Not substantially degrade performance for common cases
3. Work in target environments, e. g. IE8 and Node 0.10.

Array.from is not acceptable for example.

[kzc]

Related:

Spread operator and NodeLists https://gitlab.com/Rich-Harris/buble/issues/81

Support iterables in dangerous for-of loops https://gitlab.com/Rich-Harris/buble/merge_requests/109

Strict mode proposal https://gitlab.com/Rich-Harris/buble/issues/44

[m59peacemaker]

@adrianheine Shouldn't Array.from exist anywhere Map and Set do? The caveat that Array.from needs a polyfill if you're polyfilling and spreading Set and Map makes sense to me. Just like Object.assign when you're spreading objects.

[adrianheine]

We don't know at transpile-time if we're spreading a Map or a Set or a string or a plain array.

[zanona]

By any chance, is there any workaround for using spread on NodeList such as [...document.querySelectorAll('a')] as this works natively on modern browsers I don't think users would expect this to be changed to a different behaviour when the code is transpiled.

Coincidentally, we had issue 81 on Gitlab as @kzc referenced.

[nathancahill]

@zanona transforms: { spreadRest: false } will skip transpiling.

[zanona]

@nathancahill yes, but then it will break on unsupported browsers isn't that correct?

I am assuming that for using bublé, anyone using the spread on array-like structures will need to refactor at the moment?

Is there a solution for this use case at the moment?

[nathancahill]

No, because Bublé has no knowledge of the type, it can't transpile differently based on the type. The solution is to inject a helper function like Babel does, which Bublé doesn't support (at all?) yet. All transforms are done in place.

[zanona]

Thanks, @nathancahill that makes sense. Currently Buble transpiles Array spreads to [].concat( a ). The problem with this is that it really gets users by surprise since there's no way to fix it unless changes are made to the codebase (which I'm sure it's fine in most cases) but for libraries that rely on bublé to transpile arbitrary client code it may be a bit of a let down to impose such limitation since the transpiled code would not only break on unsupported browsers but also stop working on modern ones as well since [].concat(document.querySelectorAll('a')) is then an array of arrays [[(100)a]] and not [(100)a] as expected.

I agree and am completely in favour of reducing the size of the generated code using shims to sustain that before running. But wouldn't translating array spreads into [].concat(Array.from(a)) instead, allow users to keep using spread on arrays without breaking the behaviour at least?

It still works the same way whenever an array is passed [].contact(Array.from([1,2,3])) but also for instances that can be converted into arrays with Array.from such as NodeLists.

[nathancahill]

Only modern browsers support Array.from. [].concat() support goes back to ES3.

[zanona]

Yes, but you can use a shim for that can't you? Things you cannot do if you have [...NodeList] in which case it will simply break and there's no way out.

[adrianheine]

I understand that this is really bad. Adding a shim to bublé seems not in line with the project's current design, though. Is Array.prototype.slice.call( list ) a workable solution?

[zanona]

@adrianheine Amazing Idea! It works great. I believe shims can be added by users on their end, but your suggestion is hopefully backwards compatible and addresses spread on NodeLists. @nathancahill how does that sound to you?

[nathancahill]

Sounds great. 🙌🏻

[nathancahill]

Looking in to this a little more after some coffee. NodeList works fine. Set/Map do not since Array.prototype.slice.call( set/map ) return an empty array.

I'm think @m59peacemaker's suggestion to have a config option like objectAssign is a better option. Call it arrayFrom.

It can default to Array.from when targeting compatible browsers, and ` when targeting older browsers. That way,[].concat()` still works in older browsers where there's no Set/Map anyway. If users want to polyfill, it makes it easy too.

[adrianheine]

That would mean it would silently switch to producing the current, broken code if you change your compile target, right? If the set of engines with Set/Map support were a subset of those with Array.from, that would somewhat work as a crude hack, but that is not the case. What about the following:

const toApplyParam = function(v) {
  if (typeof Symbol !== "undefined" && Symbol.iterator && v[Symbol.iterator]) {
    var iterator = v[Symbol.iterator]()
    var res = []
    while (!(v = iterator.next()).done) res.push(v.value)
    return res
  }
  return v
};

[nathancahill]

As a shim or inline?

[adrianheine]

I was thinking of something like https://github.com/Rich-Harris/buble/blob/8f4155682bb803f4edb2c932182a0ba3bd7f6096/src/program/Program.js#L63, i. e. a helper function that gets added to the output if necessary.

[adrianheine]

I think #8 is relevant for this as well.

[zanona]

Hey guys, sorry to bump this issue. But just checking if we could agree on the best way to implement this. It's something that there's no workaround at the moment unless changing the input code. Quite a few possible solutions have been presented already, so hopefully, we could think about solving a problem that exists in a graceful way?

Thanks again

[julienetie]

@zanona

[].concat(document.querySelectorAll('a')) is then an array of arrays [[(100)a]] and not [(100)a] as expected.

At a glance [].slice.call(document.querySelectorAll('a')) results in values for array and array-like. Would this be a feasible solution or am I overlooking something?

[VitorLuizC]

@julienetie this wouldn't solve my initial problem.

[].slice.call(Array(2))
//=> (2) [empty × 2]
[...Array(2)]
//=> (2) [undefined, undefined]

Maybe [].concat.apply([], document.querySelectorAll('a')) would solve it, since it covers both cases.

[].concat.apply([], Array(2))
//=> (2) [undefined, undefined]
[...Array(2)]
//=> (2) [undefined, undefined]

[thednp]

I'm here for [...document.querySelectorAll('a')] transformed into [].concat(NodeList) instead of [].concat(Array), however Array.from(document.querySelectorAll('a')) is working fine.

Suggestions?

[imperatormk]

No progress on this yet? Trying to process Vue.js and this file is failing: https://fossies.org/linux/vue-next/packages/runtime-core/src/scheduler.ts

const deduped = [...new Set(pendingPostFlushCbs)] is transpiled to var deduped = [].concat( new Set(pendingPostFlushCbs) ) and since I have no control over this code it's rather impossible to fix. I also can't lose transforms.spreadRest.

Please let me know if there are any means of applying a workaround or anything