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

[bug?] Piping not working when watch is true #79

Open
jamesplease opened this issue Oct 10, 2015 · 13 comments
Open

[bug?] Piping not working when watch is true #79

jamesplease opened this issue Oct 10, 2015 · 13 comments

Comments

@jamesplease
Copy link

Type: bug (maybe)

What happens: Piping from webpack-stream to, say, gulp-concat isn't working when watch is set to true.

Steps to reproduce

  1. Set up webpack-stream to accept n entries using either method in this project's README
  2. Ensure that watch isn't on, and pipe the output to gulp-concat.
  3. Observe that it works
  4. Turn on watch
  5. Observe that it doesn't work

Possible cause: This might have to do with the internals of webpack's watch changing what is sent out from the pipe? I'm not sure – gotta investigate that.

Why am I doing this?

This is likely me being a goof somewhere. My set up is that I've got n independent test files for a library. I want to build them with webpack into a single file that can be loaded into the browser.

Afaik webpack doesn't support n entry points into 1 file; each independent entry point is converted into its own "chunk." For this reason, I'm piping webpack into gulp-concat to produce a single file to load up in the browser.

I don't want to manually maintain a list of test files, so somewhere along the line I've got to glob. I guess the two options are:

  1. somehow set up a require(); in my code that allows for globbing
  2. set up globbing when specifying my target files, then concatenating them

Option 1 seemed like more work (the glob-loader plugin isn't very feature-ful...no source maps, for instance), so I was trying to do option 2.

Is this silly? : P

I'll do my best to investigate this, but I figured I'd raise the issue in case anyone happens to know what the issue is.

p.s. super awesome library here, @shama!

@jamesplease
Copy link
Author

Update: the error is the conditional around this line. Removing that solves the problemo.

Will update with more as I find it...

This was added here, 094e933, without any explanation.

@shama, do you know why "Elie Rotenberg" added this? 😮

also ping @elierotenberg if you're the author of that commit 😛

jamesplease added a commit to jamesplease/webpack-stream that referenced this issue Oct 10, 2015
For whatever reason, this conditional would break the output of the module. I have no idea why this was added, but removing it does fix the issue. I've verified that this line is the culprit by removing it altogether, and in that case the build always fails, whether watch is on or not.

Resolves shama#79
@jamesplease
Copy link
Author

Well, even with the bug resolved this doesn't help my original problem. 😦 While the initial build works awesomely, subsequent updates to a file don't trigger the concat task to run!

The search continues...

@shama
Copy link
Owner

shama commented Oct 10, 2015

@jmeas That is a weird bug. Although it sounds like you can handle this use case purely with webpack. Which is recommended as webpack will handle the incremental builds much more efficiently than trying to connect up with gulp.

Instead of multiple entry points, you can create a single entry point that loads each of the previous entry points.

Otherwise check out the CommonsChunkPlugin: https://github.com/webpack/docs/wiki/optimization#multi-page-app in which you can control which parts of common chunks across multiple entry points are shared into a single file.


As for requiring globs of files, you can do that with webpack require.context:

var requireTest = require.context('./test', true, /_test\.js$/)
requireTest.keys().forEach(requireTest)

Which will require any file in the ./test folder that ends with '_test.js'.

@elierotenberg
Copy link

Hi!
My commit merely changed the code organization for consistency, absolutely 0 semantics impact IIRC!

It seems that the conditional was added here instead: 4036e53

@jamesplease
Copy link
Author

@jmeas That is a weird bug. Although it sounds like you can handle this use case purely with webpack. Which is recommended as webpack will handle the incremental builds much more efficiently than trying to connect up with gulp.

Yeah, I'm beginning to think that may be the case.

Thanks for the options on different ways to do this. I discovered the LimitChunkCountPlugin, which seems to be another way to get n => 1 builds. I'll prob. take a look at these other options to see if any seem cleanest/simplest/etc.

My commit merely changed the code organization for consistency, absolutely 0 semantics impact IIRC!

Ah! Thanks for the clarification.

@shama
Copy link
Owner

shama commented Oct 10, 2015

👍

@minwe
Copy link

minwe commented Dec 24, 2015

I got this problem when using webpack-stream with run-sequence.

@Hurtak
Copy link

Hurtak commented Jan 9, 2016

same thing here, run-sequence does not continue to next task when watch is set to true

@jamesplease
Copy link
Author

@minwe / @Hurtak, I had the same issue. I got around it with some clever finagling.

/* as part of the JS build task... */

// keep track of our first build
var firstBuild = false;

// ...start the stream, and then...
    .pipe(webpackStream({
      watch: true,
      // ...the rest of your config 
    }, null, function() {
      if (firstBuild) {
        $.livereload.listen({port: 35729, host: 'localhost', start: true});
        watch();
      } else {
        $.livereload.reload('http://localhost:5000');
      }
      firstBuild = false;
    }))

Define your watch fn like so:

function watch() {
  // for example...
  gulp.watch('src/stylus/*.styl', ['stylus']);
  $.livereload.listen();
}

Now running the task that builds your JS will also set up a watcher for your other things. Just be sure to run that task in the last "grouping" within runSequence. For instance, you could do...

gulp.task('work', function(done) {
  runSequence(
    ['lint', 'clean'],
    ['images', 'fonts', 'copyAssets', 'stylus', 'build-javascript'],
    done
  );
});

where build-javascript is the one that runs webpack with the watcher.

Lmk if you have any problems with this method!

@minwe
Copy link

minwe commented Feb 22, 2016

@jmeas Thanks. I'll try it later.

@jmurzy
Copy link
Contributor

jmurzy commented Mar 18, 2016

@jmeas I created a PR that should address this in a more gulp friendly way. See #109.

@minwe @Hurtak This should also resolve the issue with run-sequence—as well as Gulp 4's series and parallel.

Let me know how this works for you.

@minwe
Copy link

minwe commented Mar 21, 2016

@jmeas I'm using run-sequence, but if the webpack task set watch to true, the task after webpack will never be executed.

For example:

gulp.task('webpack', function() {
  return gulp.src('...')
    .pipe(webpack({watch: true}))
    .pipe(gulp.dest('...'));
});

gulp.task('otherTask', function() {
  return gulp.src('...');
  // ...
});

gulp.task('build', function(cb) {
  runSequence('webpack', 'otherTask', cb);
});

Then run gulp build, the wepback task will be executed, but otherTask will not.

@jamesplease
Copy link
Author

@minwe, correct. From my description above:

Just be sure to run that (webpack-stream) task in the last "grouping" within runSequence.

The current behavior produces a never-ending and blocking stream. With hope, @minwe's solution, #109, will be a proper solution rather than the "clever finagling" that I went with. I've been meaning to pull down the branch to try it out for myself, but I haven't had the free time.

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

Successfully merging a pull request may close this issue.

6 participants