Error implementing changes added by PR#51

[codekirei]

Hi all, I'm having trouble successfully replicating the functionality added with #51.

If I follow the example in the readme exactly:

var gulp = require('gulp');
var rev  = require('gulp-rev');
gulp.task('default', function(){
    return gulp.src(['assets/css/*.css', 'assets/js/*.js'], {base: 'assets'})
        .pipe(gulp.dest('build/assets'))
        .pipe(rev())
        .pipe(gulp.dest('build/assets'))
        .pipe(gulp.src('build/assets/rev-manifest.json', {base: 'assets'}))
        .pipe(rev.manifest())
        .pipe(gulp.dest('build/assets'));
});

I get this error:

stream.js:94
      throw er; // Unhandled stream error in pipe.
            ^
Error: write after end

Maybe it has to do with the gulp.src call mid-stream?

If it helps, I have style.css in assets/css/ and scripts.js in assets/js, and if I look in build/assets after the error, css/ has style.css and style-hash.css, while js/ only has scripts.js. This would seem to indicate the error is somewhere between rev'ing those assets, but when I remove the code specific to generating rev-manifest.json both types of assets are rev'd properly.

[bobthecow]

Try it with the es.merge version and see if that's any better?

[codekirei]

So the es.merge version runs without error but doesn't preserve old values in rev-manifest.json. I put an existing manifest with data from another project in build/assets, and it gets completely overwritten. If I'm understanding correctly, I would have expected the new key:value pairs to get appended to the bottom of the existing manifest because the keys are different.

Thus, unless I'm screwing something up on my end, while the code runs, I think it defeats the purpose of the PR?

@c0, you said you have this working in an app. Are you succesfully adding new data to rev-manifest.json (or whatever you're renaming it to) from separate tasks without overwriting? If so, would you mind sharing some examples of what your gulp.tasks look like?

Code I used:

var gulp = require('gulp');
var rev  = require('gulp-rev');
var es   = require('event-stream');

gulp.task('default', function(){

  var assets = gulp.src(['assets/css/*.css', 'assets/js/*.js'], {base: 'assets'})
    .pipe(gulp.dest('build/assets'))
    .pipe(rev())
    .pipe(gulp.dest('build/assets'));

  var manifest = gulp.src('build/assets/rev-manifest.json', {base: 'assets'});

  return es.merge(assets, manifest)
    .pipe(rev.manifest())
    .pipe(gulp.dest('build/assets'));
});

[bobthecow]

Try changing the base on your rev-manifest src call to be build/assets?

[codekirei]

Still overwriting rev-manifest.json.

Same es.merge code as above, except changed this line:

var manifest = gulp.src('build/assets/rev-manifest.json', {base: 'assets'});

to this, as instructed:

var manifest = gulp.src('build/assets/rev-manifest.json', {base: 'build/assets'});

[codekirei]

Quick update: just to be sure I'm not crazy, I split the example from the readme into two separate tasks (css/js). Both take rev-manifest.json as source with gulp-filter, but both overwrite the manifest on each run.

Code for the curious:

var gulp       = require('gulp');
var rev        = require('gulp-rev');
var gulpFilter = require('gulp-filter');

var jsFilter  = gulpFilter(['**/*.js']);
var cssFilter = gulpFilter(['**/*.css']);

var src = [
    'assets/css/**/*.css',
    'assets/js/**/*.js',
    'build/assets/rev-manifest.json'
];

function taskBase(filterType){
    return gulp.src(src, {base: 'assets'})
        .pipe(filterType)
        .pipe(gulp.dest('build/assets'))
        .pipe(rev())
        .pipe(gulp.dest('build/assets'))
        .pipe(filterType.restore())
        .pipe(rev.manifest())
        .pipe(gulp.dest('build/assets'));
}

gulp.task('css', function(){
    taskBase(cssFilter);
});

gulp.task('js', function(){
    taskBase(jsFilter);
});

gulp.task('default', function(){
    return gulp.start(
        'js',
        'css'
    );
});

[c0]

D'oh. I see what happened. I submitted a PR to fix it: https://github.com/sindresorhus/gulp-rev/pull/59

Please comment there to see if it works for you.

Backstory: I unintentionally found a different way to create a manifest.json that appeared to be appending in my app:

• gulp tasks for css / js to build into public/assets
• Separate manifest task that takes the src of public/assets, filters out fingerprinted names, and writes to the manfest.json.

So each time it took whatever was in the build dir and created a manifest, rather than using what came down the gulp pipe.

[micahlmartin]

the looker/gulp-rev fix works me. Any reason not to merge it?