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

Add sourcemaps into core #843

Closed
yocontra opened this issue Dec 29, 2014 · 39 comments
Closed

Add sourcemaps into core #843

yocontra opened this issue Dec 29, 2014 · 39 comments

Comments

@yocontra
Copy link
Member

cc @floridoo

Basically src/dest will have a sourcemap option. Now people don't need to use gulp-sourcemaps in their own files. This will DRY up a ton of stuff

@yocontra yocontra added the gulp4 label Dec 29, 2014
@yocontra yocontra added this to the gulp 4 milestone Dec 29, 2014
@ilanbiala
Copy link

Awesome!

@yocontra yocontra self-assigned this Dec 29, 2014
@addyosmani
Copy link

👍

2 similar comments
@sindresorhus
Copy link
Contributor

👍

@TrySound
Copy link
Contributor

👍

@insidewhy
Copy link

Does this mean I'll finally be able to merge two streams along with their source maps?

@yocontra
Copy link
Member Author

@nuisanceofcats Example?

@insidewhy
Copy link

@contra Like:

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

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

})

With full source maps all the way through, the "merge" (or an alternative) would merge the streams and their source maps.

@yocontra
Copy link
Member Author

That should still be possible with the current implementation though, just not as clean

@insidewhy
Copy link

@contra doesn't seem to be... I tried it like this:

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

But ended up with a bogus source map. I asked on github for help but none was forthcoming so I assumed it wasn't possible.

@yocontra
Copy link
Member Author

That should work... @floridoo any thoughts?

@johnpapa
Copy link

👍

@yocontra yocontra removed the gulp4 label Jan 27, 2015
@yocontra yocontra modified the milestones: gulp 4.1, gulp 4 Jan 27, 2015
@boogerlad
Copy link

@nuisanceofcats Any luck fixing that? I'm trying to get the same thing working.

@insidewhy
Copy link

Have been waiting for gulp v4 and experimenting with my own stuff. All node based asset pipelines have major flaws or restrictions, v4 will probably sort it all out.

@boogerlad
Copy link

Sadly I'm on v4 alpha. Many things still seem to be not perfect. If I am compiling a less file to a css file, the sourcemap will refer to the outputted css file if I'm using multiple sources. Oh well.

@phated
Copy link
Member

phated commented Jan 30, 2015

@boogerlad sourcemaps have nothing to do with v4 alpha. This was an issue to discuss implementing the boilerplate into gulp.src/dest. That has pushed off until gulp 4.1 anyway.

@boogerlad
Copy link

Sorry for going off topic.

@heikki heikki changed the title add sourcemaps into core Add sourcemaps into core Feb 14, 2015
@pikeas
Copy link

pikeas commented Feb 25, 2015

Based on my reading of this thread, there's no way to merge sourcemaps in Gulp v3, and this won't be added until v4.1, which is months away?

@yocontra
Copy link
Member Author

@boogerlad if there is an issue with gulp-sourcemaps then open an issue on gulp-sourcemaps, no point in reporting it on an unrelated issue

@yocontra yocontra modified the milestones: gulp 4, gulp 4.1 May 11, 2015
@yocontra
Copy link
Member Author

Moving this back to 4.0 since this is pretty easy to do

@yocontra
Copy link
Member Author

@floridoo any thoughts RE merging streams of sourcemaps?

@floridoo
Copy link

@contra the source maps are per file, so merging streams should not affect them in theory. maybe the sourcemaps property is dropped by the merge plugin? i will have a look when i find the time.

@insidewhy
Copy link

@floridoo Combining source maps needs special handling that currently isn't in gulp. Here's an implementation:

https://github.com/sighjs/sigh-core/blob/master/src/sourceMap.js#L26

@floridoo
Copy link

@ohjames merge does not combine files or sourcemaps, that's what gulp-concat is for. merge just merges streams.

@insidewhy
Copy link

Yep, unfortunately if you merge streams before passing them into concat (e.g. the input streams each use different transformations) then the output source maps are incorrect. I think there is/was a github issue floating around for this.

