bdkjones / CodeKit

CodeKit 3 Issue Tracker
https://codekitapp.com
82 stars 5 forks source link

'let' unsupported by uglify? #189

Closed mattpilott closed 8 years ago

mattpilott commented 8 years ago

Maybe i'm being a moron but I just tried to use let like the crazy cat I am and CodeKit failed to compile. By CodeKit I of course mean uglify.

Here's the code I tried to compile:

    let time = new Date().getHours();

    let hours = [
        4,
        12,
        17,
        23
    ];

    let greetings = [
        'Wow you\'re up late! Good morning.',
        'Good morning.',
        'Good afternoon.',
        'Good evening.'
    ];

    let greet = greetings[hours.findIndex(hour => hour > time)];

    document.querySelector('.js-greet').innerHTML = greet;

and here's the joyous little failure notice I received:

screen shot 2016-09-26 at 20 14 09

Let me know if i'm missing something please.

bdkjones commented 8 years ago

Correct. Uglify does not support ES6. That's why transpilers are in CodeKit 3. Turn on Babel.

-Bryan

Sent from my iPhone

On Sep 26, 2016, at 12:15, Matt Pilott notifications@github.com wrote:

Maybe i'm being a moron but I just tried to use let like the crazy cat I am and CodeKit failed to compile. By CodeKit I of course mean uglify.

Here's the code I tried to compile:

let time = new Date().getHours();

let hours = [
    4,
    12,
    17,
    23
];

let greetings = [
    'Wow you\'re up late! Good morning.',
    'Good morning.',
    'Good afternoon.',
    'Good evening.'
];

let greet = greetings[hours.findIndex(hour => hour > time)];

document.querySelector('.js-greet').innerHTML = greet;

and here's the joyous little failure notice I received:

Let me know if i'm missing something please.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

mattpilott commented 8 years ago

I have taken a look at uglify and see that they have a branch called 'harmony' which deals with es6. Any chance we can include that version as an option, I mean using Babel is fine, however it does seem silly to transpile just because I'm using let and/or const.

I realise this is not exactly your issue, as it's uglify putting in the limits but if that harmony branch does allow handling of es6 it sure would be a fantastic addition to codekit. I also realise we are in RC so perhaps CK3.1?

subhaze commented 8 years ago

@matt3224 I'd say using Babel is accurate, there's a preset in the works (alpha/beta) that minifies ES+ code.

https://github.com/babel/babili

You should be able to use this with CK as is via custom presets. It's... almost? beta. But worth checking out.

bdkjones commented 8 years ago

Ehh, I generally don't integrate beta stuff like alternate branches. It's a massive headache all around. Uglify will definitely adapt to deal with ES6 officially. Until then, Babel is the way to go.

Sent from my iPhone

On Sep 26, 2016, at 12:49, Michael Russell notifications@github.com wrote:

I'd say using Babel is accurate, there's a preset in the works (alpha/beta) that minifies ES+ code.

https://github.com/babel/babili

You should be able to use this with CK as is via custom presets. It's... almost? beta. But worth checking out.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

bdkjones commented 8 years ago

I also looked at Babili. It's more alpha than beta. They freely admit it fails all over the place and doesn't produce as small of an output file as Uglify. My money is on Uglify adding ES6 support. Besides, after my experiences with the rest of Babel, the last thing I want is to use more stuff written by that team. I trust it as far as I can throw it. On the other hand, the guy behind Uglify has fixed issues for me in an hour flat in the past. I like his work and I trust it.

Sent from my iPhone

On Sep 26, 2016, at 12:49, Michael Russell notifications@github.com wrote:

I'd say using Babel is accurate, there's a preset in the works (alpha/beta) that minifies ES+ code.

https://github.com/babel/babili

You should be able to use this with CK as is via custom presets. It's... almost? beta. But worth checking out.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

mattpilott commented 8 years ago

Ok cool thanks Bryan, @subhaze I have installed babili using sudo npm install babel-preset-babili -g and I added babili to the custom presets area of the Babel config in CK.

However I get an error: screen shot 2016-09-26 at 21 00 28

What have I done wrong here?

mattpilott commented 8 years ago

In that case Bryan, I will stick with uglify. Thanks @subhaze

bdkjones commented 8 years ago

Can't install Babel presets globally; you have to put them in your project folder under "node_modules". I tried to get Babel to fall back to global resolves, but they rejected that.

Sent from my iPhone

On Sep 26, 2016, at 13:03, Matt Pilott notifications@github.com wrote:

In that case Bryan, I will stick with uglify. Thanks @subhaze

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

subhaze commented 8 years ago

No doubt uglify would be the way to go, just suggesting the current way I'd go about it with the least friction of setup.

Full disclosure I've not used babili, just figured it was your best bet without a lot of work involved (if it doesn't fall flat on it's face for ya :P ).

MTCoster commented 7 years ago

I know this issue has been closed for a while, but uglify-es is now an officially supported option (still using the harmony branch) for es6 sources. Would it be possible to get an option to use this instead?

bdkjones commented 7 years ago

Transpile with Babel before minifying and you should be all set. The Uglify project is in a bit of...flux...right now. The main contributor isn't really putting out stable releases, which makes it very difficult to integrate.

Sent from my iPhone

On Aug 31, 2017, at 04:40, Matt Coster notifications@github.com wrote:

I know this issue has been closed for a while, but uglify-es is now an officially supported option (still using the harmony branch) for es6 sources. Would it be possible to get an option to use this instead?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.