gulp-community / gulp-concat

Streaming concat middleware for gulp
MIT License
792 stars 127 forks source link

Generates incorrect source map when one of the source vinyls has no sourceMap #94

Closed insidewhy closed 9 years ago

insidewhy commented 9 years ago

e.g.:

var gulp       = require('gulp')
var traceur    = require('gulp-traceur')
var sourcemaps = require('gulp-sourcemaps')
var concat     = require('gulp-concat')
var merge      = require('merge-stream')

gulp.task('default', function() {
  merge(
    gulp.src(['app/*.js', 'app/*/*.js']).pipe(sourcemaps.init()).pipe(traceur())
    gulp.src('bootstrap.js').pipe(sourcemaps.init())
  )
  .pipe(concat('output.js'))
  .pipe(sourcemaps.write())
  .pipe(gulp.dest('dist/assets'))
})

The vinyl for bootstrap.js has an empty source map attached by this line of code:

https://github.com/floridoo/gulp-sourcemaps/blob/master/index.js#L107

gulp-concat should detect empty source maps and replace them with identity source maps. Either this or that line there could be changed to attach an identity source map instead of an empty source map.

sigh takes the former approach as it's slightly more efficient and abstracts it behind the source map API. With that small change then gulp-concat will produce correct source maps :)

Here is a code sample that shows how to generate identity source maps.

First reported in https://github.com/gulpjs/gulp/issues/843#issuecomment-101400811

yocontra commented 9 years ago

I think this should be done in gulp-sourcemaps not in each plugin

insidewhy commented 9 years ago

It wouldn't need to be in each plugin, just in gulp-concat. I don't think any other plugins mind if they get an empty source map. To generate the identity map you'll need to parse the source code with something like esprima which might seem a bit heavy considering they aren't needed in most cases.

yocontra commented 9 years ago

other option is handling it in concat-with-sourcemaps but it should not be in gulp-concat

insidewhy commented 9 years ago

Okay I'll leave you to open the bug against concat-with-sourcemaps. This being an issue for the last few years is kinda what made me decide to compete with gulp and write a new asset pipeline, I really hope you can fix it, I want good competition to drive my own work forwards!

yocontra commented 9 years ago

@ohjames last couple years? gulp-sourcemaps is like less than 1yr old and gulp is still a pretty new project

insidewhy commented 9 years ago

Surprised, feels like years.

On 13 May 2015 19:49:01 BST, contra notifications@github.com wrote:

@ohjames last couple years? gulp-sourcemaps is like less than 1yr old and gulp is still a pretty new project


Reply to this email directly or view it on GitHub: https://github.com/wearefractal/gulp-concat/issues/94#issuecomment-101774079

yocontra commented 9 years ago

@ohjames Bugs can't get fixed if nobody opens a ticket. Your fault for not opening a ticket when you encountered a bug.