Add Feature: allow pass stream to each asset files before concat

[chaizhenhua]

hello @jonkemp, this patch allow transform assets before concat. now the syntax is like
$.useref.assets(opts, streamForEachAssetsBeforeConcat1, streamForEachAssetsBeforeConcat2, ...)
Now we can allow sourceMaps for assets concat in this way:

gulp.task('html', ['scripts', 'styles'], function() {
  var assets = $.useref.assets({searchPath: ['.tmp/', 'src/']},
                              $.sourcemaps.init({loadMaps: true, debug: true}));  // for each asset file load and init sourceMap
  return gulp.src('src/*.html')
    .pipe(assets)
    .pipe($.sourcemaps.write('.'))
    .pipe(assets.restore())
    .pipe($.useref())
    .pipe($.revReplace())
    .pipe(gulp.dest('./dist'))
});

[sjurba]

This looks like a good way to solve #64

• 1 to get this merged!

[sjurba]

Would it be reasonable to set the base of the files to the base of file?
var stream = vfs.src(src, {base:file.base});

I need this to be able to get the right source values in the source-map.

[chaizhenhua]

hi, @sjurba, thanks for review this.
the glob pattern already contains path info. so we do not need extra options.

[sjurba]

When using this to generate a sourcemap we need to set the base for the sources to have the correct relative paths. Without it, the sources list in the sourcemap will only contain the filename without any path info.
In other words, now the src glob has absolute urls:

/path/to/app/compA/file1.js
/path/to/app/compB/file2.js

Resulting in sourcemapping:

file1.js
file2.js

Where I want:

compA/file1.js
compB/file2.js

Setting the base property in the glob fixes that.

[chaizhenhua]

@sjurba updated. but The Travis CI build failed. it past on my own machine. I will check it later.

[mnquintana]

👍

[sjurba]

Nice!

[courtnek]

👍 I'd find this useful as well.

[jonkemp]

Why does this change break the tests?

[chaizhenhua]

{base: file.base} option breaks the tests. if we remove the option, tests will pass. I do not have enough time to deal with it.

[jonkemp]

I do not have enough time to deal with it.

@chaizhenhua if that was a serious comment, then I find it extremely disrespectful. When you propose a change that breaks tests and your response is "I don't have time to deal with it", that will greatly decrease your chances of getting a pull request merged and is just bad behavior in general.

Please refer to the guidelines on contributing here https://github.com/jonkemp/gulp-useref/blob/master/CONTRIBUTING.md and make note of the following.

Following these guidelines helps to communicate that you respect the time of the developers managing and developing this open source project.

If you don't have enough time for a pull request, then just make your comments in an existing issue or create a new one and leave it at that. Thanks.

[chaizhenhua]

@jonkemp Don't be so serious. but your advice is useful. I said I will check it later and now i fix it.

[jonkemp]

I think the nosort option was added at some point before this and should be left in, unless there is a reason it was taken out.

[chaizhenhua]

if we set nosort to true the tests may fail sometimes.
I think it's beacuse the order of the sync output is undefined.

[jonkemp]

Ok, I finally had some time to review this. It seemed like there were significant code changes here, and indeed there are. This change doesn't just add support for sourcemaps. It changes how assets are read from the file system and concatenated. It's not even specific to sourcemaps. Assets can now be piped as a stream to any module that accepts streams.

I think the biggest change here is the switch from using fs to vinyl-fs which is what makes the functionality I mentioned above possible. This is a welcome change as using fs is against gulp plugin recommendations anyway and not using it likely will make the process faster. That's something that's been on my list, but I haven't had the time and am not that familiar with streams anyway.

So I think this is good and worthy of being merged but I would like to get some more feedback on it before I include it.

@sindresorhus @silvenon @addyosmani or anyone else who has time, if you would like to code review this change, it would be greatly appreciated.

[jonkemp]

Ok, if you check the latest code in this repo, I've added a 'noconcat' option that does not concat assets, but instead adds them to the stream. It's also been refactored significantly and supports streaming of assets to other plugins like gulp-sourcemaps. I would appreciate it if you would try this out before I release it to npm. Just check out the latest version and npm link it or copy it to your node_modules folder. Thanks!

[bminer]

@jonkemp - I don't understand how this works.

Look here... https://github.com/jonkemp/gulp-useref/blob/master/index.js#L110-L112

This won't work, I don't think. In fact, I think that what needs to happen is that all of the src files are pulled together in one go, slammed through a stream, then slammed through concat, then exposed to the next stream.

My point is, I cannot get sourcemaps to work with useref.

Closest thing I have working is:

var assets = useref.assets(null
    , function() {return sourcemaps.init({loadMaps: true, debug: true});}
    , function() {return sourcemaps.write({
        "sourceRoot": "."
    });}
);

