terinjokes / gulp-uglify

Minify files with UglifyJS
MIT License
1.23k stars 155 forks source link

Source maps don't work in conjunction with concat #105

Open jamescrowley opened 9 years ago

jamescrowley commented 9 years ago

Having read around I can see there are some ongoing issues with UglifyJS and Source maps, but folks seem to indicate this should work. However, I cannot generate a valid source map for which symbols are mapped correctly and you're able to set breakpoints where you expect.

I've pushed up a simple repo that reproduces the issue for me:

https://github.com/jamescrowley/gulpIssues

I would really welcome any suggestions as to how to further narrow down this issue to either gulp-uglify or UglifyJS (and any workarounds?)

dnbard commented 9 years ago

Same issue here with browserify+react. Gulpfile:

gulp.task('javascript', function () {
    var bundleStream = browserify({
        entries: './src/app.js',
        debug: true,
        transform: [reactify]
    }).bundle()
      .pipe(source('app.js'))
      .pipe(buffer())
      .pipe(sourcemaps.init({loadMaps: true}))
      .pipe(uglify())
      .pipe(sourcemaps.write('./'))
      .pipe(gulp.dest('./dist'));
});

Sourcemap file is created but it isn't usable in latest Chrome(but still usable in FF). If I comment .pipe(uglify()) everything is working

rickihastings commented 9 years ago

Any update on this or potential solutions?

michaelBenin commented 9 years ago

Here is a solution that doesn't use gulp.

I have this working here:

https://github.com/michaelBenin/node-startup/blob/develop/Gruntfile.js#L29

Build browserify with debug enabled. Extract sourcemap using: https://github.com/duereg/grunt-extract-sourcemap/blob/master/lib/extractor.coffee#L1 Input source into uglify: https://github.com/michaelBenin/node-startup/blob/develop/automation/uglify.js

Now in my production environment in combination with rollbar, I have stack traces to the original source.

This is the only thing holding me back from going full gulp.

archasek commented 9 years ago

@jamescrowley did you find a solution? Same issue here.

jamescrowley commented 9 years ago

@archasek nope, just ended up disabling uglify during our dev build, which is when we mostly wanted the source maps anyway...

michaelBenin commented 9 years ago

https://github.com/gulpjs/gulp/blob/master/docs/recipes/browserify-uglify-sourcemap.md

On Thursday, August 6, 2015, James Crowley notifications@github.com wrote:

@archasek https://github.com/archasek nope, just ended up disabling uglify during our dev build, which is when we mostly wanted the source maps anyway...

— Reply to this email directly or view it on GitHub https://github.com/terinjokes/gulp-uglify/issues/105#issuecomment-128532763 .

wwalser commented 9 years ago

I thought my issues had to do with the fact that I was using Babel followed by Uglify but after a bit more experimentation it appears to be using gulp-uglify in combination with anything else. The fact that it's not reading input source maps seems to break any mapping that exists prior to it's invocation.

Rush commented 9 years ago

The fact that it's not reading input source maps seems to break any mapping that exists prior to it's invocation.

then it appears I have stumbled upon the same problem.

chrisfls commented 8 years ago

:+1:

OliverJAsh commented 8 years ago

I can't believe a project as mature as Gulp doesn't have an uglify task that supports input source maps :astonished:

I hoped this would be as simple as adding inSourceMap as an option. That does seem to alter something, but the mappings are still coming out incorrect. Not sure what it might be.

OliverJAsh commented 8 years ago

@Rush Does that work for you? I don't see how it would work. This task doesn't specify an input source map to uglify.

Rush commented 8 years ago

@Rush Does that work for you? I don't see how it would work. This task doesn't specify an input source map to uglify.

I don't know what you mean. Source maps should be automatically read from the location specified by source map annotations. And I haven't checked since the time of my first comment. I am busy with other stuff right now.

OliverJAsh commented 8 years ago

Uglify will output a source map that maps code from the input file to the output file. However, what if another transformation came before uglify? You would want uglify to receive the existing source map as input, and output a source map that maps back to the original source code.

We need to pass the current source map file into uglify so it outputs the correct thing. It looks like we used to do the correct thing, but that was removed in https://github.com/terinjokes/gulp-uglify/commit/8b9fc8c124fb8fc691cb3fdfc2bff26f9ea32129 to fix a bug in #56.

Rush commented 8 years ago

@OliverJAsh another transformation that comes before uglify has to generate its own source map. Uglify knows the previous source map from the annotation at the end of the file:

//# sourceMappingURL=application-a107631053.js.map

Hence nothing more needs to be passed for uglify to do the right thing.

terinjokes commented 8 years ago

