Cache webpack to use on subsequent compiles

[jmurzy]

Fixes #45. Provides an alternative solution to issues mentioned in #18 and #79 using gulp watch.

This I believe is the best way to implement gulp watch support without a major refactor.

🍺

[shama]

Thanks! Could you give an example of how this would be used and why it's not possible with the current implementation? I've never fully understood the issue with cache as that works for me. I also don't use the watch option and I don't really understand why anybody needs to use effectively two watchers.

[jmurzy]

One should only use either of the two watch options. They aren't meant to be used together. The biggest drawback of the webpack's built-in watch is that it doesn't play well with gulp.

I think your confusion may be a by-product of not having to use the current impl in combination with gulp watch. Currently, you cannot do the following because webpack-stream creates a new instance of webpack each time the build task is run causing the internal cache to be dropped (compilation times don't improve).

gulp.task('build', function() {
  return gulp.src('src/entry.js')
    .pipe(gulpWebpack({}, webpack))
    .pipe(p1())
    .pipe(p2())
    .pipe(pn())
    .pipe(gulp.dest('dist/'));
});

gulp.task('default', ['build'], () => {
  gulp.watch(['src/**/*'], ['build']);
});

Also if I were to use the built-in watch, gulp plugins that follow webpack-stream (p1, p2 .. pn) in the above pipeline would go ignored. Fixing this is especially important in situations where webpack is used only as a module bundler and rest of the workflow is delegated to gulp (uglify, postcss, gzip). This really is a must when you want to maintain dev/prod parity in your build pipeline particularly when building universal apps.

Correct me if I'm wrong but I also think if there was a more idiomatic gulp way of configuring webpack to watch, this would be it.

🍺

[jamesplease]

I pulled this down and am getting an error.

[21:45:13] Starting 'work'...
/Users/jmeas/WebDev/games/bullet-test/node_modules/webpack-stream/index.js:158
    var fs = compiler.outputFileSystem = cache.mfs;
                                       ^

TypeError: Cannot set property 'outputFileSystem' of undefined
    at Stream.<anonymous> (/Users/jmeas/WebDev/games/bullet-test/node_modules/webpack-stream/index.js:158:40)
    at _end (/Users/jmeas/WebDev/games/bullet-test/node_modules/through/index.js:65:9)
    at Stream.stream.end (/Users/jmeas/WebDev/games/bullet-test/node_modules/through/index.js:74:5)
    at DestroyableTransform.onend (/Users/jmeas/WebDev/games/bullet-test/node_modules/through2/node_modules/readable-stream/lib/_stream_readable.js:495:10)
    at DestroyableTransform.g (events.js:273:16)
    at emitNone (events.js:85:20)
    at DestroyableTransform.emit (events.js:179:7)
    at endReadableNT (/Users/jmeas/WebDev/games/bullet-test/node_modules/through2/node_modules/readable-stream/lib/_stream_readable.js:865:12)
    at nextTickCallbackWith2Args (node.js:474:9)
    at process._tickCallback (node.js:388:17)

Here's the gulp task:

function work() {
  $.livereload.listen({port: 35729, host: 'localhost', start: true});
  return gulp.src(path.join('src', config.entryFileName + '.js'))
    .pipe($.plumber())
    .pipe(webpackStream({
      output: {
        filename: exportFileName + '.js',
        libraryTarget: 'umd',
        library: config.mainVarName
      },
      module: {
        loaders: [
          { test: /\.js$/, exclude: /node_modules/, loader: 'babel-loader' }
        ]
      },
      watch: true,
      devtool: 'source-map'
    }, null, function() {
      $.livereload.reload('index.html');
    }))
    .pipe(gulp.dest(destinationFolder))
    .pipe($.filter(['*', '!**/*.js.map']))
    .pipe($.rename(exportFileName + '.min.js'))
    .pipe($.sourcemaps.init({ loadMaps: true }))
    .pipe($.uglify())
    .pipe($.sourcemaps.write('./'))
    .pipe(gulp.dest(destinationFolder));
}

Ah, so the issue may be that I'm using the internal watch property of webpack rather than gulp watch? If that's the case, there should maybe be a better error msg for folks who use the webpack watch?

---

Hm, well, can't get this to work. It seems like this causes the webpack-stream to never end, even if there's no watch task set up at all. Separately from that, it just seems to blow up if I use webpack's built-in watcher.

Haven't tried the configuration you posted – so that might work – but these other qualities seem surprising to me. Can you reproduce @jmurzy ?

Also, I'm very thankful for your efforts here! I agree that this plugin needs some work to get better integration with gulp, and especially watch tasks. My current system feels pretty hacky.

[jmurzy]

@jmeas Sorry I've been swamped lately.

Gulp watch and Webpack watch are mutually exclusive.

Can you try the following and let me know if it works?

var webpack = require('webpack');

gulp.task('build', function() {
  ...
  var wpConfig = {
    output: {
      filename: exportFileName + '.js',
      libraryTarget: 'umd',
      library: config.mainVarName
    },
    module: {
      loaders: [
        { test: /\.js$/, exclude: /node_modules/, loader: 'babel-loader' }
      ]
    },
    devtool: 'source-map'
  };
  ...
  return gulp.src(path.join('src', config.entryFileName + '.js'))
  .pipe($.plumber())
  .pipe(webpackStream(wpConfig, webpack)
  .pipe(gulp.dest(destinationFolder))
  .pipe($.filter(['*', '!**/*.js.map']))
  .pipe($.rename(exportFileName + '.min.js'))
  .pipe($.sourcemaps.init({ loadMaps: true }))
  .pipe($.uglify())
  .pipe($.sourcemaps.write('./'))
  .pipe(gulp.dest(destinationFolder))
  .pipe($.livereload());
});

gulp.task('default', ['build'], function() { 
  $.livereload.listen({port: 35729, host: 'localhost', start: true});
  gulp.watch(['src/**/*.js'], ['build']);
});

🍺

[gregorskii]

This fork worked for me on my project. Hoping it gets pulled in soon. Appreciate the work all of you are doing.

Thanks!

+1

[kvet]

This feature is very helpful. Waiting while it will be reliased

[jrop]

@shama We've been using this branch in our projects consistently for about a month now, and this would be very very useful to have in our project as soon as it gets merged in. Currently this has brought rebuilds down from ~8s to ~1s in our project.

[shama]

@jmeas Are you still running into that error with this?

@jmurzy Would you be able to fix the merge conflict here? Thanks!

[jamesplease]

@jmeas Are you still running into that error with this?

Haven't tried again tbqh. I can try tonight, but it does sound like this works for a bunch of other folks. I was trying to use this with webpack's watch option, which seems to continue to be an incompatible way to go about things, even with this PR (which is fine, I think).

If this gets in, and it continues to only work with gulp watch, then I'd recommend adding a new section to the README about it. It should explain why using webpack's watch option does not play well with streams, and demonstrate how using gulp's watch works (even tho' it looks to be pretty standard gulp watch usage, it's probably good to be explicit about what to do).

What do ya think?

[jmurzy]

@shama — done.

🍺

[jmurzy]

Ping @shama.

🍺

[jamesplease]

@shama I think this should prob get merged since so many folks are having success with it. My current system is working fine for me so I haven't swapped this in, but merging this in seems like a good idea if you've got time ✌️

[jurStv]

@shama could you please merge this? thanks

[micate]

It works for me.

[graham-u]

I have been using the changes from this PR and whilst it does solve the watch issue, I have run into a caching issue.

I have multiple gulp tasks that call webpackstream, the first of which works fine but then all subsequent calls hit problems due to line 141 here:

95ab089#diff-168726dbe96b3ce427e7fedce31bb0bcR141

Line in question: var compiler = cache.compiler || webpack(config)

All gulp task calls to webpackstream after the initial one set compiler to be the cached version which means that any webpack config passed in to webpackstream in these calls is disregarded.

This affects me as my different gulp tasks set different entry values in the passed in webpack config, but they all end up working with cached config that contains the entry configuration from the first call.

Any ideas how to solve this?

[gregorskii]

I have not been using this plugin for quite a while now. Its much easier to use webpacks API directly in gulp:

let firstRun = true;

  gulp.task('webpack', (done) => {
    const _webpackConfig = Object.assign({}, webpackConfig, {
      watch: process.env.watch === 'true'
    });

    webpack(_webpackConfig, (err, stats) => {
      if (err) {
        errorHandler.throwPluginError(
          'webpack', `Error on webpack build: ${err.message}`
        );
      }

      plugins.util.log(
        '[webpack]',
        stats.toString(config.webpack.webpackOutputConfig)
      );

      if (process.env.watch === 'true' && firstRun) {
        firstRun = false;
        done();
      } else if (process.env.watch === 'false') {
        done();
      }
    });
  });

The business about the firstRun is to stop the done callback from being called again on each recompile. I set the process.env.watch value outside of the task, and its a string because env variables can only be strings.

Hope this helps.

[graham-u]

@gregorskii Thanks for putting me on the right track, I've gone back to using the API directly. Now I have cached compilers and watching all working :)

[gregorskii]

No problem!

[belfz]

@shama @jmurzy will this ever be merged, or was there trouble with this feature? Thanks

[jmurzy]

@belfz Last commit to this lib was back in April. I'm not sure if @shama is still interested in maintaining it. You can just use my fork instead. We are using it in prod and it works very well.

🍺

[belfz]

Thanks @jmurzy. It just feels much safer if I used the official release, not the branch from forked repository. But I will give it a try 👍

[hensansi]

I used the fork with an hello javascript file and it has some strange behavior.

Initially compiles, the second time it doesn't and afterwards it does but I was trying to check why it doesn't compile on the second time and I noticed that on the subsequent compiles there is something inside of the compiler that increments too fast.

Do you have any idea why it doesn't emit the first time you make a change on a file, @jmurzy ? I am trying to debug it but I can't find why.

[wub]

Hey @shama, any chance you could add a volunteer contributor to manage this repo (no idea who)? Sucks having to manually patch several different PRs to fix my issues. Thanks Kyle!

[jamesplease]

For future visitors, this lib could be worth checking out. I temporarily pulled out webpack-stream from my build process due to this bug and others, but I intend to try out this other lib going forward.

I normally wouldn't recommend something I hadn't tried before, but this issue has been going on so long I figured I'd mention it in case others wanted to check it out.

There aren't many stars, but the author of that lib is involved in a few open source projects, which gives me confidence in his work (I also have a lot of confidence in @shama 's work!)

[rmacklin]

@shama First, thanks for merging/closing several of the other PRs that were outstanding for a while, and releasing v4.0.0!

Second, is there anything preventing this PR from getting merged? These changes significantly speed up my watch task, so it would be awesome to get them merged into the official package. Let me know if there's anything you see as blocking this from being merged. Thanks!

[jordisala1991]

Ping @shama

[shama]

Thanks for the ping. I still don't fully understand the need for this but it seems helpful for a lot of you all so I'll merge and release. Thanks for your patience and thank you @jmurzy for putting it together (especially appreciate the test and docs).

[shama]

Pinging those in this thread about: #201

[shama]

I still don't understand the use case so I was relying on those who originally wanted this to review #207 on whether that introduces a regression for you. I'm guessing it will because the test needs to be removed in order to support the change. But I haven't heard feedback so I'm going to make changes for the most vocal on this by merging #207. If you're coming to this PR because you got bit by the next release, my apologies.

[cmlenz]

Our incremental builds got really slow and would start crashing (out of memory) after a couple cycles after upgrading to webpack-stream@6. So I guess we got bitten by the change in #207