Enable Object Rest Spread by default #1835
Conversation
|
This is not a standard yet, not sure if this is a good idea |
|
@DeMoorJasper It is part of ES2018 as I already mentioned in the PR description. More points that it is worth enabling this now: /~https://github.com/tc39/proposals/blob/master/finished-proposals.md Shipping in Chrome, Firefox, Safari (both macOS and iOS), V8: http://kangax.github.io/compat-table/es2016plus/#test-object_rest/spread_properties |
|
Now I need to find out why it passes locally but not on the Travis/AppVeyor |
|
@arv you still have to add it as a dependency if I'm not mistaken. That's probably the reason |
|
@DeMoorJasper I need to figure out the test failures. Any ideas? |
|
@arv Unfortunately I don't have any clue why it's failing |
|
I can look at it tomorrow. |
test/javascript.js
Outdated
| @@ -28,6 +28,21 @@ describe('javascript', function() { | |||
| assert.equal(output.default(), 3); | |||
| }); | |||
|
|
|||
| it('should produce a basic JS bundle with object rest spread support', async function() { | |||
| let b = await bundle(__dirname + '/integration/object-rest-spread/object-rest-spread.js'); | |||
There was a problem hiding this comment.
It wasn't bundling anything, got it it bundle correctly in the test now, not sure how this worked locally. Now it still throws an error though.
Hope you can resolve it :)
arv
force-pushed
the
object-rest-spread
branch
from
b6bdab8 to
3ecb57c
Compare
| @@ -265,6 +265,14 @@ async function getEnvPlugins(targets, useBuiltIns = false) { | |||
| {}, | |||
| {targets, modules: false, useBuiltIns: useBuiltIns ? 'entry' : false} | |||
| ).plugins; | |||
|
|
|||
| // babel-preset-env version 6.x does not cover object-rest-spread so always | |||
There was a problem hiding this comment.
This does not seem like the right place for this but I'm not sure where would be a better place to put this?
There was a problem hiding this comment.
It should be fine here, but than you should probably remove it from JSAsset.js as now it probably runs it twice...
There was a problem hiding this comment.
On sidenote though, this would also run over node_modules, not sure if that's intended
There was a problem hiding this comment.
It should be fine here, but than you should probably remove it from JSAsset.js as now it probably runs it twice...
The one in JSAsset.js seems to only be used for parser options. Inspecting the plugins array we get from getEnvPlugins and it does not contain babel-plugin-transform-object-rest-spread
On sidenote though, this would also run over node_modules, not sure if that's intended
That is what I want since npm modules are now starting to use this feature.
There was a problem hiding this comment.
Yes, but babylon runs before the other transforms to create the ast, so this would run twice, at different places, but it'll still run twice
There was a problem hiding this comment.
That makes sense. Thanks.
|
Thanks |
|
Is this released ? |
|
No |
This uses Babel's plugin:
https://babeljs.io/docs/en/babel-plugin-transform-object-rest-spread
Spec:
https://tc39.github.io/ecma262/#sec-object-initializer-runtime-semantics-propertydefinitionevaluation
Fixes #1221, #1422