gulp-uglify icon indicating copy to clipboard operation
gulp-uglify copied to clipboard

Source maps don't work in conjunction with concat

Open jamescrowley opened this issue 9 years ago • 31 comments

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?)

jamescrowley avatar May 18 '15 12:05 jamescrowley

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

dnbard avatar May 25 '15 13:05 dnbard

Any update on this or potential solutions?

rickihastings avatar Jun 18 '15 15:06 rickihastings

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.

michaelBenin avatar Jun 18 '15 15:06 michaelBenin

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

archasek avatar Aug 06 '15 11:08 archasek

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

jamescrowley avatar Aug 06 '15 22:08 jamescrowley

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

On Thursday, August 6, 2015, James Crowley [email protected] 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 .

michaelBenin avatar Aug 06 '15 22:08 michaelBenin

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.

wwalser avatar Aug 11 '15 08:08 wwalser

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.

Rush avatar Aug 26 '15 23:08 Rush

:+1:

chrisfls avatar Nov 18 '15 18:11 chrisfls

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 avatar Nov 27 '15 21:11 OliverJAsh

@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.

OliverJAsh avatar Nov 27 '15 21:11 OliverJAsh

@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.

Rush avatar Nov 27 '15 21:11 Rush

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.

OliverJAsh avatar Nov 27 '15 21:11 OliverJAsh

@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.

Rush avatar Nov 27 '15 22:11 Rush

@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 avatar Nov 27 '15 22:11 terinjokes

@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.

terinjokes avatar Nov 27 '15 22:11 terinjokes

@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?

OliverJAsh avatar Nov 27 '15 22:11 OliverJAsh

@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.

terinjokes avatar Nov 27 '15 22:11 terinjokes

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 avatar Nov 27 '15 23:11 OliverJAsh

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.

OliverJAsh avatar Nov 27 '15 23:11 OliverJAsh

I believe one was opened at the time. On Nov 28, 2015 12:03 PM, "Oliver Joseph Ash" [email protected] 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 .

terinjokes avatar Nov 27 '15 23:11 terinjokes

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 avatar Nov 28 '15 12:11 OliverJAsh

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: '.' })).

OliverJAsh avatar Nov 28 '15 13:11 OliverJAsh

@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.

JoshSchreuder avatar Jan 27 '16 22:01 JoshSchreuder

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...

evtk avatar Oct 10 '16 15:10 evtk

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

creage avatar Oct 26 '16 09:10 creage

@creage 1.5.4 still breaks it here :(

evtk avatar Oct 31 '16 15:10 evtk

same bug :(

AuthorProxy avatar Feb 16 '17 22:02 AuthorProxy

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 }

evtk avatar Feb 17 '17 10:02 evtk

same bug here :/ in both 1.5.4 and 2.0.1

anthonybriand avatar Mar 06 '17 15:03 anthonybriand