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

NPM updating to 1.10 which breaks sourcemap sources files at least with concat #270

Closed
twiggy opened this issue Jan 10, 2017 · 20 comments
Closed

Comments

@twiggy
Copy link

twiggy commented Jan 10, 2017

We had our package.json set to ^1.6.0 which then updated to 1.10

If say you have pipe.concat('bundle.js') the sourcemap file will have an array like [ 'bundle.js', 'bundle.js', ... ] for each file that should have been there.

Thanks for contributing to open source! We do appreciate it.

@nmccready
Copy link
Collaborator

I need more details with a solid example. Please code up a spec into integration.js in this repo.

@brphelps
Copy link

Same issue reporting in, downgrading to 1.9.3 fixed the issue. I'll see if I can help with a repro as time allows today :).

@brphelps
Copy link

At the very least, I have a full stack here:
events.js:160
throw er; // Unhandled 'error' event
^

Error: "../../../../../wwwroot/DevTools/DevTools/dev-tools.ts" is not in the SourceMap.
at SourceMapConsumer_sourceContentFor [as sourceContentFor] (\node_modules\source-map\lib\source-map-consumer.js:704:13)
at SourceMapGenerator. (\node_modules\source-map\lib\source-map-generator.js:235:40)
at Array.forEach (native)
at SourceMapGenerator_applySourceMap [as applySourceMap] (\node_modules\source-map\lib\source-map-generator.js:234:32)
at applySourceMap (\node_modules\vinyl-sourcemaps-apply\index.js:27:15)
at transform (\node_modules\gulp-ng-annotate\index.js:26:5)
at DestroyableTransform._transform (\node_modules\gulp-ng-annotate\index.js:58:25)
at DestroyableTransform.Transform._read (\node_modules\through2\node_modules\readable-stream\lib_stream_transform.js:159:10)
at DestroyableTransform.Transform._write (\node_modules\through2\node_modules\readable-stream\lib_stream_transform.js:147:83)
at doWrite (\node_modules\through2\node_modules\readable-stream\lib_stream_writable.js:347:64)
at writeOrBuffer (\node_modules\through2\node_modules\readable-stream\lib_stream_writable.js:336:5)
at DestroyableTransform.Writable.write (\node_modules\through2\node_modules\readable-stream\lib_stream_writable.js:274:11)
at DestroyableTransform.ondata (\node_modules\through2\node_modules\readable-stream\lib_stream_readable.js:546:20)
at emitOne (events.js:96:13)
at DestroyableTransform.emit (events.js:188:7)
at readableAddChunk (\node_modules\through2\node_modules\readable-stream\lib_stream_readable.js:217:18)

@brphelps
Copy link

brphelps commented Jan 10, 2017

And the pertinent part of our gulpfile :) :


var bundle = gulp.src(srcPaths)
                .pipe(pipeif(mapCondition, sourcemaps.init({ loadMaps: true })))
                .pipe(concat(bundleFileName))
                .pipe(pipeif(mapCondition, sourcemaps.write('./')))
                .pipe(gulp.dest(bundleOutputPath))
            var bundleMin = gulp.src(srcPaths)
                .pipe(pipeif(mapCondition, sourcemaps.init({ loadMaps: true })))
                .pipe(plugins.ngAnnotate({ add: true }))
                .pipe(concat(bundlePrefix + bundleItem.name + '-min.js'))
                .pipe(pipeif(minCondtion, uglify()))
                .pipe(pipeif(mapCondition, sourcemaps.write('./')))
                .pipe(gulp.dest(bundleOutputPath));

@brphelps
Copy link

It looks like this is specific to ngAnnotate's usage of sourcemaps, so it's a little difficult for me to repro as a direct consumer of sourcemaps. Is there any additional tracing I should turn on? I was looking in the change history and the large refactoring that happened in 1.10 means I can't easily experiment line by line to see what is causing the problem.

@nmccready
Copy link
Collaborator

I am first looking into node_modules\source-map as this could be a new version problem. What version do you have of that lib?

