Merge pull request #4165 from webpack/bugfix/detect-delete

allow watcher to detect removals

lib/Compiler.js

@@ -16,6 +16,7 @@ function Watching(compiler, watchOptions, handler) {
 	this.error = null;
 	this.stats = null;
 	this.handler = handler;
+	this.closed = false;
 	if(typeof watchOptions === "number") {
 		this.watchOptions = {
 			aggregateTimeout: watchOptions
@@ -92,12 +93,14 @@ Watching.prototype._done = function(err, compilation) {
 	else
 		this.compiler.applyPlugins("failed", this.error);
 	this.handler(this.error, this.stats);
-	if(!this.error)
+	if(!this.error && !this.closed)
 		this.watch(compilation.fileDependencies, compilation.contextDependencies, compilation.missingDependencies);
 };
 
 Watching.prototype.watch = function(files, dirs, missing) {
+	this.pausedWatcher = null;
 	this.watcher = this.compiler.watchFileSystem.watch(files, dirs, missing, this.startTime, this.watchOptions, function(err, filesModified, contextModified, missingModified, fileTimestamps, contextTimestamps) {
+		this.pausedWatcher = this.watcher;
 		this.watcher = null;
 		if(err) return this.handler(err);
 
@@ -111,6 +114,7 @@ Watching.prototype.watch = function(files, dirs, missing) {
 
 Watching.prototype.invalidate = function() {
 	if(this.watcher) {
+		this.pausedWatcher = this.watcher;
 		this.watcher.pause();
 		this.watcher = null;
 	}
@@ -125,10 +129,15 @@ Watching.prototype.invalidate = function() {
 Watching.prototype.close = function(callback) {
 	if(callback === undefined) callback = function() {};
 
+	this.closed = true;
 	if(this.watcher) {
 		this.watcher.close();
 		this.watcher = null;
 	}
+	if(this.pausedWatcher) {
+		this.pausedWatcher.close();
+		this.pausedWatcher = null;
+	}
 	if(this.running) {
 		this.invalid = true;
 		this._done = () => {

lib/node/NodeWatchFileSystem.js

@@ -36,7 +36,8 @@ class NodeWatchFileSystem {
 		if(callbackUndelayed)
 			this.watcher.once("change", callbackUndelayed);
 
-		this.watcher.once("aggregated", (changes) => {
+		this.watcher.once("aggregated", (changes, removals) => {
+			changes = changes.concat(removals);
 			if(this.inputFileSystem && this.inputFileSystem.purge) {
 				this.inputFileSystem.purge(changes);
 			}
@@ -54,10 +55,15 @@ class NodeWatchFileSystem {
 		}
 		return {
 			close: () => {
-				this.watcher.close();
+				if(this.watcher) {
+					this.watcher.close();
+					this.watcher = null;
+				}
 			},
 			pause: () => {
-				this.watcher.pause();
+				if(this.watcher) {
+					this.watcher.pause();
+				}
 			}
 		};
 	}

package.json

@@ -21,7 +21,7 @@
     "supports-color": "^3.1.0",
     "tapable": "~0.2.5",
     "uglify-js": "^2.8.5",
-    "watchpack": "^1.2.0",
+    "watchpack": "^1.3.1",
     "webpack-sources": "^0.2.0",
     "yargs": "^6.0.0"
   },

test/WatchDetection.test.js

@@ -102,9 +102,9 @@ describe("WatchDetection", () => {
 				function step4() {
 					onChange = null;
 
-					watcher.close();
-
-					done();
+					watcher.close(() => {
+						setTimeout(done, 1000);
+					});
 				}
 
 				function handleError(err) {

test/WatchTestCases.test.js

@@ -21,7 +21,11 @@ function copyDiff(src, dest) {
 		if(directory) {
 			copyDiff(srcFile, destFile);
 		} else {
-			fs.writeFileSync(destFile, fs.readFileSync(srcFile));
+			var content = fs.readFileSync(srcFile);
+			if(/^DELETE\s*$/.test(content.toString("utf-8")))
+				fs.unlinkSync(destFile);
+			else
+				fs.writeFileSync(destFile, content);
 		}
 	});
 }
@@ -109,7 +113,9 @@ describe("WatchTestCases", () => {
 						copyDiff(path.join(testDirectory, run.name), tempDirectory);
 
 						const compiler = webpack(options);
-						const watching = compiler.watch({}, (err, stats) => {
+						const watching = compiler.watch({
+							aggregateTimeout: 1000
+						}, (err, stats) => {
 							if(stats.hash === lastHash)
 								return;
 							lastHash = stats.hash;
@@ -175,7 +181,7 @@ describe("WatchTestCases", () => {
 								run = runs[runIdx];
 								setTimeout(() => {
 									copyDiff(path.join(testDirectory, run.name), tempDirectory);
-								}, 50);
+								}, 1500);
 							} else {
 								watching.close();
 								process.nextTick(done);

test/watchCases/context/delete-in-context/0/index.js

@@ -0,0 +1,4 @@
+it("should detect changes in a context", function() {
+	var context = require.context("./directory");
+	context.keys().length.should.be.eql((+WATCH_STEP) % 3 * 2);
+});

test/watchCases/context/delete-in-context/1/directory/a.js

@@ -0,0 +1 @@
+// a

test/watchCases/context/delete-in-context/2/directory/b.js

@@ -0,0 +1 @@
+// b

test/watchCases/context/delete-in-context/3/directory/a.js

@@ -0,0 +1 @@
+DELETE

test/watchCases/context/delete-in-context/3/directory/b.js

@@ -0,0 +1 @@
+DELETE

test/watchCases/parsing/switching-harmony/0/index.js

@@ -11,11 +11,11 @@ it("should flag modules correctly", function() {
 	require("./hh").default.should.be.eql("hh" + WATCH_STEP);
 	require("./cc").should.be.eql("cc" + WATCH_STEP);
 	switch(WATCH_STEP) {
-		case 0:
+		case "0":
 			require("./hc").default.should.be.eql("hc0");
 			require("./ch").should.be.eql("ch0");
 			break;
-		case 1:
+		case "1":
 			require("./hc").should.be.eql("hc1");
 			require("./ch").default.should.be.eql("ch1");
 			break;

yarn.lock

@@ -3994,7 +3994,7 @@ void-elements@~2.0.1:
   version "2.0.1"
   resolved "https://registry.yarnpkg.com/void-elements/-/void-elements-2.0.1.tgz#c066afb582bb1cb4128d60ea92392e94d5e9dbec"
 
-watchpack@^1.2.0:
+watchpack@^1.3.1:
   version "1.3.1"
   resolved "https://registry.yarnpkg.com/watchpack/-/watchpack-1.3.1.tgz#7d8693907b28ce6013e7f3610aa2a1acf07dad87"
   dependencies: