From 65d5f5b5d4f157dacf122a4a576c5692b5654905 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Tue, 23 Aug 2022 20:50:21 +0300 Subject: [PATCH] cli: add `--watch` --- lib/internal/assert/assertion_error.js | 49 +++---- lib/internal/main/watch_mode.js | 128 ++++++++++++++++++ lib/internal/modules/cjs/loader.js | 10 ++ lib/internal/modules/esm/loader.js | 4 + lib/internal/util/colors.js | 18 +++ lib/internal/watch_mode/files_watcher.js | 91 +++++++++++++ src/inspector_agent.cc | 4 + src/node.cc | 4 + src/node_options.cc | 34 ++++- src/node_options.h | 6 + test/fixtures/watch-mode/failing.js | 1 + test/fixtures/watch-mode/file.mjs | 3 + test/fixtures/watch-mode/gracefulExit.js | 12 ++ test/fixtures/watch-mode/logs.mjs | 1 + test/fixtures/watch-mode/server.js | 14 ++ ...rocess-env-allowed-flags-are-documented.js | 1 + 16 files changed, 347 insertions(+), 33 deletions(-) create mode 100644 lib/internal/main/watch_mode.js create mode 100644 lib/internal/util/colors.js create mode 100644 lib/internal/watch_mode/files_watcher.js create mode 100644 test/fixtures/watch-mode/failing.js create mode 100644 test/fixtures/watch-mode/file.mjs create mode 100644 test/fixtures/watch-mode/gracefulExit.js create mode 100644 test/fixtures/watch-mode/logs.mjs create mode 100644 test/fixtures/watch-mode/server.js diff --git a/lib/internal/assert/assertion_error.js b/lib/internal/assert/assertion_error.js index 3deb8185229d7f..17e261bd4cb1ef 100644 --- a/lib/internal/assert/assertion_error.js +++ b/lib/internal/assert/assertion_error.js @@ -21,15 +21,12 @@ const { inspect } = require('internal/util/inspect'); const { removeColors, } = require('internal/util'); +const colors = require('internal/util/colors'); const { validateObject, } = require('internal/validators'); const { isErrorStackTraceLimitWritable } = require('internal/errors'); -let blue = ''; -let green = ''; -let red = ''; -let white = ''; const kReadableOperator = { deepStrictEqual: 'Expected values to be strictly deep-equal:', @@ -169,7 +166,7 @@ function createErrDiff(actual, expected, operator) { // Only remove lines in case it makes sense to collapse those. // TODO: Accept env to always show the full error. if (actualLines.length > 50) { - actualLines[46] = `${blue}...${white}`; + actualLines[46] = `${colors.blue}...${colors.white}`; while (actualLines.length > 47) { ArrayPrototypePop(actualLines); } @@ -182,7 +179,7 @@ function createErrDiff(actual, expected, operator) { // There were at least five identical lines at the end. Mark a couple of // skipped. if (i >= 5) { - end = `\n${blue}...${white}${end}`; + end = `\n${colors.blue}...${colors.white}${end}`; skipped = true; } if (other !== '') { @@ -193,15 +190,15 @@ function createErrDiff(actual, expected, operator) { let printedLines = 0; let identical = 0; const msg = kReadableOperator[operator] + - `\n${green}+ actual${white} ${red}- expected${white}`; - const skippedMsg = ` ${blue}...${white} Lines skipped`; + `\n${colors.green}+ actual${colors.white} ${colors.red}- expected${colors.white}`; + const skippedMsg = ` ${colors.blue}...${colors.white} Lines skipped`; let lines = actualLines; - let plusMinus = `${green}+${white}`; + let plusMinus = `${colors.green}+${colors.white}`; let maxLength = expectedLines.length; if (actualLines.length < maxLines) { lines = expectedLines; - plusMinus = `${red}-${white}`; + plusMinus = `${colors.red}-${colors.white}`; maxLength = actualLines.length; } @@ -216,7 +213,7 @@ function createErrDiff(actual, expected, operator) { res += `\n ${lines[i - 3]}`; printedLines++; } else { - res += `\n${blue}...${white}`; + res += `\n${colors.blue}...${colors.white}`; skipped = true; } } @@ -272,7 +269,7 @@ function createErrDiff(actual, expected, operator) { res += `\n ${actualLines[i - 3]}`; printedLines++; } else { - res += `\n${blue}...${white}`; + res += `\n${colors.blue}...${colors.white}`; skipped = true; } } @@ -286,8 +283,8 @@ function createErrDiff(actual, expected, operator) { identical = 0; // Add the actual line to the result and cache the expected diverging // line so consecutive diverging lines show up as +++--- and not +-+-+-. - res += `\n${green}+${white} ${actualLine}`; - other += `\n${red}-${white} ${expectedLine}`; + res += `\n${colors.green}+${colors.white} ${actualLine}`; + other += `\n${colors.red}-${colors.white} ${expectedLine}`; printedLines += 2; // Lines are identical } else { @@ -306,8 +303,8 @@ function createErrDiff(actual, expected, operator) { } // Inspected object to big (Show ~50 rows max) if (printedLines > 50 && i < maxLines - 2) { - return `${msg}${skippedMsg}\n${res}\n${blue}...${white}${other}\n` + - `${blue}...${white}`; + return `${msg}${skippedMsg}\n${res}\n${colors.blue}...${colors.white}${other}\n` + + `${colors.blue}...${colors.white}`; } } @@ -347,21 +344,9 @@ class AssertionError extends Error { if (message != null) { super(String(message)); } else { - if (process.stderr.isTTY) { - // Reset on each call to make sure we handle dynamically set environment - // variables correct. - if (process.stderr.hasColors()) { - blue = '\u001b[34m'; - green = '\u001b[32m'; - white = '\u001b[39m'; - red = '\u001b[31m'; - } else { - blue = ''; - green = ''; - white = ''; - red = ''; - } - } + // Reset colors on each call to make sure we handle dynamically set environment + // variables correct. + colors.refresh(); // Prevent the error stack from being visible by duplicating the error // in a very close way to the original in case both sides are actually // instances of Error. @@ -393,7 +378,7 @@ class AssertionError extends Error { // Only remove lines in case it makes sense to collapse those. // TODO: Accept env to always show the full error. if (res.length > 50) { - res[46] = `${blue}...${white}`; + res[46] = `${colors.blue}...${colors.white}`; while (res.length > 47) { ArrayPrototypePop(res); } diff --git a/lib/internal/main/watch_mode.js b/lib/internal/main/watch_mode.js new file mode 100644 index 00000000000000..879cb5bfd85b3a --- /dev/null +++ b/lib/internal/main/watch_mode.js @@ -0,0 +1,128 @@ +'use strict'; +const { + ArrayPrototypeJoin, + ArrayPrototypeMap, + ArrayPrototypePush, + ArrayPrototypePushApply, + ArrayPrototypeReduce, + ArrayPrototypeSlice, +} = primordials; +const { + prepareMainThreadExecution, + markBootstrapComplete +} = require('internal/process/pre_execution'); +const { getOptionValue } = require('internal/options'); +const { emitExperimentalWarning } = require('internal/util'); +const { FilesWatcher } = require('internal/watch_mode/files_watcher'); +const { green, blue, red, white } = require('internal/util/colors'); + +const { spawn } = require('child_process'); +const { inspect } = require('util'); +const { setTimeout, clearTimeout } = require('timers'); +const { resolve } = require('path'); +const { once, on } = require('events'); + + +prepareMainThreadExecution(false, false); +markBootstrapComplete(); + +// TODO(MoLow): Make kill signal configurable +const kKillSignal = 'SIGTERM'; +const kShouldFilterModules = getOptionValue('--watch-path').length === 0; +const kWatchedPaths = getOptionValue('--watch-path').length ? + ArrayPrototypeMap(getOptionValue('--watch-path'), (path) => resolve(path)) : + [process.cwd()]; +const kCommand = ArrayPrototypeSlice(process.argv, 1); +const kCommandStr = inspect(ArrayPrototypeJoin(kCommand, ' ')); +const args = ArrayPrototypeReduce(process.execArgv, (acc, flag, i, arr) => { + if (arr[i] !== '--watch-path' && arr[i - 1] !== '--watch-path' && arr[i] !== '--watch') { + acc.push(arr[i]); + } + return acc; +}, []); +ArrayPrototypePush(args, '--watch-report-ipc'); +ArrayPrototypePushApply(args, kCommand); + +const watcher = new FilesWatcher({ throttle: 500, mode: kShouldFilterModules ? 'filter' : 'all' }); +kWatchedPaths.forEach((p) => watcher.watchPath(p)); +let graceTimer; +let child; +let exited; + +function start() { + // Spawning in detached mode so node can control when signals are forwarded + exited = false; + const stdio = kShouldFilterModules ? ['inherit', 'inherit', 'inherit', 'ipc'] : undefined; + child = spawn(process.execPath, args, { stdio, detached: true }); + watcher.watchChildProcessModules(child); + child.once('exit', (code) => { + exited = true; + if (code === 0) { + process.stdout.write(`${blue}Completed running ${kCommandStr}${white}\n`); + } else { + process.stdout.write(`${red}Failed running ${kCommandStr}${white}\n`); + } + }); +} + +async function killAndWait(signal = kKillSignal) { + child?.removeAllListeners(); + if (!child || child.killed || exited) { + return; + } + const onExit = once(child, 'exit'); + child.kill(signal); + const { 0: exitCode } = await onExit; + return exitCode; +} + +function reportGracefulTermination() { + let reported = false; + clearTimeout(graceTimer); + graceTimer = setTimeout(() => { + reported = true; + process.stdout.write(`${blue}Waiting for graceful termination${white}\n`); + }, 1000).unref(); + return () => { + clearTimeout(graceTimer); + if (reported) { + process.stdout.write(`${green}gracefully terminated${white}\n`); + } + }; +} + +async function stop() { + watcher.clearFileFilters(); + const clearGraceReport = reportGracefulTermination(); + await killAndWait(); + clearGraceReport(); +} + +async function restart() { + // process.stdout.write('\u001Bc'); + process.stdout.write(`${green}Restarting ${kCommandStr}${white}\n`); + await stop(); + start(); +} + +(async () => { + emitExperimentalWarning('Watch mode'); + start(); + + // eslint-disable-next-line no-unused-vars + for await (const _ of on(watcher, 'changed')) { + await restart(); + } +})(); + +// Exiting gracefully to avoid stdout/stderr getting written after +// parent process is killed. +// this is fairly safe since user code cannot run in this process +function signalHandler(signal) { + return async () => { + watcher.clear(); + process.exit(await killAndWait(signal)); + }; +} +process.on('SIGTERM', signalHandler('SIGTERM')); +process.on('SIGINT', signalHandler('SIGINT')); diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 796c527b011139..bc2fa4eb427d73 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -100,6 +100,7 @@ const { const { getOptionValue } = require('internal/options'); const preserveSymlinks = getOptionValue('--preserve-symlinks'); const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); +const shouldReportRequiredModules = getOptionValue('--watch-report-ipc'); // Do not eagerly grab .manifest, it may be in TDZ const policy = getOptionValue('--experimental-policy') ? require('internal/process/policy') : @@ -168,6 +169,12 @@ function updateChildren(parent, child, scan) { ArrayPrototypePush(children, child); } +function reportModuleToWatchMode(filename) { + if (shouldReportRequiredModules && process.send) { + process.send({ 'watch:require': filename }); + } +} + const moduleParentCache = new SafeWeakMap(); function Module(id = '', parent) { this.id = id; @@ -776,6 +783,7 @@ Module._load = function(request, parent, isMain) { // cache key names. relResolveCacheIdentifier = `${parent.path}\x00${request}`; const filename = relativeResolveCache[relResolveCacheIdentifier]; + reportModuleToWatchMode(filename); if (filename !== undefined) { const cachedModule = Module._cache[filename]; if (cachedModule !== undefined) { @@ -828,6 +836,8 @@ Module._load = function(request, parent, isMain) { module.id = '.'; } + reportModuleToWatchMode(filename); + Module._cache[filename] = module; if (parent !== undefined) { relativeResolveCache[relResolveCacheIdentifier] = filename; diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 79e9b2446c6bbf..ca2abd320ab3e9 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -475,6 +475,10 @@ class ESMLoader { getOptionValue('--inspect-brk') ); + if (getOptionValue('--watch-report-ipc') && process.send) { + process.send({ 'watch:import': url }); + } + const job = new ModuleJob( this, url, diff --git a/lib/internal/util/colors.js b/lib/internal/util/colors.js new file mode 100644 index 00000000000000..1f4b4dd1c9c6cc --- /dev/null +++ b/lib/internal/util/colors.js @@ -0,0 +1,18 @@ +'use strict'; + +module.exports = { + blue: '', + green: '', + white: '', + red: '', + refresh() { + if (process.stderr.isTTY && process.stderr.hasColors()) { + module.exports.blue = '\u001b[34m'; + module.exports.green = '\u001b[32m'; + module.exports.white = '\u001b[39m'; + module.exports.red = '\u001b[31m'; + } + } +}; + +module.exports.refresh(); diff --git a/lib/internal/watch_mode/files_watcher.js b/lib/internal/watch_mode/files_watcher.js new file mode 100644 index 00000000000000..f48955e574d222 --- /dev/null +++ b/lib/internal/watch_mode/files_watcher.js @@ -0,0 +1,91 @@ +'use strict'; + +const { + SafeMap, + SafeSet, +} = primordials; + +const { validateNumber, validateOneOf } = require('internal/validators'); +const { kEmptyObject } = require('internal/util'); +const { TIMEOUT_MAX } = require('internal/timers'); + +const EventEmitter = require('events'); +const { watch } = require('fs'); +const { fileURLToPath } = require('url'); +const { resolve } = require('path'); +const { setTimeout } = require('timers'); + +class FilesWatcher extends EventEmitter { + #watchers = new SafeMap(); + #filteredFiles = new SafeSet(); + #throttling = new SafeSet(); + #throttle; + #mode; + + constructor(options = kEmptyObject) { + super(); + let { throttle, mode } = options; + throttle ??= 500; + mode ??= 'filter'; + + validateNumber(throttle, 'options.throttle', 0, TIMEOUT_MAX); + validateOneOf(mode, 'options.mode', ['filter', 'all']); + this.#throttle = throttle; + this.#mode = mode; + } + + watchPath(path) { + if (this.#watchers.has(path)) { + return; + } + const watcher = watch(path, { recursive: true }); + watcher.on('change', (eventType, fileName) => this.#onChange(resolve(fileName))); + this.#watchers.set(path, watcher); + } + filterFile(file) { + // TODO(MoLow): Add a new watcher in case file is not a child of one of the existing watchers + this.#filteredFiles.add(file); + } + watchChildProcessModules(child) { + if (this.#mode !== 'filter') { + return; + } + child.on('message', (message) => { + if (message['watch:require']) { + this.filterFile(message['watch:require']); + } + if (message['watch:import']) { + try { + this.filterFile(fileURLToPath(message['watch:import'])); + } catch { + // Failed watching file. ignore + } + } + }); + } + clearFileFilters() { + this.#filteredFiles.clear(); + } + clear() { + for (const watcher of this.#watchers.values()) { + watcher.removeAllListeners(); + watcher.close(); + } + this.#filteredFiles.clear(); + this.#watchers.clear(); + } + + #onChange(trigger) { + if (this.#throttling.has(trigger)) { + return; + } + if (this.#mode === 'filter' && !this.#filteredFiles.has(trigger)) { + return; + } + this.#throttling.add(trigger); + this.emit('changed'); + setTimeout(() => this.#throttling.delete(trigger), this.#throttle).unref(); + } +} + +module.exports = { FilesWatcher }; diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index 34bb11e7d7122c..64e0f01b2acead 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -676,6 +676,10 @@ bool Agent::Start(const std::string& path, const DebugOptions& options, std::shared_ptr> host_port, bool is_main) { + if (!options.allow_attaching_debugger) { + return false; + } + path_ = path; debug_options_ = options; CHECK_NOT_NULL(host_port); diff --git a/src/node.cc b/src/node.cc index 2891c18bb9aa9a..454fb1f04fb4ab 100644 --- a/src/node.cc +++ b/src/node.cc @@ -490,6 +490,10 @@ MaybeLocal StartExecution(Environment* env, StartExecutionCallback cb) { return StartExecution(env, "internal/main/test_runner"); } + if (env->options()->watch_mode) { + return StartExecution(env, "internal/main/watch_mode"); + } + if (!first_argv.empty() && first_argv != "-") { return StartExecution(env, "internal/main/run_main_module"); } diff --git a/src/node_options.cc b/src/node_options.cc index 0869cbb974be86..f4168e5ef633d5 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -156,11 +156,32 @@ void EnvironmentOptions::CheckOptions(std::vector* errors) { errors->push_back("either --test or --interactive can be used, not both"); } + if (watch_mode) { + // TODO(MoLow): Support (incremental?) watch mode within test runner + errors->push_back("either --test or --watch can be used, not both"); + } + + debug_options_.allow_attaching_debugger = false; if (debug_options_.inspector_enabled) { errors->push_back("the inspector cannot be used with --test"); } } + if (watch_mode) { + if (syntax_check_only) { + errors->push_back("either --watch or --check can be used, not both"); + } + + if (has_eval_string) { + errors->push_back("either --watch or --eval can be used, not both"); + } + + if (force_repl) { + errors->push_back("either --watch or --interactive can be used, not both"); + } + debug_options_.allow_attaching_debugger = false; + } + #if HAVE_INSPECTOR if (!cpu_prof) { if (!cpu_prof_name.empty()) { @@ -586,7 +607,18 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { "", /* undocumented, only for debugging */ &EnvironmentOptions::verify_base_objects, kAllowedInEnvironment); - + AddOption("--watch", + "run in watch mode", + &EnvironmentOptions::watch_mode, + kAllowedInEnvironment); + AddOption("--watch-path", + "path to watch", + &EnvironmentOptions::watch_mode_paths, + kAllowedInEnvironment); + Implies("--watch-path", "--watch"); + AddOption("--watch-report-ipc", + "", /* undocumented, used internally */ + &EnvironmentOptions::watch_mode_report_to_parent); AddOption("--check", "syntax check script without executing", &EnvironmentOptions::syntax_check_only); diff --git a/src/node_options.h b/src/node_options.h index ca43192d85a4b4..b064e92a6909f1 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -71,6 +71,8 @@ class DebugOptions : public Options { DebugOptions(DebugOptions&&) = default; DebugOptions& operator=(DebugOptions&&) = default; + + bool allow_attaching_debugger = true; // --inspect bool inspector_enabled = false; // --debug @@ -172,6 +174,10 @@ class EnvironmentOptions : public Options { false; #endif // DEBUG + bool watch_mode = false; + bool watch_mode_report_to_parent = false; + std::vector watch_mode_paths; + bool syntax_check_only = false; bool has_eval_string = false; bool experimental_wasi = false; diff --git a/test/fixtures/watch-mode/failing.js b/test/fixtures/watch-mode/failing.js new file mode 100644 index 00000000000000..dc98d168174e64 --- /dev/null +++ b/test/fixtures/watch-mode/failing.js @@ -0,0 +1 @@ +throw new Error('fails'); \ No newline at end of file diff --git a/test/fixtures/watch-mode/file.mjs b/test/fixtures/watch-mode/file.mjs new file mode 100644 index 00000000000000..488335b204c07e --- /dev/null +++ b/test/fixtures/watch-mode/file.mjs @@ -0,0 +1,3 @@ +import { log } from './logs.mjs'; + +log(123); diff --git a/test/fixtures/watch-mode/gracefulExit.js b/test/fixtures/watch-mode/gracefulExit.js new file mode 100644 index 00000000000000..1295d84fc55b85 --- /dev/null +++ b/test/fixtures/watch-mode/gracefulExit.js @@ -0,0 +1,12 @@ + +setInterval(() => {}, 1000); + +process.on('SIGINT', () => { + setTimeout(() => process.exit(0), 1500); +}); +process.on('SIGTERM', () => { + setTimeout(() => process.exit(0), 1500); +}); +process.on('exit', () => { + console.log('exit'); +}); \ No newline at end of file diff --git a/test/fixtures/watch-mode/logs.mjs b/test/fixtures/watch-mode/logs.mjs new file mode 100644 index 00000000000000..bb6a029def26ad --- /dev/null +++ b/test/fixtures/watch-mode/logs.mjs @@ -0,0 +1 @@ +export const log = (...args) => console.log('CUSTOM LOG', ...args); diff --git a/test/fixtures/watch-mode/server.js b/test/fixtures/watch-mode/server.js new file mode 100644 index 00000000000000..a44d68ea830a78 --- /dev/null +++ b/test/fixtures/watch-mode/server.js @@ -0,0 +1,14 @@ +require('./gracefulExit'); +const http = require('http'); + +const startTime = new Date(); + +const server = http.createServer((req, res) => { + res.writeHead(200, { 'Content-Type': 'application/json' }) + .end(JSON.stringify({ ok: true, uptime: new Date() - startTime, startTime })); +}); + +server.listen(8080, (err) => console.log({ err, startTime }, 'listening on port 8080')); + + +// process.exit(1); \ No newline at end of file diff --git a/test/parallel/test-process-env-allowed-flags-are-documented.js b/test/parallel/test-process-env-allowed-flags-are-documented.js index dc4c704b6f0b88..d2561652831ade 100644 --- a/test/parallel/test-process-env-allowed-flags-are-documented.js +++ b/test/parallel/test-process-env-allowed-flags-are-documented.js @@ -106,6 +106,7 @@ assert(undocumented.delete('--no-node-snapshot')); assert(undocumented.delete('--loader')); assert(undocumented.delete('--verify-base-objects')); assert(undocumented.delete('--no-verify-base-objects')); +assert(undocumented.delete('--watch-report-ipc')); // Remove negated versions of the flags. for (const flag of undocumented) {