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

Using Uglify 3.0 results in errors #636

Closed
JeroenVinke opened this Issue Jun 1, 2017 · 6 comments

Comments

Projects
None yet
4 participants
@JeroenVinke
Member

JeroenVinke commented Jun 1, 2017

I'm submitting a bug report

  • Library Version:
    0.29.0

Please tell us about your environment:

  • Operating System:
    OSX 10.x|Linux (distro)|Windows [7|8|8.1|10]

  • Node Version:
    6.2.0

  • NPM Version:
    3.8.9
  • Browser:
    all | Chrome XX | Firefox XX | Edge XX | IE XX | Safari XX | Mobile Chrome XX | Android X.X Web Browser | iOS XX Safari | iOS XX UIWebView | iOS XX WKWebView

  • Language:
    all | TypeScript X.X | ESNext

Current behavior:
When uglify-js@3.0.0 is used, production builds fail:

image

Digging a bit further yields the follow error:

{ error: 
   { DefaultsError: `fromString` is not a supported option
       at DefaultsError.get (eval at <anonymous> (/Users/romainlanz/workspace/fivb/app.fivb.org/node_modules/uglify-js/tools/node.js:21:1), <anonymous>:86:23)
       at formatError (util.js:640:15)
       at formatValue (util.js:544:18)
       at formatProperty (util.js:798:15)
       at util.js:646:12
       at Array.map (native)
       at formatObject (util.js:645:15)
       at formatValue (util.js:585:16)
       at inspect (util.js:200:10)
       at exports.format (util.js:66:24)
     message: '`fromString` is not a supported option',
     defs: 
      { compress: {},
        ie8: false,
        keep_fnames: false,
        mangle: {},
        output: {},
        parse: {},
        sourceMap: false,
        timings: false,
        toplevel: false,
        warnings: false,
        wrap: false } } }

This is because Uglify-js@3 has a new API that is not backwards compatible with uglify-js@2:

image

Expected/desired behavior:

  • What is the expected behavior?
    aurelia-cli to work with uglify-js@3

  • What is the motivation / use case for changing the behavior?

@sam-piper

This comment has been minimized.

Show comment
Hide comment
@sam-piper

sam-piper Jun 16, 2017

We just tried upgrading to uglify-js@3.0.15 - au build --env prod still fails, but there are no errors, instead the app-bundle.js / vendor-bundle.js files just contain the text "undefined".

Will wait for an update to aurelia-cli here.

sam-piper commented Jun 16, 2017

We just tried upgrading to uglify-js@3.0.15 - au build --env prod still fails, but there are no errors, instead the app-bundle.js / vendor-bundle.js files just contain the text "undefined".

Will wait for an update to aurelia-cli here.

@Aniel

This comment has been minimized.

Show comment
Hide comment
@Aniel

Aniel Jun 16, 2017

@sam-piper same here without --env The build works. If you specify an environment the bundle has the text undefined

Aniel commented Jun 16, 2017

@sam-piper same here without --env The build works. If you specify an environment the bundle has the text undefined

@huochunpeng

This comment has been minimized.

Show comment
Hide comment
@huochunpeng

huochunpeng Jun 21, 2017

Contributor

There is another issue, besides unsupported "fromString".

line 242 in bundle.js

let minificationOptions = Object.assign({ fromString: true }, buildOptions.getValue('minify'));

When you assign an object {} with a string "lorem", you got {"0": "l", "1": "o", "2": "r", "3": "e", "4": "m"}, that's what happened here.

The "minify" value "stage & prod" was literally injected to minificationOptions like a char array.

I inspected minificationResult in few lines below. See error

'`0` is not a supported option'.

Didn't see "fromString" error here, probably on my computer, uglify reads minificationOptions property "0" before property "fromString".

@JeroenVinke , the question is why we need Object.assign({ fromString: true }, buildOptions.getValue('minify')) in the first place?

Also suggest to add checking on minificationResult (could return error) before using its code and map, to avoid unreadable error message from undefined sourcemap.

Contributor

huochunpeng commented Jun 21, 2017

There is another issue, besides unsupported "fromString".

line 242 in bundle.js

let minificationOptions = Object.assign({ fromString: true }, buildOptions.getValue('minify'));

When you assign an object {} with a string "lorem", you got {"0": "l", "1": "o", "2": "r", "3": "e", "4": "m"}, that's what happened here.

The "minify" value "stage & prod" was literally injected to minificationOptions like a char array.

I inspected minificationResult in few lines below. See error

'`0` is not a supported option'.

Didn't see "fromString" error here, probably on my computer, uglify reads minificationOptions property "0" before property "fromString".

@JeroenVinke , the question is why we need Object.assign({ fromString: true }, buildOptions.getValue('minify')) in the first place?

Also suggest to add checking on minificationResult (could return error) before using its code and map, to avoid unreadable error message from undefined sourcemap.

@JeroenVinke

This comment has been minimized.

Show comment
Hide comment
@JeroenVinke

JeroenVinke Jun 21, 2017

Member

@huochunpeng we added that so that you can supply the minification options through aurelia.json. It looks like you have been looking into this. Would you like to create a PR to add support for uglifyjs@3?

Member

JeroenVinke commented Jun 21, 2017

@huochunpeng we added that so that you can supply the minification options through aurelia.json. It looks like you have been looking into this. Would you like to create a PR to add support for uglifyjs@3?

@huochunpeng

This comment has been minimized.

Show comment
Hide comment
@huochunpeng

huochunpeng Jun 21, 2017

Contributor

I was trying to do a fix. So buildOptions.getValue('minify') was trying to get uglify options object from aurelia.json, but actually got the condition string "stage & prod", do you know what's the designed pattern to pass uglify options through aurelia.json?

Contributor

huochunpeng commented Jun 21, 2017

I was trying to do a fix. So buildOptions.getValue('minify') was trying to get uglify options object from aurelia.json, but actually got the condition string "stage & prod", do you know what's the designed pattern to pass uglify options through aurelia.json?

@JeroenVinke

This comment has been minimized.

Show comment
Hide comment
@JeroenVinke
Member

JeroenVinke commented Jun 21, 2017

huochunpeng added a commit to huochunpeng/cli that referenced this issue Jun 22, 2017

fix(bundle): support both Uglify v3 and v2.
Keep supporting Uglify v2.
Fix error when using Uglify v3.
Did not update `lib/dependencies.json` uglify-js version.

fixes #636

huochunpeng added a commit to huochunpeng/cli that referenced this issue Jun 23, 2017

fix(bundle): support both Uglify v3 and v2.
Keep supporting Uglify v2.
Fix error when using Uglify v3.
Update `lib/dependencies.json` uglify-js version to latest 3.0.19.

fixes #636

@JeroenVinke JeroenVinke closed this in #662 Jul 11, 2017

IvoPaunov added a commit to IvoPaunov/aero that referenced this issue Aug 4, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment