hughsk / uglifyify

A browserify transform which minifies your code using UglifyJS2
Other
375 stars 61 forks source link

Switching from 3.0.4 to 4.0.0 results in TypeError: Cannot read property 'replace' of undefined while parsing file #66

Open tristanls opened 7 years ago

tristanls commented 7 years ago

Actual command I use:

browserify -g [ babelify --presets [ react es2015 ] ] -g [ envify --NODE_ENV production ] -g [ uglifyify -c ] ./src/gui.jsx -o ./build/js/gui.js --extension=.jsx

It succeeds with uglifyify version 3.0.4 and fails with version 4.0.0 as follows:

TypeError: Cannot read property 'replace' of undefined while parsing file: src/gui.jsx
    at Stream.ready (node_modules/uglifyify/index.js:78:24)
    at Stream.<anonymous> (node_modules/uglifyify/index.js:100:12)
    at _end (node_modules/through/index.js:65:9)
    at Stream.stream.end (node_modules/through/index.js:74:5)
    at DestroyableTransform.onend (node_modules/through2/node_modules/readable-stream/lib/_stream_readable.js:499:10)
    at Object.onceWrapper (events.js:314:30)
    at emitNone (events.js:110:20)
    at DestroyableTransform.emit (events.js:207:7)
    at endReadableNT (node_modules/readable-stream/lib/_stream_readable.js:920:12)
    at _combinedTickCallback (internal/process/next_tick.js:102:11)
"devDependencies": {
    "babel-cli": "6.24.1",
    "babel-preset-es2015": "6.24.1",
    "babel-preset-react": "6.24.1",
    "babelify": "7.3.0",
    "browserify": "14.4.0",
    "envify": "4.0.0",
    "uglifyify": "4.0.0",
    "watchify": "3.9.0"
  }

Furthermore, removing uglifyify from the toolchain also succeeds, so that the dependencies listed above succeed using this command:

browserify -g [ babelify --presets [ react es2015 ] ] -g [ envify --NODE_ENV production ] ./src/gui.jsx -o ./build/js/gui.js --extension=.jsx
lrlna commented 7 years ago

Hey! I am looking into it for you.

Out of curiosity does something like this uglifyify -c dead_code still return an error for you?

tristanls commented 7 years ago

Is this what you mean?

$ cat pr.js 
"use strict";

module.exports = "nope";
$ ./node_modules/.bin/browserify -g [ uglifyify -c ] pr.js 
TypeError: Cannot read property 'replace' of undefined while parsing file: pr.js
    at Stream.ready (node_modules/uglifyify/index.js:78:24)
    at Stream.<anonymous> (node_modules/uglifyify/index.js:100:12)
    at _end (node_modules/through/index.js:65:9)
    at Stream.stream.end (node_modules/through/index.js:74:5)
    at DestroyableTransform.onend (node_modules/through2/node_modules/readable-stream/lib/_stream_readable.js:499:10)
    at Object.onceWrapper (events.js:314:30)
    at emitNone (events.js:110:20)
    at DestroyableTransform.emit (events.js:207:7)
    at endReadableNT (node_modules/through2/node_modules/readable-stream/lib/_stream_readable.js:920:12)
    at _combinedTickCallback (internal/process/next_tick.js:102:11)
tristanls commented 7 years ago

For completeness, it succeeds on 3.0.4:

$ npm install uglifyify --no-save
+ uglifyify@3.0.4
removed 1 package and updated 1 package in 3.265s
$ ./node_modules/.bin/browserify -g [ uglifyify -c ] pr.js 
(function e(t,n,r){function s(o,u){if(!n[o]){if(!t[o]){var a=typeof require=="function"&&require;if(!u&&a)return a(o,!0);if(i)return i(o,!0);var f=new Error("Cannot find module '"+o+"'");throw f.code="MODULE_NOT_FOUND",f}var l=n[o]={exports:{}};t[o][0].call(l.exports,function(e){var n=t[o][1][e];return s(n?n:e)},l,l.exports,e,t,n,r)}return n[o].exports}var i=typeof require=="function"&&require;for(var o=0;o<r.length;o++)s(r[o]);return s})({1:[function(require,module,exports){
"use strict";module.exports="nope";
},{}]},{},[1]);
lrlna commented 7 years ago

No, not quite. I wanted to see if passing an option to--compress might help, since the new uglify comes with a bunch of them you could work with.

But let me try to just replicate it just as you have it in the latter two examples and see where I get on.

lrlna commented 7 years ago

Okay, cool, I was able to replicate and it's having issues on extend; interestingly only with command-line options. I use it outside of the command-line context, so never noticed this issue with the original pr. Give me a bit, and I should have a fix for you.

Alxmerino commented 7 years ago

I'm seeing the same issue but with 3.0.4 I am running it programmatically as opposed to the CLI

iamjochem commented 7 years ago

the TypeError is masking the real error the return value from ujs.minify() (see here), is always an object BUT it may contain an error property, in which case it is likely that it does not have a code property (which the code is blindly assuming it it).

The underlying error (at least those I have had) alwasy relate to invalid options (for uglify) - this is because the uglify-es modules is being used (which breaks API compatibility with uglify@v2).

I think this modules should do suitable checking of the return value of ujs.minify() (does min.code exist, does min.error exist, etc ... and then display, throw the returned error data), also alway pass a copy/clone of the opts object to ujs.minify() (really the given opts should be cloned and then the clone manipulated)

iamjochem commented 7 years ago

I hacked the following PR, which tackles the swallowed minify() errors & problems related to not-cloning options before manipulating and/or passing them on : PR-69

lrlna commented 7 years ago

Ok, so uglify no longer supports shortened CLI in there JS api, but hopefully that is fixed for you in 📦 4.0.2.

I still want to address creating a new opts object, and that should be fixed by @iamjochem #69.

tristanls commented 7 years ago

I don't have a minimal reproduction yet, but using uglifyify@4.0.2 my production use case results in:

`_flags` is not a supported option
yoshuawuyts commented 7 years ago

Sounds like the browserify option ._flags isn't being deleted when it should be

On Wed, Jun 28, 2017 at 9:28 PM Tristan Slominski notifications@github.com wrote:

I don't have a minimal reproduction yet, but my production use case results in:

_flags is not a supported option

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/hughsk/uglifyify/issues/66#issuecomment-311762950, or mute the thread https://github.com/notifications/unsubscribe-auth/ACWleqg3ZwkjU7cpAiZYon1xkOQrQZ2Pks5sIqlvgaJpZM4N6JfK .

sassanh commented 7 years ago

I'm also affected by `_flags` is not a supported option