Open soryy708 opened 5 years ago
I made an automated test bench that reproduces this at various versions of watchify.
The test bench uses the source and gulpfile as said previously. What it tests for specifically is for the process being frozen. The way it works is as follows:
gulpfile.js
is invoked via require('child_process').exec
with a timeout
option.timeout
was exceeded, meaning the process froze.My results are as follows:
3.3.1
through 3.11.1
3.0.0
through 3.3.0
I did not test on versions prior to 3.3.0
, but the test bench can be trivially modified to include them in the testing.
Here's a diff between 3.3.0
and 3.3.1
where I presume the breaking change happened:
diff --git a/index.js b/index.js
index 921df02..afe6ccc 100644
--- a/index.js
+++ b/index.js
@@ -16,6 +16,7 @@ function watchify (b, opts) {
var delay = typeof opts.delay === 'number' ? opts.delay : 600;
var changingDeps = {};
var pending = false;
+ var updating = false;
var wopts = {persistent: true};
if (opts.ignoreWatch) {
@@ -93,14 +94,20 @@ function watchify (b, opts) {
watchFile(mfile, dep);
});
});
+ b.on('bundle', function (bundle) {
+ updating = true;
+ bundle.on('error', onend);
+ bundle.on('end', onend);
+ function onend () { updating = false }
+ });
function watchFile (file, dep) {
dep = dep || file;
if (ignored) {
- if (!ignoredFiles.hasOwnProperty(file)) {
- ignoredFiles[file] = anymatch(ignored, file);
- }
- if (ignoredFiles[file]) return;
+ if (!ignoredFiles.hasOwnProperty(file)) {
+ ignoredFiles[file] = anymatch(ignored, file);
+ }
+ if (ignoredFiles[file]) return;
}
if (!fwatchers[file]) fwatchers[file] = [];
if (!fwatcherFiles[file]) fwatcherFiles[file] = [];
@@ -119,6 +126,9 @@ function watchify (b, opts) {
function invalidate (id) {
if (cache) delete cache[id];
if (pkgcache) delete pkgcache[id];
+ changingDeps[id] = true;
+ if (updating) return;
+
if (fwatchers[id]) {
fwatchers[id].forEach(function (w) {
w.close();
@@ -126,13 +136,14 @@ function watchify (b, opts) {
delete fwatchers[id];
delete fwatcherFiles[id];
}
- changingDeps[id] = true
// wait for the disk/editor to quiet down first:
if (!pending) setTimeout(function () {
pending = false;
- b.emit('update', Object.keys(changingDeps));
- changingDeps = {};
+ if (!updating) {
+ b.emit('update', Object.keys(changingDeps));
+ changingDeps = {};
+ }
}, delay);
pending = true;
}
diff --git a/package.json b/package.json
index 163e411..9113a23 100644
--- a/package.json
+++ b/package.json
@@ -1,12 +1,12 @@
{
"name": "watchify",
- "version": "3.3.0",
+ "version": "3.3.1",
"description": "watch mode for browserify builds",
"main": "index.js",
"bin": "bin/cmd.js",
"dependencies": {
"anymatch": "^1.3.0",
- "browserify": "^11.0.0",
+ "browserify": "^11.0.1",
"chokidar": "^1.0.0",
"defined": "^1.0.0",
"outpipe": "^1.1.0",
I just ran the test bench on the same versions range, but this time using requireq
instead of import
.
In 3.0.0
through 3.3.0
it says "Cannot find module 'someNonExistantPackage'" as it should. In all newer versions it freezes.
So the workaround is to use version 3.3.0
... which is 4 years old at this point.
Looks like the root cause may be an issue with the implementation of a fix done years ago:
track updating status to remove race condition that deletes watchers
Commenting out b.on('bundle', function (bundle) { ... })
fixes this bug.
Now lets see how to fix it properly...
The offending line of code is specifically bundle.on('error', onend);
. If this is commented out, this bug stops happening.
Perhaps we're preventing browserify from handling the error properly by overwriting the callback instead of adding a new one?
If
browserify
is given thewatchify
plugin, it will silently freeze if there's an import of a non-existent module anywhere in the source files reachable viaentries
.I've isolated a minimal complete reproducible example:
src.js:
gulpfile.js:
What I expect to happen is for an error to be thrown. I should see it either in
.on('error', console.error)
or by the gulp task crashing. What happens in practice is that the task freezes without printing anything.If the
watchify
plugin is removed from the chain, then the task continues and errors successfully:which is expected, because I removed the babel transform from the example to keep it minimal. If babel is added, then
dst.js
is successfully generated as long aswatchify
is not used.This happens to me on: