From a12996298e60c6d614de8d8538d2d581cef3f01e Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Tue, 25 Jul 2023 23:25:03 +0300 Subject: [PATCH] watch: use debounce instead of throttle PR-URL: https://github.com/nodejs/node/pull/48926 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Chemi Atlow Reviewed-By: Yagiz Nizipli Reviewed-By: Trivikram Kamat --- lib/internal/main/watch_mode.js | 2 +- lib/internal/test_runner/runner.js | 2 +- lib/internal/watch_mode/files_watcher.js | 20 ++++++++++--------- .../test-watch-mode-files_watcher.mjs | 10 +++++----- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/lib/internal/main/watch_mode.js b/lib/internal/main/watch_mode.js index 5d0d29cc2ff9da..8b3133bb8ecc07 100644 --- a/lib/internal/main/watch_mode.js +++ b/lib/internal/main/watch_mode.js @@ -44,7 +44,7 @@ const args = ArrayPrototypeFilter(process.execArgv, (arg, i, arr) => arg !== '--watch' && arg !== '--watch-preserve-output'); ArrayPrototypePushApply(args, kCommand); -const watcher = new FilesWatcher({ throttle: 500, mode: kShouldFilterModules ? 'filter' : 'all' }); +const watcher = new FilesWatcher({ debounce: 500, mode: kShouldFilterModules ? 'filter' : 'all' }); ArrayPrototypeForEach(kWatchedPaths, (p) => watcher.watchPath(p)); let graceTimer; diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 0f2749fd15f0ea..0918b98dc15b88 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -428,7 +428,7 @@ function runTestFile(path, root, inspectPort, filesWatcher, testNamePatterns) { function watchFiles(testFiles, root, inspectPort, signal, testNamePatterns) { const runningProcesses = new SafeMap(); const runningSubtests = new SafeMap(); - const watcher = new FilesWatcher({ throttle: 500, mode: 'filter', signal }); + const watcher = new FilesWatcher({ debounce: 500, mode: 'filter', signal }); const filesWatcher = { __proto__: null, watcher, runningProcesses, runningSubtests }; watcher.on('changed', ({ owners }) => { diff --git a/lib/internal/watch_mode/files_watcher.js b/lib/internal/watch_mode/files_watcher.js index 848c17f4115616..6a1052ab0188e6 100644 --- a/lib/internal/watch_mode/files_watcher.js +++ b/lib/internal/watch_mode/files_watcher.js @@ -24,19 +24,19 @@ const supportsRecursiveWatching = process.platform === 'win32' || class FilesWatcher extends EventEmitter { #watchers = new SafeMap(); #filteredFiles = new SafeSet(); - #throttling = new SafeSet(); + #debouncing = new SafeSet(); #depencencyOwners = new SafeMap(); #ownerDependencies = new SafeMap(); - #throttle; + #debounce; #mode; #signal; - constructor({ throttle = 500, mode = 'filter', signal } = kEmptyObject) { + constructor({ debounce = 500, mode = 'filter', signal } = kEmptyObject) { super(); - validateNumber(throttle, 'options.throttle', 0, TIMEOUT_MAX); + validateNumber(debounce, 'options.debounce', 0, TIMEOUT_MAX); validateOneOf(mode, 'options.mode', ['filter', 'all']); - this.#throttle = throttle; + this.#debounce = debounce; this.#mode = mode; this.#signal = signal; @@ -74,16 +74,18 @@ class FilesWatcher extends EventEmitter { } #onChange(trigger) { - if (this.#throttling.has(trigger)) { + if (this.#debouncing.has(trigger)) { return; } if (this.#mode === 'filter' && !this.#filteredFiles.has(trigger)) { return; } - this.#throttling.add(trigger); + this.#debouncing.add(trigger); const owners = this.#depencencyOwners.get(trigger); - this.emit('changed', { owners }); - setTimeout(() => this.#throttling.delete(trigger), this.#throttle).unref(); + setTimeout(() => { + this.#debouncing.delete(trigger); + this.emit('changed', { owners }); + }, this.#debounce).unref(); } get watchedPaths() { diff --git a/test/parallel/test-watch-mode-files_watcher.mjs b/test/parallel/test-watch-mode-files_watcher.mjs index 19f71a04e40df8..a059eb30dc63d9 100644 --- a/test/parallel/test-watch-mode-files_watcher.mjs +++ b/test/parallel/test-watch-mode-files_watcher.mjs @@ -26,7 +26,7 @@ describe('watch mode file watcher', () => { beforeEach(() => { changesCount = 0; - watcher = new FilesWatcher({ throttle: 100 }); + watcher = new FilesWatcher({ debounce: 100 }); watcher.on('changed', () => changesCount++); }); @@ -51,7 +51,7 @@ describe('watch mode file watcher', () => { assert.strictEqual(changesCount, 1); }); - it('should throttle changes', async () => { + it('should debounce changes', async () => { const file = path.join(tmpdir.path, 'file2'); writeFileSync(file, 'written'); watcher.filterFile(file); @@ -61,7 +61,7 @@ describe('watch mode file watcher', () => { writeFileSync(file, '2'); writeFileSync(file, '3'); writeFileSync(file, '4'); - await setTimeout(200); // throttle * 2 + await setTimeout(200); // debounce * 2 writeFileSync(file, '5'); const changed = once(watcher, 'changed'); writeFileSync(file, 'after'); @@ -86,8 +86,8 @@ describe('watch mode file watcher', () => { await writeAndWaitForChanges(watcher, file); writeFileSync(file, '1'); + assert.strictEqual(changesCount, 1); - await setTimeout(200); // avoid throttling watcher.clearFileFilters(); writeFileSync(file, '2'); // Wait for this long to make sure changes are triggered only once @@ -97,7 +97,7 @@ describe('watch mode file watcher', () => { it('should watch all files in watched path when in "all" mode', { skip: !supportsRecursiveWatching }, async () => { - watcher = new FilesWatcher({ throttle: 100, mode: 'all' }); + watcher = new FilesWatcher({ debounce: 100, mode: 'all' }); watcher.on('changed', () => changesCount++); const file = path.join(tmpdir.path, 'file5');