SAP-archive / grunt-openui5

Grunt tasks around OpenUI5
Apache License 2.0
90 stars 34 forks source link

ES6+ support #69

Closed 4lexBaum closed 6 years ago

4lexBaum commented 6 years ago

FEATURE REQUEST

Currently it is not possible to use new JavaScript Features (const, let, Arrow Functions ...). Please update the compress task and replace the old UglifyJS2 module (supports only ES5) otherwise the whole preload task, which works great two years ago, is no longer usable...

RandomByte commented 6 years ago

Hi @4lexBaum,
For non-productive scenarios you could already use the new ui5-tooling which uses uglify-es and therefore supports ES6+ features.

We'll look into whether we should do the necessary changes to grunt-openui5, as the new tooling is meant to be its successor anyways.

surmiran commented 6 years ago

if you do not want to use ui5-tooling or wait for a solution, you can replace uglify-js with uglify-es yourself by editing the tasks/preload.js file in grunt-openui5:

Just replace the require statement: var uglify = require('uglify-es'); And remove the following line, as fromString is no valid option for uglify-es: options.compress.uglifyjs.fromString = true;

Don't forget to do a

npm i uglify-es

from within /node_modules/grunt-openui5/

I've run into the same issue as you and been building my preload files this way for some time now.

RandomByte commented 6 years ago

@MartinFluor you're welcome to create a PR for that! I think that change is pretty straightforward and fairly safe regarding compatibility.

4lexBaum commented 6 years ago

Hi @RandomByte @MartinFluor,

thanks for your fast feedback. I've also created a PR where I added the es6 option to enable the features (like for eslint). I've done the same steps locally as @MartinFluor yesterday in our projects and it works fine 👍

surmiran commented 6 years ago

@4lexBaum i guess making uglify-es available trough a flag is safer than straight up replacing the old uglify-js 👍

RandomByte commented 6 years ago

Thank you two! 🎉

However, I'm not aware of any incompatibilities that could be caused by hard-replacing uglify-js with uglify-es. As I said earlier, the new ui5-tooling also uses uglify-es and so far we did not face any issues when working with older/ES5 applications. Did you run into any issues so far?

surmiran commented 6 years ago

@RandomByte I've not encountered any issues yet while using ugilfy-es instead of ugilfy-js. I've been using that change for building apps which are in production.

DirkSW commented 6 years ago

Hi @RandomByte ... i like the solution with the flag #71 So it doesn't break something and others have the option to use this without branching out your github project. Will you merge this ?

RandomByte commented 6 years ago

Hi @DirkSW, I don't see any need for the flag. We are not aware of, and we do not expect any issues from switching to uglify-es. Do you know of any issues?

I think it is always better to only add flags if they serve a purpose and to offer the most features (like ES6 support) simply per default.

I would prefer #70. We just need to look into the failing unit tests (which would also fail for #71 if the es6 flag would have been set to true for those tests).

DirkSW commented 6 years ago

Hi @RandomByte , any news to ES6 support. #70 is as well fine with me.

matz3 commented 6 years ago

Regarding incompatibilities:

Switching to UglifyJS 3 will introduce a breaking change for the option compress.uglifyjs, as UglifyJS changed some options from v2 to v3 (see: https://github.com/mishoo/UglifyJS2). This applies for both uglify-js (no ES6) and uglify-es (ES6).

I don't mind introducing this breaking change to go forward and align to the same UglifyJS version we use in the new UI5 Tooling. We should just make sure to mention that in the release notes and make clear in which case one needs to take manual action when upgrading.

matz3 commented 6 years ago

Released with v0.14.0