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

gulp-uglify generates source maps but mapping is incorrect #236

Open
dragonflypl opened this issue Oct 14, 2016 · 16 comments · May be fixed by #362
Open

gulp-uglify generates source maps but mapping is incorrect #236

dragonflypl opened this issue Oct 14, 2016 · 16 comments · May be fixed by #362

Comments

@dragonflypl
Copy link

I have this task to generate bundled & minified JavaScript file:

    return gulp.src(paths)
        .pipe(paths.sortByFilename ? sorter(key) : gulpUtil.noop())
        .pipe(plumber(eh('Error compiling javascript (' + key + ')')))
        .pipe(sourcemaps.init())
        .pipe(gulpBabel())
        .pipe(gulpConcat(key + '.js'))
        .pipe(gulp.dest(cp.build(gomModule) + '/js'))
        .pipe(gulpNgAnnotate({add: true, single_quotes: true}))
        .pipe(gulpUglify())
        .pipe(gulpRename({ extname: '.min.js' }))
        .pipe(sourcemaps.write("."))
        .pipe(gulp.dest(cp.build(gomModule) + '/js'));   

Looks ok: .min.js gets mangled etc and sourcemaps are generated. But unfortunatelly debugging is not possible. Mapping is incorrect - stepping through the code / locals is broken. gulp-uglify seems to be the culprit because if I remove it from the the task, mapping works (with ngAnnotations in place as well as with babel transformation). Any idea that's going on? User versions are:

"gulp-sourcemaps": "^2.1.1",
"gulp-ng-annotate": "~2.0.0",
"gulp-babel": "^6.1.2",
"gulp-concat": "^2.6.0",
"gulp-uglify": "^2.0.0",
"gulp-rename": "~1.2.0"
@g5codyswartz
Copy link

Right now my order is something like the following and I'm also experiencing this. Running w/o uglify allows for it to map properly.

.pipe(sourcemaps.init())
    .pipe(babel({presets: ['es2015']}))
    .pipe(uglify())
    .pipe(rename(function(path) { ... }))
    .pipe(sourcemaps.write('.', { ... }))

@g5codyswartz
Copy link

I just rolled back to v1.5.4 and this is now working for me.

@pinalbhatt
Copy link

I am also getting issue with sourcemap generated. with both 2.0 and 1.5.4 also.

@g5codyswartz
Copy link

@pinalbhatt Posting what your chain looks like can help diagnose the problem.

@ghost
Copy link

ghost commented Mar 27, 2017

I am also having this issue.

The following does not work with gulp-uglify 2.1.2, and problematic sourcemaps are produced:

return gulp
	.src( 'src/app.js' )
	.pipe( sourcemaps.init() )
	.pipe( babel( {
			presets: [ 'es2015' ],
			plugins: [ 'transform-class-properties' ],
		}))
	.pipe( uglify() )
	.pipe( sourcemaps.write( '.' ) )
	.pipe( gulp.dest( 'dist' ) )

However, as per @g5codyswartz's suggestion, it works fine with gulp-uglify 1.5.4
There is clearly some issue in later versions of gulp-uglify

"babel-plugin-transform-class-properties": "^6.23.0",
"babel-preset-es2015": "^6.24.0",
"gulp": "^3.9.1",
"gulp-babel": "^6.1.2",
"gulp-eslint": "^3.0.1",
"gulp-sourcemaps": "^2.4.1",
"gulp-uglify": "^2.1.2"

@Ventajou
Copy link

Ventajou commented Jan 3, 2018

Ran into this with 3.0.0, I had to set compress: false in the options for source maps to still work.

@kmmbvnr
Copy link

kmmbvnr commented Jan 8, 2018

Thanks, everyone in this thread. 1.5.4 works.

@bartlubbersen
Copy link

I think I am still having this problem, but there seem to be a lot of people complaining about sourcemaps without any solutions. The problem I am having is that the sourcemaps are generated, but if I add a console.error on a certain line the line number in the browser console is wrong. Which results in being sent to the wrong line in the file. I also checked the file itself and that actually perfectly matches the development file, so not sure why browsers are mapped to the wrong line.

