create less watchers when no options are passed

fix type bug improve test case
webpack · Aug 6, 2020 · 732f8b7 · 732f8b7
1 parent 7c197b4
commit 732f8b7
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 26 deletions.
diff --git a/lib/watchEventSource.js b/lib/watchEventSource.js
@@ -254,7 +254,7 @@ const execute = () => {
 			} else {
 				for (const filePath of filePaths) {
 					const w = createDirectWatcher(filePath);
-					for (const watcher of entry) {
+					for (const watcher of entry.keys()) {
 						const old = underlyingWatcher.get(watcher);
 						if (old === w) continue;
 						w.add(watcher);

diff --git a/lib/watchpack.js b/lib/watchpack.js
@@ -13,6 +13,7 @@ const watchEventSource = require("./watchEventSource");
 let EXISTANCE_ONLY_TIME_ENTRY; // lazy required
 
 const EMPTY_ARRAY = [];
+const EMPTY_OPTIONS = {};
 
 function addWatchersToSet(watchers, set) {
 	for (const w of watchers) {
@@ -64,11 +65,12 @@ const cachedNormalizeOptions = options => {
 class Watchpack extends EventEmitter {
 	constructor(options) {
 		super();
-		if (!options) options = {};
-		if (typeof options.aggregateTimeout !== "number") {
-			options.aggregateTimeout = 200;
-		}
+		if (!options) options = EMPTY_OPTIONS;
 		this.options = options;
+		this.aggregateTimeout =
+			typeof options.aggregateTimeout === "number"
+				? options.aggregateTimeout
+				: 200;
 		this.watcherOptions = cachedNormalizeOptions(options);
 		this.watcherManager = getWatcherManager(this.watcherOptions);
 		this.fileWatchers = new Map();
@@ -77,7 +79,7 @@ class Watchpack extends EventEmitter {
 		this.paused = false;
 		this.aggregatedChanges = new Set();
 		this.aggregatedRemovals = new Set();
-		this.aggregateTimeout = 0;
+		this.aggregateTimer = undefined;
 		this._onTimeout = this._onTimeout.bind(this);
 	}
 
@@ -249,7 +251,7 @@ class Watchpack extends EventEmitter {
 
 	close() {
 		this.paused = true;
-		if (this.aggregateTimeout) clearTimeout(this.aggregateTimeout);
+		if (this.aggregateTimer) clearTimeout(this.aggregateTimer);
 		for (const w of this.fileWatchers.values()) w.close();
 		for (const w of this.directoryWatchers.values()) w.close();
 		this.fileWatchers.clear();
@@ -258,7 +260,7 @@ class Watchpack extends EventEmitter {
 
 	pause() {
 		this.paused = true;
-		if (this.aggregateTimeout) clearTimeout(this.aggregateTimeout);
+		if (this.aggregateTimer) clearTimeout(this.aggregateTimer);
 	}
 
 	getTimes() {
@@ -344,30 +346,24 @@ class Watchpack extends EventEmitter {
 		file = file || item;
 		if (this.paused) return;
 		this.emit("change", file, mtime, type);
-		if (this.aggregateTimeout) clearTimeout(this.aggregateTimeout);
+		if (this.aggregateTimer) clearTimeout(this.aggregateTimer);
 		this.aggregatedRemovals.delete(item);
 		this.aggregatedChanges.add(item);
-		this.aggregateTimeout = setTimeout(
-			this._onTimeout,
-			this.options.aggregateTimeout
-		);
+		this.aggregateTimer = setTimeout(this._onTimeout, this.aggregateTimeout);
 	}
 
 	_onRemove(item, file, type) {
 		file = file || item;
 		if (this.paused) return;
 		this.emit("remove", file, type);
-		if (this.aggregateTimeout) clearTimeout(this.aggregateTimeout);
+		if (this.aggregateTimer) clearTimeout(this.aggregateTimer);
 		this.aggregatedChanges.delete(item);
 		this.aggregatedRemovals.add(item);
-		this.aggregateTimeout = setTimeout(
-			this._onTimeout,
-			this.options.aggregateTimeout
-		);
+		this.aggregateTimer = setTimeout(this._onTimeout, this.aggregateTimeout);
 	}
 
 	_onTimeout() {
-		this.aggregateTimeout = 0;
+		this.aggregateTimer = undefined;
 		const changes = this.aggregatedChanges;
 		const removals = this.aggregatedRemovals;
 		this.aggregatedChanges = new Set();

diff --git a/test/Watchpack.js b/test/Watchpack.js
@@ -590,12 +590,11 @@ describe("Watchpack", function() {
 	});
 
 	it("should detect a single change to future timestamps", function(done) {
-		var w = new Watchpack({
-			aggregateTimeout: 1000
-		});
-		var w2 = new Watchpack({
+		const options = {
 			aggregateTimeout: 1000
-		});
+		};
+		var w = new Watchpack(options);
+		var w2 = new Watchpack(options);
 		w.on("change", function() {
 			throw new Error("should not report change event");
 		});
@@ -604,15 +603,41 @@ describe("Watchpack", function() {
 		});
 		testHelper.file("a");
 		testHelper.tick(400, function() {
-			w2.watch([path.join(fixtures, "a")], []);
+			w2.watch([path.join(fixtures, "a")], [], Date.now());
 			testHelper.tick(1000, function() {
 				// wait for initial scan
 				testHelper.mtime("a", Date.now() + 1000000);
 				testHelper.tick(400, function() {
-					w.watch([path.join(fixtures, "a")], []);
+					w.watch([path.join(fixtures, "a")], [], Date.now());
+					testHelper.tick(1000, function() {
+						w2.close();
+						w.close();
+						done();
+					});
+				});
+			});
+		});
+	});
+
+	it("should create different watchers for different options", function(done) {
+		var w = new Watchpack({
+			aggregateTimeout: 1000
+		});
+		var w2 = new Watchpack({
+			aggregateTimeout: 1000
+		});
+		testHelper.file("a");
+		testHelper.tick(400, function() {
+			w.watch([path.join(fixtures, "a")], [], Date.now());
+			w2.watch([path.join(fixtures, "a")], [], Date.now());
+			testHelper.tick(1000, function() {
+				testHelper.file("a");
+				testHelper.tick(400, function() {
+					testHelper.file("a");
 					testHelper.tick(1000, function() {
 						w2.close();
 						w.close();
+						console.log("test done");
 						done();
 					});
 				});