shkuznetsov / gulp-minify-inline

gulp plugin that minifies inline JS and CSS
MIT License
28 stars 5 forks source link

Throws an error when using ES6 template literals #18

Closed pensierinmusica closed 7 years ago

pensierinmusica commented 7 years ago

gulp-minify-inline (v0.2.1) throws an error when using ES6 template literals.

This works fine:

var win = window.open('https://youtu.be/' + this.videoId, '_blank');

This throws an error:

var win = window.open(`https://youtu.be/${this.videoId}`, '_blank');
SyntaxError: Unexpected character '`'
    at JS_Parse_Error.get (eval at <anonymous> (.../node_modules/gulp-minify-inline/node_modules/uglify-js/tools/node.js:27:1), <anonymous>:86:23)

It looks like the error comes from uglify-js, which doesn't seem to support ES6 syntax. Shall we switch to uglify-es instead? Do you want me to send a PR?

Cheers!

shkuznetsov commented 7 years ago

Hi @pensierinmusica

Yeah, looks like I have not updated the dependencies for far too long. I am keen to upgrade, but would want to better understand backward compatibility situation. Like will it run on pre-harmony versions of node? Also uglify-es looks much less mature than uglify-js. Shall we make it optional maybe?

pensierinmusica commented 7 years ago

@shkuznetsov mmm if I understand correctly they're just two different versions of the same software:

The package.json file of uglify-es says it supports Node >=0.8.0, so if that is correct it should work on pre-harmony.

When you run the tests does it give you any issue?

Hope this helps!

shkuznetsov commented 7 years ago

Oh you're so right, I've sooooo lost the thread, sorry. Yes please if you've got PR — please feel free to drop in. Otherwise I'll have a look in a few days.

shkuznetsov commented 7 years ago

Fixed, bumped major version (v1.0.0) due to underlying plugins API changes

pensierinmusica commented 7 years ago

Awesome!