Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

loader-merge: Fix usage of object-rest-spread #890

Merged
merged 1 commit into from
May 26, 2018

Conversation

AlanFoster
Copy link
Contributor

According to the current version of babel, the usage of the object rest spread plugin name should be prefixed with transform

{
  "plugins": [
    ["transform-object-rest-spread", { "useBuiltIns": true }]
  ]
}

https://babeljs.io/docs/plugins/transform-object-rest-spread/#via-node-api

Copy link
Member

@edmorley edmorley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR! Keeping the docs up to date is really important.

The master branch is now using Babel 7, whose preset-env includes this plugin, which has also been renamed to proposal-object-rest-spread:
https://github.com/babel/babel/blob/v7.0.0-beta.47/packages/babel-preset-env/data/plugins.json#L227

Given that the plugin is now included, I wonder if there is another example "not-included-by-default" plugin we can use instead? Or else something generic like 'my-babel-plugin' so it never goes out of date causing confusion?

It would be good to fix the docs on the release/v8 branch to use the correct Babel 6 name of transform-object-rest-spread though.

@edmorley
Copy link
Member

Also, the docs are in two places in the repo - once under docs/packages/ and once under packages/ (an unfortunate side-effect of our current hosting strategy; soon to be fixed). Could you update both locations? :-)

@eliperelman
Copy link
Member

@AlanFoster would you be willing to update this PR based on @edmorley's comments?

According to the current version of babel, the usage of the object rest spread plugin name should be prefixed with `transform`

```
{
  "plugins": [
    ["transform-object-rest-spread", { "useBuiltIns": true }]
  ]
}
```

https://babeljs.io/docs/plugins/transform-object-rest-spread/#via-node-api
@AlanFoster
Copy link
Contributor Author

something generic like 'my-babel-plugin' so it never goes out of date causing confusion?

Good idea! PR updated 👍

@edmorley edmorley added the docs label May 26, 2018
Copy link
Member

@edmorley edmorley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! Many thanks for the PR :-)

@edmorley
Copy link
Member

The failed Netlify build is because master was broken at the time of the PR, so just going to ignore that result and merge anyway.

After this PR, I think we should also add object-rest-spread to the web preset on release/v8 as well as backport this docs fix there - which should help further with #883.

@edmorley edmorley merged commit 9915904 into neutrinojs:master May 26, 2018
edmorley added a commit that referenced this pull request Jun 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

3 participants