Skip to content
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

UglifyJsPlugin breaks source maps #2213

Closed
webpack-bot opened this issue Jul 7, 2017 · 9 comments · Fixed by #2224
Closed

UglifyJsPlugin breaks source maps #2213

webpack-bot opened this issue Jul 7, 2017 · 9 comments · Fixed by #2224

Comments

@webpack-bot
Copy link

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
Webpack generates faulty source maps when the webpack.optimize.UglifyJsPlugin is active. The generated source maps suffer from various issues, e.g.:

  • Unable to set breakpoints on some lines.
  • Some lines are skipped when stepping through the code.
  • Call stacks are incorrect.
  • Unable to step into some functions.
  • Unable to console print variables.
  • Code executed out of order, with respect to what the debugger is showing.

If the current behavior is a bug, please provide the steps to reproduce.
Below is a minimal setup for creating the various issues mentioned above.

webpack.config.js

const webpack = require('webpack');
module.exports = {
	entry: { 'a': './a', },
	output: { filename: '[name].bundle.js', },
	plugins: [ new webpack.optimize.UglifyJsPlugin({ sourceMap: true, }), ],
	devtool: 'source-map',
};

a.js

import bar from './b';

function foo(n) {
	var i = bar(); // Call stack incorrect
	console.log('FOO', i + n); // Cannot set breakpoint on this line, if there is breakpoint on line 4
}

var n = Math.abs(1); // Console print n: Object {a: Object}. 'BAR', 'FOO 3' is console logged when stepping past this line
foo(n); // Cannot step into (F11) foo here

b.js

function bar() {
	var n = Math.abs(2); // Cannot console print n: "Uncaught ReferenceError: n is not defined"
	console.log('BAR'); // This line is skipped when stepping
	return n;
}

export default bar;

What is the expected behavior?
Source maps should not cause weird browser debugger behavior when UglifyJsPlugin is active.

Please mention other relevant information such as the browser version, Node.js version, webpack version and Operating System.

  • Webpack, version 3.0.0
  • Node.js, version 8.0.0
  • Chrome, version 59.0.3071.104
  • Ubuntu, version 16.04

This issue was moved from webpack/webpack#5165 by @sokra. Orginal issue was by @oskargustafsson.

This seem to be a uglifyjs bug.

UglifyJS generates this code for your example:

var r=n(1),o=Math.abs(1);!function(t){var e=r.a();console.log("FOO",e+t)}(o)}

Not how the foo function is inlined into the call.

But the SourceMappings seem to be wrong in this case. There is no reference to the call in the SourceMap anymore.

SourceMap

Chrome seem to use the SourceMap reference at the ( of the call.

...;console.log("FOO",e+t)}(o)}
                           ^

This maps to the incorrect line:

image

I tried changing the SourceMap to this:

image

That seem to work in this case.

@kzc
Copy link
Contributor

kzc commented Jul 7, 2017

@sokra @oskargustafsson

As part of the normal minification process with function and variable inlining these observations are expected since uglify causes side-effect-free code to be rearranged and variables/parameters disappear as they are folded into other values:

  • Unable to set breakpoints on some lines.
  • Some lines are skipped when stepping through the code.
  • Call stacks are incorrect.
  • Unable to step into some functions.
  • Unable to console print variables.
  • Code executed out of order, with respect to what the debugger is showing.

If better debuggability is desired, the compress option inline can be disabled - although this would make the uglified output slightly larger. If you want to see the value of every single variable, you could disable compress altogether and only enable mangle - however this will on average cause the minified output to be 5% larger.

It might be possible for uglify to correct one aspect of function inlining - pointing to the opening paren instead of the first argument as per @sokra's final example above.

@kzc
Copy link
Contributor

kzc commented Jul 7, 2017

To further illustrate why debugging does not work with inlining:

$ bin/uglifyjs -V
uglify-js 3.0.23
$ cat inline.js 
!function(){
    function foo(bar, baz) {
        var quux = bar * bar - baz;
        var value = quux * 2;
        console.log(value);
    }
    foo(10, 20);
}();
$ bin/uglifyjs inline.js -c passes=2
console.log(160);

@alexlamsl
Copy link
Collaborator

It might be possible for uglify to correct one aspect of function inlining - pointing to the opening paren instead of the first argument as per @sokra's final example above.

AFAICT that there is no special souce map positioning for AST_Call, and the reason why it is pointed at right before o is due to DEFMAP(AST_Symbol):
https://github.com/mishoo/UglifyJS2/blob/af0262b7e5fd3dbf83619cdb375ab18c41def3e7/lib/output.js#L1402

In fact if you look at console.log("F... it's also pointed to right after the opening parenthesis.

@kzc
Copy link
Contributor

kzc commented Jul 10, 2017

The following patch creates a new source mapping at the opening paren of an IIFE. Otherwise the source map is the same.

--- a/lib/output.js
+++ b/lib/output.js
@@ -1104,6 +1104,9 @@ function OutputStream(options) {
         self.expression.print(output);
         if (self instanceof AST_New && !need_constructor_parens(self, output))
             return;
+        if (self.expression instanceof AST_Function) {
+            output.add_mapping(self.start);
+        }
         output.with_parens(function(){
             self.args.forEach(function(expr, i){
                 if (i) output.comma();

Compare source mappings:

Hover mouse over foo(value);.

Commands used:

$ uglifyjs -V
uglify-js 3.0.24
$ cat test.js
!function(){
  function foo(size) {
    var random = size * Math.random();
    console.log("FOO", size * random);
  }
  console.log("start");
  var value = 123;
  foo(value);
  console.log("end");
}();
$ uglifyjs test.js -m -c --source-map includeSources,filename=test.min.js.map -o test.min.js

Nothing else in this ticket is actionable. You can't debug variables that were folded into other values, nor can you get accurate stack traces for functions that have been inlined and no longer exist.

@alexlamsl
Copy link
Collaborator

@kzc interesting tool and thanks for the patch. I'll create that PR to address this issue now 👍

alexlamsl added a commit to alexlamsl/UglifyJS that referenced this issue Jul 10, 2017
@kzc
Copy link
Contributor

kzc commented Jul 10, 2017

It doesn't really improve the Chrome debugging experience. When you try to set a breakpoint on foo(value); it just bounces you over to function foo. Nothing we can do about that. It stands to reason that the better uglify does eliminating and folding code, the worse the debugging experience will be.

@alexlamsl
Copy link
Collaborator

@kzc how about mapping ) to self.end instead? Would that make UX better?

@kzc
Copy link
Contributor

kzc commented Jul 10, 2017

how about mapping ) to self.end instead?

I don't think Chrome places any significance on the closing paren. I've had a trial and error crash course on source maps in the last day - the technology is really poorly documented!

self.end is not terribly useful. Better to change source map handling as little as possible.

@alexlamsl
Copy link
Collaborator

I don't think Chrome places any significance on the closing paren. I've had a trial and error crash course on source maps in the last day - it's really poorly documented!

You have my respect and sympathy on that one 😅

self.end is not terribly useful. Better to change source map handling as little as possible.

👍

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

Successfully merging a pull request may close this issue.

3 participants