Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: make run and watch modes friends #53457

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -2596,8 +2596,14 @@ Use `--watch-path` to specify what paths to watch.
This flag cannot be combined with
`--check`, `--eval`, `--interactive`, or the REPL.

If using `--run` and `--watch` Node.js will read the `package.json` scripts
field each restart.
If using `--run` and `--watch` Node.js positional arguments
will throw an error.

```bash
node --watch index.js
node --watch --run start
```

### `--watch-path`
Expand Down
16 changes: 16 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -2359,6 +2359,22 @@ The `package.json` [`"exports"`][] field does not export the requested subpath.
Because exports are encapsulated, private internal modules that are not exported
cannot be imported through the package resolution, unless using an absolute URL.

<a id="ERR_PACKAGE_SCRIPT_MISSING"></a>

### `ERR_PACKAGE_SCRIPT_MISSING`

The `package.json` "scripts" field does not contain the requested script.
In order to use `--run` ensure that the contents are an object containing a
`"scripts"` field that contains the missing script name as a field.

```json
{
"scripts": {
"start": "node server.js"
}
}
```

<a id="ERR_PARSE_ARGS_INVALID_OPTION_VALUE"></a>

### `ERR_PARSE_ARGS_INVALID_OPTION_VALUE`
Expand Down
9 changes: 9 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1633,6 +1633,15 @@ E('ERR_PACKAGE_PATH_NOT_EXPORTED', (pkgPath, subpath, base = undefined) => {
return `Package subpath '${subpath}' is not defined by "exports" in ${
pkgPath}package.json${base ? ` imported from ${base}` : ''}`;
}, Error);
E('ERR_PACKAGE_SCRIPT_MISSING', (pjsonPath, script, parsedJSON, base) => {
if (!pjsonPath) {
return `unable to find package.json from directory ${base}`;
}
if (!('scripts' in parsedJSON)) {
return `package.json from directory ${base} (${pjsonPath}) has no "scripts" field`;
}
return `package.json from directory ${base} (${pjsonPath}) has no entry in "scripts": ${JSONStringify(script)}`;
}, Error);
E('ERR_PARSE_ARGS_INVALID_OPTION_VALUE', '%s', TypeError);
E('ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL', "Unexpected argument '%s'. This " +
'command does not take positional arguments', TypeError);
Expand Down
102 changes: 80 additions & 22 deletions lib/internal/main/watch_mode.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
'use strict';
const {
ArrayPrototypeFilter,
ArrayPrototypeForEach,
ArrayPrototypeJoin,
ArrayPrototypeMap,
ArrayPrototypePush,
ArrayPrototypePushApply,
ArrayPrototypeSlice,
JSONParse,
StringPrototypeStartsWith,
} = primordials;

Expand All @@ -18,14 +19,21 @@ const {
exitCodes: { kNoFailure },
} = internalBinding('errors');
const { getOptionValue } = require('internal/options');
const {
ERR_PACKAGE_SCRIPT_MISSING,
ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL,
} = require('internal/errors').codes;
const { FilesWatcher } = require('internal/watch_mode/files_watcher');
const { green, blue, red, white, clear } = require('internal/util/colors');

const { spawn } = require('child_process');
const { inspect } = require('util');
const { setTimeout, clearTimeout } = require('timers');
const { resolve } = require('path');
const { resolve, join } = require('path');
const { once } = require('events');
const { getNearestParentPackageJSON } = require('internal/modules/package_json_reader');
const { readFileSync } = require('fs');
const { emitWarning } = require('internal/process/warning');

prepareMainThreadExecution(false, false);
markBootstrapComplete();
Expand All @@ -35,43 +43,93 @@ const kKillSignal = 'SIGTERM';
const kShouldFilterModules = getOptionValue('--watch-path').length === 0;
const kWatchedPaths = ArrayPrototypeMap(getOptionValue('--watch-path'), (path) => resolve(path));
const kPreserveOutput = getOptionValue('--watch-preserve-output');
const kRun = getOptionValue('--run');
const kCommand = ArrayPrototypeSlice(process.argv, 1);
const kCommandStr = inspect(ArrayPrototypeJoin(kCommand, ' '));

const argsWithoutWatchOptions = [];
let kCommandStr = inspect(ArrayPrototypeJoin(kCommand, ' '));