Lastly turn on some logging on gulp-sourcemaps.

DEBUG=gulp-sourcemaps:*

Then we can possibly see where it gets to in gulp-sourcemaps.

Your stack is puzzling as I dont see gulp-sourcemaps in it at all.

@brphelps
Copy link

I don't suspect it's a related package version, from what I can tell NPM is only swapping out the source for gulp-sourcemaps.

npm install gulp-sourcemaps@1.10.0

@ms/mseg-eae-pylon-gulp@0.0.0-alpha
`-- gulp-sourcemaps@1.10.0

---At this point it's broken---

npm install gulp-sourcemaps@1.9.3
@ms/mseg-eae-pylon-gulp@0.0.0-alpha
`-- gulp-sourcemaps@1.9.3

---At this point it's working---

@brphelps
Copy link

brphelps commented Jan 10, 2017

Ran with the debug flags -- basically, the "not working" one looks like this:

gulp-sourcemaps:write { includeContent: true, addComment: true, charset: 'utf8' } +411ms gulp-sourcemaps:write { includeContent: true, addComment: true, charset: 'utf8' } +8ms gulp-sourcemaps:write { includeContent: true, addComment: true, charset: 'utf8' } +4ms gulp-sourcemaps:write { includeContent: true, addComment: true, charset: 'utf8' } +12ms gulp-sourcemaps:write { includeContent: true, addComment: true, charset: 'utf8' } +2ms gulp-sourcemaps:write { includeContent: true, addComment: true, charset: 'utf8' } +13ms gulp-sourcemaps:init { loadMaps: true } +222ms gulp-sourcemaps:utils has preExisting +1ms gulp-sourcemaps:init:internals:loadMaps:_getInlineSources comment REMOVED +1ms gulp-sourcemaps:init { loadMaps: true } +17ms gulp-sourcemaps:utils has preExisting +2ms gulp-sourcemaps:init:internals:loadMaps:_getInlineSources comment REMOVED +2ms

At this point in the not working flow, I see the error stack I posted above.

And the working one looks like this (and has more of the same, basically it looks like it's doing it's job).

gulp-sourcemaps:write { includeContent: true, addComment: true, charset: 'utf8' } +434ms
  gulp-sourcemaps:write { includeContent: true, addComment: true, charset: 'utf8' } +6ms
  gulp-sourcemaps:write { includeContent: true, addComment: true, charset: 'utf8' } +5ms
  gulp-sourcemaps:write { includeContent: true, addComment: true, charset: 'utf8' } +9ms
  gulp-sourcemaps:write { includeContent: true, addComment: true, charset: 'utf8' } +2ms
  gulp-sourcemaps:write { includeContent: true, addComment: true, charset: 'utf8' } +2ms
  gulp-sourcemaps:init { loadMaps: true } +241ms
  gulp-sourcemaps:init:loadMaps loadMaps +1ms
  gulp-sourcemaps:utils has preExisting +1ms
  gulp-sourcemaps:init:loadMaps comment REMOVED +2ms
  gulp-sourcemaps:init { loadMaps: true } +15ms
  gulp-sourcemaps:init:loadMaps loadMaps +1ms
  gulp-sourcemaps:utils has preExisting +0ms
  gulp-sourcemaps:init:loadMaps comment REMOVED +5ms
  gulp-sourcemaps:init { loadMaps: true } +16ms
  gulp-sourcemaps:init:loadMaps loadMaps +0ms
  gulp-sourcemaps:init { loadMaps: true } +19ms
  gulp-sourcemaps:init:loadMaps loadMaps +1ms

@nmccready
Copy link
Collaborator

Ok I think I know the issue. I changed the sources to be flattened within the sourceRoot which is one of the main reasons I bumped to 1.10 and 2.3 .

See lines:

https://github.com/floridoo/gulp-sourcemaps/blob/master/src/write/index.internals.js#L42-L46

The old functionality just passed filePath on. So in your write add:

