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

Disable ES2015 transforms based on node version using babel-preset-env #878

Merged
merged 4 commits into from
Oct 11, 2016
Merged

Disable ES2015 transforms based on node version using babel-preset-env #878

merged 4 commits into from
Oct 11, 2016

Conversation

shubheksha
Copy link
Contributor

Fixes #863

presets: [
[require('babel-preset-env').default, {
"targets": {
"node": parseInt(process.versions.node),
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Let's change this to parseFloat as we discussed to fix features introduced in minor Node releases.
  2. This line misses an indentation.

module.exports = {
presets: [
[require('babel-preset-env').default, {
"targets": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: quotes are unnecessary here, let's leave just targets and node.

if (env === 'test') {
module.exports = {
presets: [
[require('babel-preset-env').default, {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add a comment above this line.
Something like

// ES features necessary for user's Node version

}
else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: please move else to the previous line, i.e.

} else {

@gaearon gaearon added this to the 0.7.0 milestone Oct 11, 2016
@gaearon
Copy link
Contributor

gaearon commented Oct 11, 2016

Looks great, thank you!

@gaearon gaearon merged commit 351e449 into facebook:master Oct 11, 2016
@hzoo
Copy link

hzoo commented Oct 11, 2016

We could totally do "node": parseInt(process.versions.node) as a string option for users that use the json config that would internally run parseFloat(process.versions.node) with a process value or something. babel/babel-preset-env#9 @shubheksha would you want to make a PR for that?

@gaearon
Copy link
Contributor

gaearon commented Oct 11, 2016

@hzoo Note I think it should be parseFloat, or comparison of e.g. 6 and 6.5 will say the feature is not supported.

@hzoo
Copy link

hzoo commented Oct 11, 2016

Oops yeah totally, I just copied the wrong thing from the diff 😛

@shubheksha
Copy link
Contributor Author

@hzoo - yes, gladly, thank you!

feiqitian pushed a commit to feiqitian/create-react-app that referenced this pull request Oct 25, 2016
facebook#878)

* Disable ES2015 transforms based on node version using babel-preset-env

* pass major version number for node to babel-preset-env instead of version string

* use parseFloat() instead of parseInt() to parse node version

* fixed style nits
jarlef pushed a commit to jarlef/create-react-app that referenced this pull request Nov 28, 2016
facebook#878)

* Disable ES2015 transforms based on node version using babel-preset-env

* pass major version number for node to babel-preset-env instead of version string

* use parseFloat() instead of parseInt() to parse node version

* fixed style nits
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants