From 4c9c76f1d16f0b6c96013f8cfaef6145448a94bc Mon Sep 17 00:00:00 2001 From: Mark Molinaro Date: Wed, 17 Nov 2021 01:01:36 -0800 Subject: [PATCH 1/2] Support splitting of files and directories in getTimeInfoEntries --- lib/DirectoryWatcher.js | 45 +++++++------------ lib/watchpack.js | 33 +++++--------- ...oryWatcher.test.js => DirectoryWatcher.js} | 0 test/Watchpack.js | 38 ++++++++++++++++ 4 files changed, 64 insertions(+), 52 deletions(-) rename test/{DirectoryWatcher.test.js => DirectoryWatcher.js} (100%) diff --git a/lib/DirectoryWatcher.js b/lib/DirectoryWatcher.js index f18dbef..f5861af 100644 --- a/lib/DirectoryWatcher.js +++ b/lib/DirectoryWatcher.js @@ -39,7 +39,6 @@ class Watcher extends EventEmitter { this.directoryWatcher = directoryWatcher; this.path = filePath; this.startTime = startTime && +startTime; - this._cachedTimeInfoEntries = undefined; } checkStartTime(mtime, initial) { @@ -136,8 +135,6 @@ class DirectoryWatcher extends EventEmitter { } setMissing(itemPath, initial, type) { - this._cachedTimeInfoEntries = undefined; - if (this.initialScan) { this.initialScanRemoved.add(itemPath); } @@ -206,7 +203,6 @@ class DirectoryWatcher extends EventEmitter { accuracy, timestamp: mtime }); - this._cachedTimeInfoEntries = undefined; if (!old) { const key = withoutCase(filePath); @@ -249,7 +245,6 @@ class DirectoryWatcher extends EventEmitter { if (!old) { const now = Date.now(); - this._cachedTimeInfoEntries = undefined; if (this.nestedWatching) { this.createNestedWatcher(directoryPath); } else { @@ -280,7 +275,6 @@ class DirectoryWatcher extends EventEmitter { createNestedWatcher(directoryPath) { const watcher = this.watcherManager.watchDirectory(directoryPath, 1); watcher.on("change", (filePath, mtime, type, initial) => { - this._cachedTimeInfoEntries = undefined; this.forEachWatcher(this.path, w => { if (!initial || w.checkStartTime(mtime, initial)) { w.emit("change", filePath, mtime, type, initial); @@ -293,7 +287,6 @@ class DirectoryWatcher extends EventEmitter { setNestedWatching(flag) { if (this.nestedWatching !== !!flag) { this.nestedWatching = !!flag; - this._cachedTimeInfoEntries = undefined; if (this.nestedWatching) { for (const directory of this.directories.keys()) { this.createNestedWatcher(directory); @@ -431,7 +424,6 @@ class DirectoryWatcher extends EventEmitter { } } this.lastWatchEvent = Date.now(); - this._cachedTimeInfoEntries = undefined; if (!stats) { this.setMissing(filePath, false, eventType); } else if (stats.isDirectory()) { @@ -714,48 +706,41 @@ class DirectoryWatcher extends EventEmitter { return obj; } - getTimeInfoEntries() { - if (this._cachedTimeInfoEntries !== undefined) - return this._cachedTimeInfoEntries; - const map = new Map(); - let safeTime = this.lastWatchEvent; + getTimeInfoEntries(fileTimestamps, directoryTimestamps, safeTime) { + safeTime.value = Math.max(safeTime.value, this.lastWatchEvent); for (const [file, entry] of this.files) { fixupEntryAccuracy(entry); - safeTime = Math.max(safeTime, entry.safeTime); - map.set(file, entry); + safeTime.value = Math.max(safeTime.value, entry.safeTime); + fileTimestamps.set(file, entry); } if (this.nestedWatching) { for (const w of this.directories.values()) { - const timeInfoEntries = w.directoryWatcher.getTimeInfoEntries(); - for (const [file, entry] of timeInfoEntries) { - if (entry) { - safeTime = Math.max(safeTime, entry.safeTime); - } - map.set(file, entry); - } + w.directoryWatcher.getTimeInfoEntries( + fileTimestamps, + directoryTimestamps, + safeTime + ); } - map.set(this.path, { - safeTime + directoryTimestamps.set(this.path, { + safeTime: safeTime.value }); } else { for (const dir of this.directories.keys()) { // No additional info about this directory - map.set(dir, EXISTANCE_ONLY_TIME_ENTRY); + directoryTimestamps.set(dir, EXISTANCE_ONLY_TIME_ENTRY); } - map.set(this.path, EXISTANCE_ONLY_TIME_ENTRY); + directoryTimestamps.set(this.path, EXISTANCE_ONLY_TIME_ENTRY); } if (!this.initialScan) { for (const watchers of this.watchers.values()) { for (const watcher of watchers) { const path = watcher.path; - if (!map.has(path)) { - map.set(path, null); + if (!directoryTimestamps.has(path)) { + directoryTimestamps.set(path, null); } } } - this._cachedTimeInfoEntries = map; } - return map; } close() { diff --git a/lib/watchpack.js b/lib/watchpack.js index 9f1dca1..bc502fa 100644 --- a/lib/watchpack.js +++ b/lib/watchpack.js @@ -275,29 +275,18 @@ class Watchpack extends EventEmitter { return obj; } - getTimeInfoEntries() { - if (EXISTANCE_ONLY_TIME_ENTRY === undefined) { - EXISTANCE_ONLY_TIME_ENTRY = require("./DirectoryWatcher") - .EXISTANCE_ONLY_TIME_ENTRY; - } - const directoryWatchers = new Set(); - addWatchersToSet(this.fileWatchers.values(), directoryWatchers); - addWatchersToSet(this.directoryWatchers.values(), directoryWatchers); + getTimeInfoEntries(fileTimestamps, directoryTimestamps) { + const allWatchers = new Set(); + addWatchersToSet(this.fileWatchers.values(), allWatchers); + addWatchersToSet(this.directoryWatchers.values(), allWatchers); const map = new Map(); - for (const w of directoryWatchers) { - const times = w.getTimeInfoEntries(); - for (const [path, entry] of times) { - if (map.has(path)) { - if (entry === EXISTANCE_ONLY_TIME_ENTRY) continue; - const value = map.get(path); - if (value === entry) continue; - if (value !== EXISTANCE_ONLY_TIME_ENTRY) { - map.set(path, Object.assign({}, value, entry)); - continue; - } - } - map.set(path, entry); - } + // if timestamp maps are passed in, populate them, otherwise return a new map with both file and directory timestamps. + if (!fileTimestamps && !directoryTimestamps) { + fileTimestamps = directoryTimestamps = map; + } + const safeTime = { value: 0 }; + for (const w of allWatchers) { + w.getTimeInfoEntries(fileTimestamps, directoryTimestamps, safeTime); } return map; } diff --git a/test/DirectoryWatcher.test.js b/test/DirectoryWatcher.js similarity index 100% rename from test/DirectoryWatcher.test.js rename to test/DirectoryWatcher.js diff --git a/test/Watchpack.js b/test/Watchpack.js index ade8021..48f5438 100644 --- a/test/Watchpack.js +++ b/test/Watchpack.js @@ -524,6 +524,44 @@ describe("Watchpack", function() { }); }); + it("should watch file in a sub directory (passed in maps)", function(done) { + var w = new Watchpack({ + aggregateTimeout: 1000 + }); + var changeEvents = []; + w.on("change", function(file) { + if (changeEvents[changeEvents.length - 1] === file) return; + changeEvents.push(file); + }); + w.on("aggregated", function(changes) { + Array.from(changes).should.be.eql([path.join(fixtures, "dir")]); + changeEvents.should.be.eql([path.join(fixtures, "dir", "sub", "a")]); + const files = new Map(); + const directories = new Map(); + const times = w.getTimeInfoEntries(); + w.getTimeInfoEntries(files, directories); + // aggregated results should be the same as seperated maps + Array.from(times).sort().should.be.eql([...Array.from(files), ...Array.from(directories)].sort()) + const dir = directories.get(path.join(fixtures, "dir")); + const sub = directories.get(path.join(fixtures, "dir", "sub")); + const a = files.get(path.join(fixtures, "dir", "sub", "a")); + dir.should.be.type("object"); + dir.should.have.property("safeTime"); + sub.safeTime.should.be.aboveOrEqual(a.safeTime); + dir.safeTime.should.be.aboveOrEqual(sub.safeTime); + w.close(); + done(); + }); + testHelper.dir("dir"); + testHelper.dir(path.join("dir", "sub")); + testHelper.tick(function() { + w.watch([], [path.join(fixtures, "dir")]); + testHelper.tick(function() { + testHelper.file(path.join("dir", "sub", "a")); + }); + }); + }); + it("should watch 2 files in a not-existing directory", function(done) { var w = new Watchpack({ aggregateTimeout: 1000 From a3b2b82ec7df1e86119653cea113a5a2bb78030c Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Wed, 24 Nov 2021 16:33:59 +0100 Subject: [PATCH 2/2] provide additional method instead of changing existing one avoid deeply collecting watchers since they are always collected during time collection add existence file time info for directories fix safeTime collection --- README.md | 11 ++++++++--- lib/DirectoryWatcher.js | 22 ++++++++++++++-------- lib/watchpack.js | 21 +++++++++------------ test/Watchpack.js | 17 +++++++++++++---- 4 files changed, 44 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index 64a7686..ab785b3 100644 --- a/README.md +++ b/README.md @@ -112,14 +112,19 @@ const { changes, removals } = wp.getAggregated(); // when futher changes happen // Can also be used when paused. -// Watchpack.prototype.getTimeInfoEntries() -var fileTimes = wp.getTimeInfoEntries(); -// returns a Map with all known time info objects for files and directories +// Watchpack.prototype.collectTimeInfoEntries(fileInfoEntries: Map, directoryInfoEntries: Map) +wp.collectTimeInfoEntries(fileInfoEntries, directoryInfoEntries); +// collects time info objects for all known files and directories // this include info from files not directly watched // key: absolute path, value: object with { safeTime, timestamp } // safeTime: a point in time at which it is safe to say all changes happened before that // timestamp: only for files, the mtime timestamp of the file +// Watchpack.prototype.getTimeInfoEntries() +var fileTimes = wp.getTimeInfoEntries(); +// returns a Map with all known time info objects for files and directories +// similar to collectTimeInfoEntries but returns a single map with all entries + // (deprecated) // Watchpack.prototype.getTimes() var fileTimes = wp.getTimes(); diff --git a/lib/DirectoryWatcher.js b/lib/DirectoryWatcher.js index f5861af..0ba6dd9 100644 --- a/lib/DirectoryWatcher.js +++ b/lib/DirectoryWatcher.js @@ -706,29 +706,34 @@ class DirectoryWatcher extends EventEmitter { return obj; } - getTimeInfoEntries(fileTimestamps, directoryTimestamps, safeTime) { - safeTime.value = Math.max(safeTime.value, this.lastWatchEvent); + collectTimeInfoEntries(fileTimestamps, directoryTimestamps) { + let safeTime = this.lastWatchEvent; for (const [file, entry] of this.files) { fixupEntryAccuracy(entry); - safeTime.value = Math.max(safeTime.value, entry.safeTime); + safeTime = Math.max(safeTime, entry.safeTime); fileTimestamps.set(file, entry); } if (this.nestedWatching) { for (const w of this.directories.values()) { - w.directoryWatcher.getTimeInfoEntries( - fileTimestamps, - directoryTimestamps, - safeTime + safeTime = Math.max( + safeTime, + w.directoryWatcher.collectTimeInfoEntries( + fileTimestamps, + directoryTimestamps, + safeTime + ) ); } + fileTimestamps.set(this.path, EXISTANCE_ONLY_TIME_ENTRY); directoryTimestamps.set(this.path, { - safeTime: safeTime.value + safeTime }); } else { for (const dir of this.directories.keys()) { // No additional info about this directory directoryTimestamps.set(dir, EXISTANCE_ONLY_TIME_ENTRY); } + fileTimestamps.set(this.path, EXISTANCE_ONLY_TIME_ENTRY); directoryTimestamps.set(this.path, EXISTANCE_ONLY_TIME_ENTRY); } if (!this.initialScan) { @@ -741,6 +746,7 @@ class DirectoryWatcher extends EventEmitter { } } } + return safeTime; } close() { diff --git a/lib/watchpack.js b/lib/watchpack.js index bc502fa..e43be5f 100644 --- a/lib/watchpack.js +++ b/lib/watchpack.js @@ -10,16 +10,13 @@ const EventEmitter = require("events").EventEmitter; const globToRegExp = require("glob-to-regexp"); 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) { - if (w !== true && !set.has(w.directoryWatcher)) { + if (!set.has(w.directoryWatcher)) { set.add(w.directoryWatcher); - addWatchersToSet(w.directoryWatcher.directories.values(), set); } } } @@ -275,20 +272,20 @@ class Watchpack extends EventEmitter { return obj; } - getTimeInfoEntries(fileTimestamps, directoryTimestamps) { + getTimeInfoEntries() { + const map = new Map(); + this.collectTimeInfoEntries(map, map); + return map; + } + + collectTimeInfoEntries(fileTimestamps, directoryTimestamps) { const allWatchers = new Set(); addWatchersToSet(this.fileWatchers.values(), allWatchers); addWatchersToSet(this.directoryWatchers.values(), allWatchers); - const map = new Map(); - // if timestamp maps are passed in, populate them, otherwise return a new map with both file and directory timestamps. - if (!fileTimestamps && !directoryTimestamps) { - fileTimestamps = directoryTimestamps = map; - } const safeTime = { value: 0 }; for (const w of allWatchers) { - w.getTimeInfoEntries(fileTimestamps, directoryTimestamps, safeTime); + w.collectTimeInfoEntries(fileTimestamps, directoryTimestamps, safeTime); } - return map; } getAggregated() { diff --git a/test/Watchpack.js b/test/Watchpack.js index 48f5438..69eb82a 100644 --- a/test/Watchpack.js +++ b/test/Watchpack.js @@ -538,15 +538,23 @@ describe("Watchpack", function() { changeEvents.should.be.eql([path.join(fixtures, "dir", "sub", "a")]); const files = new Map(); const directories = new Map(); - const times = w.getTimeInfoEntries(); - w.getTimeInfoEntries(files, directories); - // aggregated results should be the same as seperated maps - Array.from(times).sort().should.be.eql([...Array.from(files), ...Array.from(directories)].sort()) + w.collectTimeInfoEntries(files, directories); const dir = directories.get(path.join(fixtures, "dir")); + const dirAsFile = files.get(path.join(fixtures, "dir")); const sub = directories.get(path.join(fixtures, "dir", "sub")); + const subAsFile = files.get(path.join(fixtures, "dir", "sub")); const a = files.get(path.join(fixtures, "dir", "sub", "a")); dir.should.be.type("object"); dir.should.have.property("safeTime"); + dirAsFile.should.be.type("object"); + dirAsFile.should.not.have.property("safeTime"); + sub.should.be.type("object"); + sub.should.have.property("safeTime"); + subAsFile.should.be.type("object"); + subAsFile.should.not.have.property("safeTime"); + a.should.be.type("object"); + a.should.have.property("safeTime"); + a.should.have.property("timestamp"); sub.safeTime.should.be.aboveOrEqual(a.safeTime); dir.safeTime.should.be.aboveOrEqual(sub.safeTime); w.close(); @@ -554,6 +562,7 @@ describe("Watchpack", function() { }); testHelper.dir("dir"); testHelper.dir(path.join("dir", "sub")); + testHelper.dir(path.join("dir", "sub2")); testHelper.tick(function() { w.watch([], [path.join(fixtures, "dir")]); testHelper.tick(function() {