@OliverJAsh I rely on @floridoo for source maps related things. I apologize that you're currently not happy. I'm hoping to avoid needing to cleanup the output from UglifyJS, especially when UglifyJS tends to replace the filenames with "?".

I'm on vacation this week (shh, I'm cheating by being on here), but I'll take another look at this the beginning of next week.

terinjokes commented 8 years ago

@Rush In a pipeline the "application-a107631053.js.map" wouldn't be written out to disk, so UglifyJS wouldn't be able to use that.

OliverJAsh commented 8 years ago

@terinjokes Hey, thanks for the quick reply!

Can you explain why you did https://github.com/terinjokes/gulp-uglify/commit/8b9fc8c124fb8fc691cb3fdfc2bff26f9ea32129? If I revert this, source maps behave how I expect. If this is fixing something else, can you think of a way we can fix this bug as well as whatever that bug was?

terinjokes commented 8 years ago

@OliverJAsh UglifyJS breaks the "sources" array when there's input source maps (it replaces them with a single "?" file). I've attempted to resolve this by copying the array from the input map to the output map, which resolves it in simple cases, but in more complex cases, UglifyJS seems to reorder the maps, resulting in the files no longer being accurate.

I don't have a clear understanding of when this is happening as my use of gulp-uglify doesn't seem to trigger it. This leads me to be hesitant to re-introduce inputSourceMaps, despite how much I would like them.

OliverJAsh commented 8 years ago

Do you know if there's an issue on Uglify JS for that? If we could get that fixed upstream, we could do the right thing in this gulp task.

OliverJAsh commented 8 years ago

Even on simple setups it is problematic. This is with https://github.com/terinjokes/gulp-uglify/commit/8b9fc8c124fb8fc691cb3fdfc2bff26f9ea32129 reverted (locally).

Given service-worker.js:

var foo = 1;
console.log(foo);
var baz = function () {};
console.log(baz);

My gulp pipeline:

gulp.task('build-service-worker', () => (
    gulp.src('./public-src/service-worker.js')
        .pipe(sourcemaps.init())
        .pipe(babel())
        .pipe(uglify())
        .pipe(sourcemaps.write('.'))
        .pipe(gulp.dest('./public'))
));

Output file:

"use strict";var foo=1;console.log(foo);var baz=function(){};console.log(baz);
//# sourceMappingURL=service-worker.js.map

Output source map:

{"version":3,"sources":["service-worker.js"],"names":["foo","console","log","baz"],"mappings":"YAAA,IAAIA,KAAM,CACVC,SAAQC,IAAIF,IADZ,IAAIG,KAAM,YACVF,SAAQC,IAAIC;;AADZ,IAAI,GAAG,GAAG,CAAC,CAAC;AACZ,OAAO,CAAC,GAAG,CAAC,GAAG,CAAC,CAAC;AADjB,IAAI,GAAG,GAAG,SAAN,GAAG,GAAK,EAAA,CAAA;AACZ,OAAO,CAAC,GAAG,CAAC,GAAG,CAAC,CAAC","file":"service-worker.js","sourcesContent":["var foo = 1;\nconsole.log(foo);\nvar baz = function () {};\nconsole.log(baz);\n"],"sourceRoot":"/source/"}

The mappings are incorrect when tested in dev tools.

terinjokes commented 8 years ago

I believe one was opened at the time. On Nov 28, 2015 12:03 PM, "Oliver Joseph Ash" notifications@github.com wrote:

Do you know if there's an issue on Uglify JS for that? If we could get that fixed upstream, we could do the right thing in this gulp task.

— Reply to this email directly or view it on GitHub https://github.com/terinjokes/gulp-uglify/issues/105#issuecomment-160224595 .

OliverJAsh commented 8 years ago

I've just spent some time looking into this.

I now understand why we don't need to provide an input source map to Uglify. It's because vinyl-sourcemaps-apply will rebase the source map that comes out of Uglify on top of the existing source map, i.e. one provided by a previous task: https://github.com/floridoo/vinyl-sourcemaps-apply/blob/master/index.js#L22-L23. So, there's no need to provide an input source map to Uglify. The work is done for us.

Given foo.js:

var foo = 1;
console.log(foo);
var baz = function () {};
console.log(baz);

Here's my gulpfile:

gulp.task('build-foo', () => (
    gulp.src('./public-src/foo.js')
        .pipe(sourcemaps.init())
        .pipe(babel())
        .pipe(uglify())
        .pipe(sourcemaps.write('.', { sourceRoot: '.' }))
        .pipe(gulp.dest('./public'))
));

The output source map. foo.js.map:

{
  "version": 3,
  "sources": [
    "foo.js"
  ],
  "names": [
    "foo",
    "console",
    "log",
    "baz"
  ],
  "mappings": "AAAA,YAAA,IAAIA,KAAM,CACVC,SAAQC,IAAIF,IACZ,IAAIG,KAAM,YACVF,SAAQC,IAAIC",
  "file": "foo.js",
  "sourcesContent": [
    "var foo = 1;\nconsole.log(foo);\nvar baz = function () {};\nconsole.log(baz);\n"
  ],
  "sourceRoot": "."
}

All the mappings here are correct, but there is one issue, which prevents Chrome DevTools from using the source map: sources cannot be the same as file. If I change this from foo.js to anything else, e.g. bar.js, it works!

Maybe this is something that needs to be fixed in Chrome DevTools (https://code.google.com/p/chromium/issues/detail?id=562870), but for the time being I am working around it by adding this line below https://github.com/terinjokes/gulp-uglify/blob/daa6a28719fcb3f35e89de7b2044b3b9788573b5/minifier.js#L80.

file.sourceMap.sources = ['source.js']

I also don't think this problem is specific to this gulp task—it will occur whenever you use gulp-sourcemaps AFAIK.

OliverJAsh commented 8 years ago

I think there is still a problem with the source map mappings. When I console log, I see the line number is correctly resolved:

image

… but I can't add breakpoints in some places, and in the places where I can, the scope mappings don't work:

image

If I remove uglify and just use babel, everything works perfectly. So there's something else wrong here :unamused:

From reading around, it seems that the merge done by vinyl-sourcemaps-apply cannot be trusted: https://github.com/terinjokes/gulp-uglify/issues/56#issuecomment-101950321, https://github.com/floridoo/vinyl-sourcemaps-apply/issues/10, and https://github.com/mozilla/source-map/issues/216#issuecomment-150897798. So I had a go at using the inSourceMap option of Uglify, like this task used to, but that seems to have its own problems merging the two source maps: https://github.com/mishoo/UglifyJS2/issues/882. This could be just because Uglify uses the same library under the hood as vinyl-sourcemaps-apply, which is source-map.

:unamused:

Conclusion

There are two issues:

  1. gulp-sourcemaps outputs source map with a "sources" attribute that is the same as "file", which Chrome DevTools doesn't like https://github.com/terinjokes/gulp-uglify/issues/105#issuecomment-160292080 https://code.google.com/p/chromium/issues/detail?id=562870
  2. source map merging will use the lowest resolution of the two inputs, i.e. the uglify source map, which makes it almost useless https://github.com/mozilla/source-map/issues/216

These are both issues not specific to gulp-uglify, but the latter is probably more evident as uglify will output a very low resolution source map, which is the nature of uglification of course.

Update

You can workaround issue 1 (above) by removing sourceRoot: '.' from .pipe(sourcemaps.write('.', { sourceRoot: '.' })).

JoshSchreuder commented 8 years ago

@OliverJAsh I'm having a similar problem. console.log works fine, but any kind of syntactic error such as throw new Error() won't have line numbers resolved correctly from the sourcemap:

image

My task looks like:

gulp.src(bundle.input)
    .pipe($.sourcemaps.init())
      .pipe($.concat(bundle.output))
    .pipe($.sourcemaps.write())
    .pipe(gulp.dest('./Scripts/min'))

Where bundle is a standard JS object:

{
    input: [
        'Areas/Administration/Scripts/admin-app.js',
        'Areas/Administration/Scripts/controllers/admin-controller.js',
        'Areas/Administration/Scripts/services/admin-landing-service.js',
        'Areas/Administration/Scripts/models/admin-config.js'
    ],
    output: 'adv-administration.min.js'
}

This is without uglify even getting involved (which I would like to get working too :smile:). All I'm doing is combining a few files and outputting them with a sourcemap, but I can't seem to get it working.

evtk commented 8 years ago

Is there any update on this? I have a simple build task involving concat, angularFilesort, ngAnnotate and uglify. As soon as I enable the uglify, the sourcemap will not be used/triggered by chrome...

creage commented 8 years ago

Running version 1.5.4 doesn't break sourcemaps, while 2.x branch does.

evtk commented 7 years ago

@creage 1.5.4 still breaks it here :(

AuthorProxy commented 7 years ago

same bug :(

evtk commented 7 years ago

A colleague of mine has done some trial and error on this issue. He has concluded that with the following options, the sourcemaps are in fact working:

compress: { sequences: false }

anthonybriand commented 7 years ago

same bug here :/ in both 1.5.4 and 2.0.1

MrHektor commented 7 years ago

@EvtK that does not solve the issue for us. Problem still occurs with this option using uglify and concat.