Skip to content
This repository has been archived by the owner on May 11, 2018. It is now read-only.

Add uglify as a target #178

Merged
merged 9 commits into from
Mar 3, 2017
Merged

Add uglify as a target #178

merged 9 commits into from
Mar 3, 2017

Conversation

yavorsky
Copy link
Member

@yavorsky yavorsky commented Mar 2, 2017

Add uglify as a target. Fixes #134.
I've not given attention to uglify's harmony brunch because of there no info about what ES6 features they currently support and how stable this support is (seems like not much for now).

Also, as for me, there is no difference for us between uglify and uglify 2. Both haven't es6 support at all. So allowed uglify values are numbers and booleans.

If uglify hasn't es6 support, what's the difference between no targets specifying at all?
The difference is in built-ins. For uglify no matter what polyfills you are using - it's just methods and there are no problems to parse them.

Ex:
.babelrc

{"presets":
  [
    ["env", {
      "targets": {
        "chrome": 55,
        "uglify": true
      },
      "useBuiltIns": true,
      "modules": false
    }]
  ]
}

Will use all the possible plugins and only built-ins based on specified targets (except uglify).

@yavorsky yavorsky changed the title Uglify Add uglify as a target Mar 2, 2017
Copy link
Member

@existentialism existentialism left a comment

Choose a reason for hiding this comment

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

Awesome!

README.md Outdated

`number | true`

If you are using minification with uglify then targeting later browsers would throw a syntax error. To prevent it - specify `uglify` option. It will just use all plugins and as result compile fully to ES5, but with `useBuiltIns` it will include only polyfills according to your targets.
Copy link
Member

Choose a reason for hiding this comment

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

Just a couple grammar nits:

If you are using UglifyJS to minify your code, then targeting later browsers will throw a syntax error.

To prevent this - specify the uglify option, which will enable all plugins and as a result, fully compile your code to ES5. Note, that useBuiltIns will work as before, and only include the polyfills that your target(s) need.

Copy link
Member Author

Choose a reason for hiding this comment

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

@existentialism Oh, thanks!!! 😊

@existentialism
Copy link
Member

@yavorsky minor, but I pushed a debug test when using uglify output (wanted to make sure it shows "uglify: true)

targetOpts.node = getCurrentNodeVersion();
}

if (targetOpts.hasOwnProperty("uglify") && !targetOpts.uglify) {
Copy link
Member

Choose a reason for hiding this comment

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

If we didn't have to do this, would be good? doesn't seem necessary

Copy link
Member Author

@yavorsky yavorsky Mar 3, 2017

Choose a reason for hiding this comment

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

It will allow null or false and use it exactly like a true. I think we must check other targets the same way.

@@ -122,6 +122,14 @@ A query to select browsers (ex: last 2 versions, > 5%) using [browserslist](http

Note, browsers' results are overridden by explicit items from `targets`.

### `targets.uglify`

`number | true`
Copy link
Member

Choose a reason for hiding this comment

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

although currently we only support uglify2

Copy link
Member Author

Choose a reason for hiding this comment

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

@hzoo I think 1 also. No?

Copy link
Member

Choose a reason for hiding this comment

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

I guess I just mean it's the same for whatever value

@felixfbecker
Copy link

This doesn't seem to enable transpilation of new.target - I get an UglifyJS error

Unexpected token: punc (.)

@hzoo
Copy link
Member

hzoo commented Sep 13, 2017

@felixfbecker it wasn't supported at all in Babel until recently, and we didn't add it to the preset yet, and maybe you should try the newer uglify version?

@felixfbecker
Copy link

@hzoo Sorry, I assumed so because the PR was merged in July: babel/babel#5906
What would I need to do to get it working?

@hzoo
Copy link
Member

hzoo commented Sep 13, 2017

It's actually because we never actually added it to the env/es2015 presets. I guess you could do it manually in plugins for now as a workaround

@felixfbecker
Copy link

Shouldn't it be included through the class transform?

The new.target property, is transpiled to this.constructor. As, with pure function constructor based prototype inheritance, the this instance is created by the child class, and given to the parent class constructor & methods (see super), this.constructor refers to the child class prototype constructor property, itself fixed to the child class function itself, matching to the constructor invoked by new.

https://www.npmjs.com/package/babel-plugin-transform-class#newtarget

Could I do a PR to add it?

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