for (let i = 0; i < process.execArgv.length; i++) {
const arg = process.execArgv[i];
if (StringPrototypeStartsWith(arg, '--watch')) {
i++;
const nextArg = process.execArgv[i];
if (nextArg && StringPrototypeStartsWith(nextArg, '-')) {
ArrayPrototypePush(argsWithoutWatchOptions, nextArg);
const argsWithoutWatchOptions = ArrayPrototypeFilter(process.execArgv, (v, i) => {
if (StringPrototypeStartsWith(v, '--watch')) {
return false;
}
if (v === '--run') {
return false;
}
if (i > 0 && v[0] !== '-') {
const prevArg = process.execArgv[i - 1];
if (StringPrototypeStartsWith(prevArg, '--watch')) {
return false;
}
if (prevArg === '--run') {
return false;
}
continue;
}
ArrayPrototypePush(argsWithoutWatchOptions, arg);
}
return true;
});

ArrayPrototypePushApply(argsWithoutWatchOptions, kCommand);

const watcher = new FilesWatcher({ debounce: 200, mode: kShouldFilterModules ? 'filter' : 'all' });
ArrayPrototypeForEach(kWatchedPaths, (p) => watcher.watchPath(p));

let graceTimer;
/**
* @type {ChildProcess}
*/
let child;
let exited;

function start() {
exited = false;
const stdio = kShouldFilterModules ? ['inherit', 'inherit', 'inherit', 'ipc'] : 'inherit';
child = spawn(process.execPath, argsWithoutWatchOptions, {
stdio,
env: {
...process.env,
WATCH_REPORT_DEPENDENCIES: '1',
},
});
if (!kRun) {
child = spawn(process.execPath, argsWithoutWatchOptions, {
stdio,
env: {
...process.env,
WATCH_REPORT_DEPENDENCIES: '1',
},
});
} else {
if (kCommand.length !== 0) {
throw new ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL('cannot provide positional argument to both --run and --watch');
}
// Always get new data in case it changed
const base = process.cwd();
const pkgJSON = getNearestParentPackageJSON(join(base, 'package.json'), false);
if (!pkgJSON) {
throw new ERR_PACKAGE_SCRIPT_MISSING(null, kRun, undefined, base);
}
let rawJSON;
try {
rawJSON = JSONParse(readFileSync(pkgJSON.data.pjsonPath, {
encoding: 'utf8',
}));
} catch (e) {
emitWarning(`Error parsing JSON in ${pkgJSON.data.pjsonPath}. ${e}`);
}
if (rawJSON) {
if (!('scripts' in rawJSON)) {
throw new ERR_PACKAGE_SCRIPT_MISSING(pkgJSON.data.pjsonPath, kRun, rawJSON, base);
}
if (!(kRun in rawJSON.scripts)) {
throw new ERR_PACKAGE_SCRIPT_MISSING(pkgJSON.data.pjsonPath, kRun, rawJSON, base);
}
const script = rawJSON.scripts[kRun];
const newCommandStr = inspect(script);
if (newCommandStr !== kCommandStr) {
kCommandStr = newCommandStr;
process.stdout.write(`${blue}Running ${kCommandStr}${white}\n`);
}
child = spawn(script, kCommand, {
stdio,
shell: true,
env: {
...process.env,
WATCH_REPORT_DEPENDENCIES: '1',
},
});
}
}
watcher.watchChildProcessModules(child);
child.once('exit', (code) => {
exited = true;
Expand Down
7 changes: 4 additions & 3 deletions lib/internal/modules/package_json_reader.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,15 @@ function deserializePackageJSON(path, contents) {
* }} options
* @returns {import('typings/internalBinding/modules').PackageConfig}
*/
function read(jsonPath, { base, specifier, isESM } = kEmptyObject) {
function read(jsonPath, { base, specifier, isESM, cached = true } = kEmptyObject) {
// This function will be called by both CJS and ESM, so we need to make sure
// non-null attributes are converted to strings.
const parsed = modulesBinding.readPackageJSON(
jsonPath,
isESM,
base == null ? undefined : `${base}`,
specifier == null ? undefined : `${specifier}`,
cached,
);

return deserializePackageJSON(jsonPath, parsed);
Expand All @@ -107,8 +108,8 @@ function readPackage(requestPath) {
* @param {string} checkPath The path to start searching from.
* @returns {undefined | {data: import('typings/internalBinding/modules').PackageConfig, path: string}}
*/
function getNearestParentPackageJSON(checkPath) {
const result = modulesBinding.getNearestParentPackageJSON(checkPath);
function getNearestParentPackageJSON(checkPath, cached = true) {
const result = modulesBinding.getNearestParentPackageJSON(checkPath, undefined, undefined, undefined, cached);

if (result === undefined) {
return undefined;
Expand Down
3 changes: 2 additions & 1 deletion src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -992,7 +992,8 @@ InitializeOncePerProcessInternal(const std::vector<std::string>& args,
}
}

if (!per_process::cli_options->run.empty()) {
if (!per_process::cli_options->run.empty() &&
!per_process::cli_options->per_isolate->per_env->watch_mode) {
// TODO(@anonrig): Handle NODE_NO_WARNINGS, NODE_REDIRECT_WARNINGS,
// --disable-warning and --redirect-warnings.
if (per_process::cli_options->per_isolate->per_env->warnings) {
Expand Down
31 changes: 20 additions & 11 deletions src/node_modules.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,17 @@ Local<Array> BindingData::PackageConfig::Serialize(Realm* realm) const {
}

const BindingData::PackageConfig* BindingData::GetPackageJSON(
Realm* realm, std::string_view path, ErrorContext* error_context) {
Realm* realm,
std::string_view path,
ErrorContext* error_context,
bool cached) {
auto binding_data = realm->GetBindingData<BindingData>();

auto cache_entry = binding_data->package_configs_.find(path.data());
if (cache_entry != binding_data->package_configs_.end()) {
return &cache_entry->second;
if (cached == true) {
auto cache_entry = binding_data->package_configs_.find(path.data());
if (cache_entry != binding_data->package_configs_.end()) {
return &cache_entry->second;
}
}

PackageConfig package_config{};
Expand Down Expand Up @@ -228,14 +233,14 @@ const BindingData::PackageConfig* BindingData::GetPackageJSON(
}
// package_config could be quite large, so we should move it instead of
// copying it.
auto cached = binding_data->package_configs_.insert(
auto after_cached = binding_data->package_configs_.insert(
{std::string(path), std::move(package_config)});

return &cached.first->second;
return &after_cached.first->second;
}

void BindingData::ReadPackageJSON(const FunctionCallbackInfo<Value>& args) {
CHECK_GE(args.Length(), 1); // path, [is_esm, base, specifier]
CHECK_GE(args.Length(), 1); // path, [is_esm, base, specifier, cached = true]
CHECK(args[0]->IsString()); // path

Realm* realm = Realm::GetCurrent(args);
Expand All @@ -255,6 +260,8 @@ void BindingData::ReadPackageJSON(const FunctionCallbackInfo<Value>& args) {
Utf8Value specifier(isolate, args[3]);
error_context.specifier = specifier.ToString();
}
// everything except exactly false default to cached
auto cached = !(args[4]->IsFalse());

THROW_IF_INSUFFICIENT_PERMISSIONS(
realm->env(),
Expand All @@ -264,11 +271,13 @@ void BindingData::ReadPackageJSON(const FunctionCallbackInfo<Value>& args) {
// TODO(StefanStojanovic): Remove ifdef after
// path.toNamespacedPath logic is ported to C++
#ifdef _WIN32
auto package_json = GetPackageJSON(
realm, "\\\\?\\" + path.ToString(), is_esm ? &error_context : nullptr);
auto package_json = GetPackageJSON(realm,
"\\\\?\\" + path.ToString(),
is_esm ? &error_context : nullptr,
cached);
#else
auto package_json =
GetPackageJSON(realm, path.ToString(), is_esm ? &error_context : nullptr);
auto package_json = GetPackageJSON(
realm, path.ToString(), is_esm ? &error_context : nullptr, cached);
#endif
if (package_json == nullptr) {
return;
Expand Down
3 changes: 2 additions & 1 deletion src/node_modules.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ class BindingData : public SnapshotableObject {
static const PackageConfig* GetPackageJSON(
Realm* realm,
std::string_view path,
ErrorContext* error_context = nullptr);
ErrorContext* error_context = nullptr,
bool cached = true);
static const PackageConfig* TraverseParent(
Realm* realm, const std::filesystem::path& check_path);
};
Expand Down
3 changes: 2 additions & 1 deletion src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,8 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors,
} else if (test_runner_force_exit) {
errors->push_back("either --watch or --test-force-exit "
"can be used, not both");
} else if (!test_runner && (argv->size() < 1 || (*argv)[1].empty())) {
} else if (!(test_runner || !per_process::cli_options->run.empty()) &&
(argv->size() < 1 || (*argv)[1].empty())) {
errors->push_back("--watch requires specifying a file");
}

Expand Down
43 changes: 43 additions & 0 deletions test/sequential/test-watch-mode.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ function createTmpFile(content = 'console.log("running");', ext = '.js', basenam
return file;
}

function createPackageJSON(content = {}, basename = tmpdir.path) {
const file = path.join(basename, 'package.json');
writeFileSync(file, JSON.stringify(content, null, 2));
return file;
}

async function runWriteSucceed({
file,
watchedFile,
Expand Down Expand Up @@ -550,6 +556,43 @@ console.log(values.random);
]);
});

it('should run when `--watch --run`', async () => {
const script = Math.random();
const output = Math.random();
const command = `echo ${output}`;
const file = createPackageJSON({
scripts: {
[script]: command
}
});
const args = ['--watch', '--run', script];
const { stdout, stderr } = await runWriteSucceed({ file, watchedFile: file, watchFlag: null, args, options: {
cwd: path.dirname(file)
} });

assert.strictEqual(stderr, '');
assert.deepStrictEqual(stdout, [
`Running '${command}'`,
`${output}`,
`Completed running '${command}'`,
]);
});

it('should error when `--watch --run` with positional arguments', async () => {
const file = createPackageJSON({
scripts: {
start: 'echo 123'
}
});
const args = ['--watch', '--run', 'start', 'positional'];
const { stdout, stderr } = await runWriteSucceed({ file, watchedFile: file, watchFlag: null, args, options: {
cwd: path.dirname(file)
} });

assert.match(stderr, /cannot provide positional argument to both --run and --watch/);
assert.deepStrictEqual(stdout, []);
});

it('should run when `--watch -r ./foo.js`', async () => {
const projectDir = tmpdir.resolve('project7');
mkdirSync(projectDir);
Expand Down
Loading