@insidewhy
Copy link

My previous comment is incorrect/misleading though, you're right gulp-concat does exactly that... I think.

@yocontra
Copy link
Member Author

@ohjames before we bring this into core I want to make sure that any edge cases are handled. If you have any other concerns/oddities feel free to raise them here

@yocontra
Copy link
Member Author

Added into src on vinyl-fs master, going to do dest later today then make it available for testing on 4.0 👍 gulpjs/vinyl-fs@60685eb

@insidewhy
Copy link

Cool, anyone else think the default setting for source maps should be true rather than false?

@insidewhy
Copy link

@contra I tested the merge + concat again with the git master commit, the same test I ran back in my comment from December. It still fails but I found the issue, essentially it's because one of the sources is not transformed (the bootstrap.js which should just be appended raw) and hence 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 :)

@yocontra
Copy link
Member Author

@ohjames I thought about making it true by default, but I think it makes more sense to opt-in to having more files pop up in the output and more CPU usage

@phated
Copy link
Member

phated commented Jul 12, 2015

Looks like this is 50% done (included in gulp.src but not gulp.dest). Marking as "help wanted" incase anyone wants to send a PR to https://github.com/wearefractal/vinyl-fs that adds the dest support.

@yocontra
Copy link
Member Author

@phated Just added it into .dest this morning, needs tests and docs though

@joshribakoff
Copy link

Inlining the sourcemaps like this breaks lots of older browsers like IE. If we want to support IE, is there some way to disable this? The behavior I seem to be seeing is that its enabled by default now, which means after updating gulp, lots of older browsers suddenly broke with no obvious fix.

Edit Actually not sure what's going on here. I had my calls to gulp-sourcemaps commented out, and was getting intemittent sourcemaps at random. I removed the lines altogether & got no sourcemaps in output. I'm not looking for help debugging, just pointing out a possible headache that would be caused if something like this were to be merged in & enabled by default.

@yocontra
Copy link
Member Author

@joshribakoff Not sure why you are saying it's on by default when that was never said and certainly isn't true

@yocontra
Copy link
Member Author

Also FYI for any interested parties, the sourcemaps option is now available for src and dest on vinyl-fs master. Please test out the changes and I'll cut a release if everything looks good

@yocontra
Copy link
Member Author

im going to close this since it has been implemented, but it still needs testing

@eddiemonge
Copy link

Whats the api for this?

@Dan503
Copy link

Dan503 commented Dec 27, 2018

// How to use native Sourcemaps in Gulp 4 using ES5

var sass = require('gulp-sass');

gulp.task('sass', function() {
  return gulp.src('src/styles/main.scss', { sourcemaps: true })
    .pipe(sass().on('error', sass.logError))
    .pipe(gulp.dest('site/css', { sourcemaps: true }))
});

In short you need to set { sourcemaps: true } on both gulp.src and gulp.dest for it to work.

Here is a slightly sleeker ES6 way of writing it:

// How to use native Sourcemaps in Gulp 4 using ES6

import sass from 'gulp-sass';

const sourcemaps = true;

gulp.task('sass', () => {
  return gulp.src('src/styles/main.scss', { sourcemaps })
    .pipe(sass().on('error', sass.logError))
    .pipe(gulp.dest('site/css', { sourcemaps }))
});

It outputs the sourcemap inside the CSS file though. Is there a way to output .map files instead without resorting to the gulp-sourcemaps plugin?

@Dan503
Copy link

Dan503 commented Dec 27, 2018

Ahh I found the bit of documentation covering how to do external source maps :D

https://gulpjs.com/docs/en/api/src#sourcemaps

// How to use native external Sourcemaps in Gulp 4

var sass = require('gulp-sass');

gulp.task('sass', function() {
  return gulp.src('src/styles/main.scss', { sourcemaps: true })
    .pipe(sass().on('error', sass.logError))
    .pipe(gulp.dest('site/css', { sourcemaps: '.' }))
});

'.' = a relative path to the directory you want the sourcemap to go in. '.' = same directory as the output file.

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

No branches or pull requests