search

browserify / watchify

watch mode for browserify builds
Other
1.79k stars 203 forks source link

not updating race condition fix #249

Closed ghost closed 9 years ago

ghost commented 9 years ago

This patch fixes #216 by tracking the updating status and refusing to close watchers while the new bundle is being generated. Previously, a race condition would remove watchers from fwatchers when 2 parallel 'update' events would use the same data structures.

This issue is easy to replicate on a big project by doing: while true; do touch main.js; done. Before, this would stop watchify from tracking changes to main.js very quickly. With this fix, watchify keeps on cranking.

zertosh commented 9 years ago

lgtm! somewhat related, how do you feel about scheduling the update event on a debounce instead of a throttle?

zertosh commented 9 years ago

oh wait... if you choose not to call bundle() on update, then nothing ever sets updating back to false. updating = true should probably happen in the b.on('bundle', ...), right?

ghost commented 9 years ago

When I put updating=true in the bundle event, the test suite hangs, so I put it back where it was previously.

zertosh commented 9 years ago

Ah because this line in browserify should read output.pipe(concat(...));. The tests pass in a callback to bundle() instead of reading from the output stream. When used in that form, because of that bug, end never happens and the tests hang.

I'm stuck at work right now, but I can fix that later tonight.

ghost commented 9 years ago

Fixed upstream issue in browserify 11.0.1.

zertosh commented 9 years ago

Nice! Just tried https://github.com/substack/watchify/commit/227be4e3849fc8b35d35d017f2091975a846d2aa and the tests pass now :smile:

MrMMorris commented 9 years ago

shoot, looks like this didn't fix my issue... Maybe it's related to the fsevents issue :(

It seems only a couple files don't trigger rebuilds