and I have modified useref as follows:

/*                streams.forEach(function (stream) {
                    src = src.pipe(stream() );
                });*/

                // Add assets to the stream
                // If noconcat option is false, concat the files first.
                src
                    .pipe(streams[0]())
                    .pipe(gulpif(!opts.noconcat, concat(name)))
                    .pipe(streams[1]())

Hope that helps...

[JetFault]

I also haven't been able to get sourcemaps working.

I'm using this for my task:

gulp.task('html', [], function() {
  var htmlFilter = $.filter('*.html'),
    jsFilter = $.filter('**/*.js'),
    cssFilter = $.filter('**/*.css'),
    assets = $.useref.assets({}, $.sourcemaps.init({loadMaps: true, debug: true}));


  return gulp.src(['./tmp/*.html'])
    .pipe(assets)
      .pipe($.rev())
      .pipe($.sourcemaps.write('./maps'))
    .pipe(assets.restore())
    .pipe($.useref())
    .pipe($.revReplace())
    .pipe(gulp.dest('dist'));
});

And the files its being run on is just a bunch of bower_components files:

    <!-- build:js assets/scripts/vendor.js-->
    <!-- bower:js-->
    <script src="../bower_components/jquery/dist/jquery.js"></script>
    <script src="../bower_components/lodash/dist/lodash.js"></script>
    <!-- endbower-->
    <!-- endbuild-->

Sourcemaps aren't being created at all.

[Chalkin]

I don't get the sourcemaps to work.

That's the error thrown:

/node_modules/gulp-useref/index.js:111
                    src.pipe(stream());
                             ^
TypeError: object is not a function
    at /node_modules/gulp-useref/index.js:111:30
    at Array.forEach (native)
    at DestroyableTransform.<anonymous> (/node_modules/gulp-useref/index.js:110:25)
    at Array.forEach (native)
    at DestroyableTransform.<anonymous> (/node_modules/gulp-useref/index.js:70:32)
    at Array.forEach (native)
    at DestroyableTransform.through.obj.end [as _transform] (/node_modules/gulp-useref/index.js:63:15)
    at DestroyableTransform.Transform._read (/node_modules/gulp-useref/node_modules/through2/node_modules/readable-stream/lib/_stream_transform.js:184:10)
    at DestroyableTransform.Transform._write (/node_modules/gulp-useref/node_modules/through2/node_modules/readable-stream/lib/_stream_transform.js:172:12)
    at doWrite (/node_modules/gulp-useref/node_modules/through2/node_modules/readable-stream/lib/_stream_writable.js:237:10)

gulp.task('scripts-prod', function() {

    var jsFilter = plugins.filter('**/*.js');
    var cssFilter = plugins.filter('**/*.css');
    //var userefAssets = plugins.useref.assets();
    var assets = plugins.useref.assets({searchPath: ['.tmp/', 'src/']},
        plugins.sourcemaps.init({loadMaps: true, debug: true}));  // for each asset file load and init sourceMap

    return gulp.src(paths.indexHtml)
        .pipe(assets)
        .pipe(plugins.sourcemaps.write('.'))
        .pipe(jsFilter)
        .pipe(plugins.uglify())
        .pipe(jsFilter.restore())
        .pipe(cssFilter)
        .pipe(cssFilter.restore())
        .pipe(plugins.rev())
        .pipe(assets.restore())
        .pipe(plugins.useref())
        .pipe(plugins.revReplace())
        .pipe(gulp.dest(paths.distFolder));
});

[k41n]

I am getting exactly the same error as @Chalkin :

  assets = plugins.useref.assets({searchPath: path.temp}, sourcemaps.init({debug: true}))

  gulp.src(["#{path.app}/index.html", "#{path.app}/index-widget.html", "#{path.app}/index-form.html", "#{path.app}/index-form-and-hub.html", "#{path.app}/proxy.html", "#{path.app}/iframe-content.html"])
  # Concatenate with gulp-useref
  .pipe(assets)
  # .pipe(sourcemaps.write('.'))
  .pipe(plugins.if('*.js', plugins.uglify(config.uglify)))
  .pipe(plugins.if('*.css', plugins.csso()))

  .pipe(gulp.dest("#{path.temp}"))

TypeError: object is not a function
  at /Users/k41n/ng-social-hub/node_modules/gulp-useref/index.js:114:36

npm list gulp-useref
...
└── gulp-useref@1.2.0

[fedotxxl]

same exception

[xts-velkumars]

same exception, if i used below line
var assets = plugins.useref.assets({searchPath: ['.tmp/', 'src/']},

npm list gulp-useref
└── gulp-useref@1.3.0