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
Correctly transform spreads to use proper concat method #9108
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/9551/ |
...ugin-transform-destructuring/test/fixtures/destructuring/array-unpack-optimisation/output.js
Outdated
Show resolved
Hide resolved
One bad thing about this PR is that before it if somebody tried to spread Set in loose mode, clear error was thrown, but now he will get wrong output of |
I agree with @smashercosmo. This also allows to spread non-iterable values again, which is wrong (e.g.: |
Sorry if this is the wrong place to comment, but this has references to a lot of related history. In a modern browser that supports the spread operator natively, This doesn't seem like the intended result with Babels support of the spread operator? |
@fdanielsen I'm seeing the same thing as you, with |
The loose transform only handles Arrays. You'll have to use the spec transform (as well as polyfilling |
I don't see a Are the docs out of date? Happy to send a PR if that is the case 😃 Edit: oops, I didn't realize I had preset-env configured with |
Spec in this case is the default. We only add an explicit |
Got it. Thanks for the clarification 👍 |
* Correctly transform spreads to use proper concat method * Add tests to ensure array spread clones elements
This reverts some of the optimizations of #6763.
Transforming a spread to this
is dangerous as we do not know what
arr
is exactly. It might be a subclass ofArray
with changed behavior of its concat function.Instead I changed it to use
Other option would be to use what was there before #6763
which is smaller, but needs to create an additional array.
Also added comments to hopefully make the logic understandable.