I am using gulp-uglify in combination with gulp-concat and disabling gulp-uglify does solve the problem, but ofcourse that is no solution. Besides both should support sourcemaps just fine according to the documentation. I already tried all other suggested sollutions in other issues and this issue and nothing seems to help. I tested in both Firefox and Chrome and both had the same issue.

Is there any update or solution for this?

@Jimbly
Copy link

Jimbly commented Jun 13, 2020

Spent a bunch of time looking into this, and it appears the latest [email protected] + [email protected] is applying the sourceMap mapping twice - if I replace applySourceMap(file, sourceMap); with instead simply file.sourceMap = sourceMap; (in minify.js), the output sourcemap appears to be correct. Since gulp-uglify is passing in the existing mappings to uglify-js, it seems uglify-js is generating the whole composite map (not just a map from the input source).

@terinjokes
Copy link
Owner

IIRC this is supposed to be fine, but there's probably enough ambiguity in the sourcemap spec to cause problems.

I can apply that change, but need to figure out a good way to test this so there's no regression.

@Jimbly
Copy link

Jimbly commented Jun 13, 2020

It's seems like it can't possibly be fine. If map A maps lines 10:5 and 8:4 and map B does a (relative) map of 5:4 and 4:2, the output must be 10:4 and 8:2, but if that relative map is applied a second time, the output ends up being along the lines of mapping lines 10:2, 8:2 and 5:4, 2 of 3 of which are not valid. In this case, it's applying what should be an absolute map (mapping all the way back to the original source) as a relative map. Judging from another comment in that file, it makes me think that at one point uglify-js was not generating the absolute mapping, but a relative mapping (which is what vinyl-source-map-apply expects). I am quite unfamiliar with the low-level stuff and the spec, so I certainly could be confused about something in here though =).

@terinjokes
Copy link
Owner

What I meant was the sourcemap tool should have enough information to know that it has the output mapping as input, not a relative mapping, and act accordingly. I haven't thought about the broken state of sourcemaps in so long it's possible I'm wrong.

I recall needing to do this to get the sourcmaps output by UglifyJS to attach to the sourcemaps of the input, particularly in cases where the original code was written in a language compiled to JS (like coffeescript). Without applying the sourcemaps you wouldn't see the original code in the browser.

@Jimbly
Copy link

Jimbly commented Jun 13, 2020

I think applySourceMap always thinks it's applying a relative mapping, as it does see the old mappings, however since we're also passing the mappings into uglify-js it seems as though it's generating an absolute mapping back to the original (not just back to the source that was passed in), and then when that's passed into applySourceMap, it's applying it as a relative mapping (since from context, that's what it appears to be).

All that being said, the change I mentioned definitely fixed my case for piping gulp-babel into gulp-uglify (as is the case with all of the people posting earlier in this thread). In a different project I'm piping browserify (with babelify as a transform) into gulp-uglify, and that project seems to be getting perfect source mapping (but now I would expect it to have the same issue), so there's something I'm missing. I'll dig into it a bit more tomorrow or Monday and report back. That other project is on a different version of gulp, babel, uglify, and gulp-uglify at this point, so it might take some poking =).

@terinjokes
Copy link
Owner

I'm guessing UglifyJS changed their behavior at some point, you can find older issues where that definitely wasn't the case.

@robpalme
Copy link

Good find @Jimbly

applySourcemaps is not an idempotent operation. If encoding the sourcemap twice worked in the past, that was probably just a fortunate accident.

@Jimbly
Copy link

Jimbly commented Jun 13, 2020

Ah, figured out why it was mysteriously working in my other project: When doing babel -> browserify ->gulp-uglify, the browserify step is "renaming" the file, so in that case, the applySourceMaps in gulp-uglify ends up being a no-op (source-map finds no mappings with a matching file name, so discards all of the old mappings and just copies in the new one). If my browserify bundle was named exactly the same as one of my input files, I suspect it would be going wrong in the same way.

Conclusion: Since uglify-js is already creating a merged sourcemap (because we ask it to by specifying options.sourceMap.content), gulp-uglify should just set file.sourceMap = sourceMap instead of calling applySourceMap(file, sourceMap). Opened PR #362

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.

9 participants