Different sized bundles on repeated build about uglifyify

[yoshuawuyts]

Lol, feel like this would be an uglifyjs thing - this is just a thin wrapper haha

…

On Fri, Mar 31, 2017 at 3:11 PM Chris Rawnsley ***@***.***> wrote: I have noticed that this module I am not getting deterministic builds. Minimal test case below. When you run npm test it will just repeat the exact same Browserify bundle step but it will give you different size bundles. package.json { "private": true, "dependencies": { "browserify": "^14.1.0", "react": "^15.4.2", "react-dom": "^15.4.2", "uglifyify": "^3.0.4" }, "scripts": { "test": "for i in 0 1 2 3 4 5 6 7 8 9; do echo \"require('react-dom')\" | browserify - -g uglifyify | wc -c; done" } } Shell: npm install && npm test My output: > @ test /Users/casr/Source/scratch/uglifyify-test> for i in 0 1 2 3 4 5 6 7 8 9; do echo "require('react-dom')" | browserify - -g uglifyify | wc -c; done 319901 319901 320050 319926 319366 319777 319777 319777 319777 319926 I have also uploaded this as a gist at https://gist.github.com/casr/664cf9a2011b940436e1c59c2716f6d5 if you want want to clone it from there. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#61>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACWlenNIqSEkItPjtv9krGalCk0JufIYks5rrPt5gaJpZM4Mvphb> .

[casr]

¯\(°_o)/¯

also this:

❯ for i in 0 1 2 3 4 5 6 7 8 9; do echo "require('react-dom')" | ./node_modules/.bin/browserify - | ./node_modules/.bin/uglifyjs -cm 2>/dev/null| wc -c; done
  242537
  242537
  242537
  242537
  242537
  242537
  242537
  242537
  242537
  242537

[casr]

Also, "Minimal test case below." was a bit cheeky to say. react is a beast.

[casr]

Okay, I have a much smaller test case without React involved. Repository here https://github.com/casr/uglifyify-bug

It seems to have something to do with including files that are similar to one and other. I'm guessing the reason it's unstable has something to do with the order in which files are pushed through the uglifyify transform.

--- output-1.js	2017-03-31 22:26:02.000000000 +0100
+++ output-35.js	2017-03-31 22:26:12.000000000 +0100
@@ -4,14 +4,13 @@
 module.exports="a string";
 },{}],3:[function(require,module,exports){
 var c=require("./c");module.exports=[c];
-
 },{"./c":2}],4:[function(require,module,exports){
 require("./d");
 
 },{"./d":3}],5:[function(require,module,exports){
 arguments[4][2][0].apply(exports,arguments)
 },{"dup":2}],6:[function(require,module,exports){
-var c=require("./c");module.exports=[c];
-},{"./c":5}],7:[function(require,module,exports){
+arguments[4][3][0].apply(exports,arguments)
+},{"./c":5,"dup":3}],7:[function(require,module,exports){
 require("../a"),require("./d");
 },{"../a":4,"./d":6}]},{},[1]);

Note: Because this test case is so small, it doesn't trigger quite as often as the React one. But at least the builds are quicker...

[hughsk]

Theory: the variations are coming from browserify, and it's only showing up with uglifyify because it's a long, blocking task.

[casr]

I reran the same test with Browserify's --no-dedupe switch and still got differences. This time it was just new lines. Adding a new line gives a new hash for its contents which then leads to much more dramatic differences when it's run through the dedupe part of Browserify's pipeline.

Probably related to #57 although the suggested fix didn't work for me.

[casr]

I was playing around and I think I've managed to fix it, however, my solution hooks more directly into the UglifyJS API. In doing so I've rewritten the source map code to just use UglifyJS's built in one. It all seems to be working fine but I would like to write some tests to confirm before submitting a PR.

The approach I have taken so far is to try and do the bare minimum to just get rid dead code and extraneous require calls. I think that was in keeping with the spirit of the readme. Why? The thinking is that you will be piping it through uglifyjs -cm anyway so there's not point doing the work twice.

On a large codebase like React's it's resulting in quicker build times and ever so slightly smaller bundles (probably related to the dedupe problem). If you're happy to accept a PR along those lines then I'll try and get something done by next weekend.

In other news, UglifyJS's API has made me sad.

[casr]

More research: The real problem is in fact that opts._flags doesn't always get sent through. This causes the debug flag to occasionally flip to it's default value of true.

  var debug = '_flags' in opts
    ? opts._flags.debug
    : true

This then causes UglifyJS to produce a source map:

    if (min.map && min.map !== 'null') {
      var map = convert.fromJSON(min.map)
      // ... snip ...
      this.queue('\n')
      this.queue(map.toComment())
    }

which then causes Browserify to remove the sourcemap (as it thinks it is out of debug mode) but leave the new line which is why we get strange results with the dedupe operation later.

So it would seem to be a Browserify issue at this stage. I need to check their issue tracker and see if the occasional passing through of opts._flags is intended behaviour or not.

[casr]

So I followed the transform logic through and there didn't seem to be a problem on Browserify's side but I noticed the opts object persisted through runs and I remembered seeing this: https://github.com/hughsk/uglifyify/blob/621cfede/index.js#L17

Deleting the _flags key breaks the debug logic in future file transforms. @hughsk do you know why you added the delete or is it safe to remove that line? I couldn't find anything in the commit logs for it.