var bundle = gulp.src(srcPaths)
                .pipe(pipeif(mapCondition, sourcemaps.init({ loadMaps: true })))
                .pipe(concat(bundleFileName))
                .pipe(pipeif(mapCondition, sourcemaps.write('./', (filePath) => return filePath;)))
                .pipe(gulp.dest(bundleOutputPath))
var bundleMin = gulp.src(srcPaths)
                .pipe(pipeif(mapCondition, sourcemaps.init({ loadMaps: true })))
                .pipe(plugins.ngAnnotate({ add: true }))
                .pipe(concat(bundlePrefix + bundleItem.name + '-min.js'))
                .pipe(pipeif(minCondtion, uglify()))
                .pipe(pipeif(mapCondition, sourcemaps.write('./', {mapSources: (filePath) => return filePath;})))
                .pipe(gulp.dest(bundleOutputPath));

@nmccready
Copy link
Collaborator

My guess was from this Error: "../../../../../wwwroot/DevTools/DevTools/dev-tools.ts" is not in the SourceMap.

@brphelps
Copy link

brphelps commented Jan 10, 2017

That change doesn't seem to have an effect.

Pertinent code is now:

var bundle = gulp.src(srcPaths)
                .pipe(pipeif(mapCondition, sourcemaps.init({ loadMaps: true })))
                .pipe(concat(bundleFileName))
                .pipe(pipeif(mapCondition, sourcemaps.write('./', {mapSources: function(filePath) { return filePath; }})))
                .pipe(gulp.dest(bundleOutputPath))
            var bundleMin = gulp.src(srcPaths)
                .pipe(pipeif(mapCondition, sourcemaps.init({ loadMaps: true })))
                .pipe(plugins.ngAnnotate({ add: true }))
                .pipe(concat(bundlePrefix + bundleItem.name + '-min.js'))
                .pipe(pipeif(minCondtion, uglify()))
                .pipe(pipeif(mapCondition, sourcemaps.write('./', {mapSources: function(filePath) { return filePath; }})))
                .pipe(gulp.dest(bundleOutputPath));

@nmccready
Copy link
Collaborator

What happens if you remove some steps. First ngAnnotate, then concat?

@nmccready
Copy link
Collaborator

Anyway instead of going back and forth here if you make a reproducible example and PR it to integration.js on 1.X I can actually run it. I am not cutting and pasting code that might throw.

@brphelps
Copy link

Issue seems to be specific to ngAnnotate being in the pipe.

I'll try to make time to do that today. For the time being we're targeting version 1.9.3.

@twiggy
Copy link
Author

twiggy commented Jan 10, 2017

Here is a really simple tar ball that will recreate the issue.

Just two test js, a simple concat pipeline. Look in the current bundle.js.map and it should see the obvious problem."sources":["/bundle.js","/bundle.js"]

run "npm install" followed by "gulp test" to create bundle.js and bundle.js.map

Rollback version package.json and rm -R node_modules to do a fresh install. vs 2.2.3 and 1.9.3 and below seem to work fine.

Thanks!
test.tar.gz

@nmccready
Copy link
Collaborator

Thanks @twiggy

@nmccready
Copy link
Collaborator

@twiggy as for your issue, doing .pipe(sourcemaps.write('.', {mapSources: (file) => {return file;}})) does fix your problem. I will dig into it so you don't have to do it.

@nmccready
Copy link
Collaborator

I see the bug working on it.

nmccready added a commit to realtymaps/gulp-sourcemaps that referenced this issue Jan 11, 2017
nmccready added a commit to realtymaps/gulp-sourcemaps that referenced this issue Jan 11, 2017
@nmccready
Copy link
Collaborator

nmccready commented Jan 11, 2017

I think this is resolved see 0afd11c#diff-08e28e98a118d512b13f4fc7423da77aR234

Pushed/Published 2.3.1, and 1.10.1

@twiggy
Copy link
Author

twiggy commented Jan 11, 2017

thanks @nmccready !

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

No branches or pull requests

3 participants