Skip to content
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
11 changes: 11 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -2637,6 +2637,9 @@ forked processes, or clustered processes.
<!-- YAML
added: v22.0.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/64606
description: Passing `--run` without a command lists the available scripts.
- version: v22.3.0
pr-url: https://github.com/nodejs/node/pull/53032
description: NODE_RUN_SCRIPT_NAME environment variable is added.
Expand All @@ -2653,6 +2656,14 @@ changes:
This runs a specified command from a package.json's `"scripts"` object.
If a missing `"command"` is provided, it will list the available scripts.

Passing `--run` without a command lists the available scripts and exits:

```console
$ node --run
Available scripts are:
test: node --test
```

`--run` will traverse up to the root directory and finds a `package.json`
file to run the command from.

Expand Down
3 changes: 2 additions & 1 deletion src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1133,7 +1133,8 @@ InitializeOncePerProcessInternal(const std::vector<std::string>& args,
}
}

if (!per_process::cli_options->run.empty()) {
// A bare `--run` (empty value) lists the available scripts; a value runs it.
if (per_process::cli_options->has_run) {
auto positional_args = task_runner::GetPositionalArgs(args);
result->early_return_ = true;
task_runner::RunTask(
Expand Down
18 changes: 14 additions & 4 deletions src/node_options-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <algorithm>
#include <cstdlib>
#include <ranges>
#include <type_traits>
#include "node_options.h"
#include "util.h"

Expand Down Expand Up @@ -453,18 +454,21 @@ void OptionsParser<Options>::Parse(

std::string value;
if (info.type != kBoolean && info.type != kNoOp && info.type != kV8Option) {
// `--run` may be passed without a script name to list available scripts,
// so an omitted value is not an error and must not swallow a later flag.
const bool optional_value = name == "--run";
Comment on lines +457 to +459

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better solution to modifying the options parser directly? We have other CLI flags that have different behavior depending on whether or not they have an argument passed?

How does --inspect do this?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely familiar with the code, but looking at --inspect, it's a boolean option so works a bit differently I think:

node/src/node_options.cc

Lines 475 to 479 in 0bcc6ef

AddOption("--inspect",
"activate inspector on host:port (default: 127.0.0.1:9229)",
&DebugOptions::inspector_enabled,
kAllowedInEnvvar);
AddAlias("--inspect=", { "--inspect-port", "--inspect" });

So --inspect and --inspect=1234 work, but --inspect 1234 isn't valid.

If I'm understanding correctly, --run can't do this, because it's space separated.

To avoid the hardcoded name == "--run", what would you suggest?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, perhaps I was mis-remembering that --inspect required an = to run 😓. I'm still wary about changing the parser, since there probably should be a way for a flag to be passed with an optional value, regardless of =.

Sorry about misremembering, I'll ask around to see if there's a way to do this, and if not, we can always add it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! It's also very possible I'm misreading this or missing something, and this is already supported somewhere - sorry if so!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt that, you (so far) demonstrate a great understanding of the codebase. It's probable that we just never encountered this scenario before.

Great work so far :-)

if (equals_index != std::string::npos) {
value = arg.substr(equals_index + 1);
if (value.empty()) {
if (value.empty() && !optional_value) {
missing_argument();
break;
}
} else {
if (args.empty()) {
} else if (args.empty() || (optional_value && args.first()[0] == '-')) {
if (!optional_value) {
missing_argument();
break;
}

} else {
value = args.pop_first();

if (!value.empty() && value[0] == '-') {
Expand Down Expand Up @@ -512,6 +516,12 @@ void OptionsParser<Options>::Parse(
default:
UNREACHABLE();
}

// Record that `--run` was seen so an empty value can be distinguished from
// the option being absent. Guarded so it only compiles for the owning type.
if constexpr (std::is_same_v<Options, PerProcessOptions>) {
if (name == "--run") options->has_run = true;
}
}
options->CheckOptions(errors, orig_args);
}
Expand Down
3 changes: 3 additions & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,9 @@ class PerProcessOptions : public Options {
bool print_version = false;
std::string experimental_sea_config;
std::string run;
// Tracks whether `--run` was passed, since an empty `run` is ambiguous
// between "not passed" and "passed without a script name" (lists scripts).
bool has_run = false;

std::string build_sea;
#ifdef NODE_HAVE_I18N_SUPPORT
Expand Down
48 changes: 31 additions & 17 deletions src/node_task_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,27 @@ FindPackageJson(const std::filesystem::path& cwd) {
return {{package_json_path, raw_content, path_env_var}};
}

// Prints every "name: command" pair in the scripts object to the given stream.
static void PrintScripts(FILE* out,
simdjson::ondemand::object& scripts_object) {
// Reset the object to iterate from the beginning, in case it was read before.
scripts_object.reset();
simdjson::ondemand::value value;
for (auto field : scripts_object) {
std::string_view key_str;
std::string_view value_str;
if (!field.unescaped_key().get(key_str) && !field.value().get(value) &&
!value.get_string().get(value_str)) {
fprintf(out,
" %.*s: %.*s\n",
static_cast<int>(key_str.size()),
key_str.data(),
static_cast<int>(value_str.size()),
value_str.data());
}
}
}

void RunTask(const std::shared_ptr<InitializationResultImpl>& result,
std::string_view command_id,
const std::vector<std::string_view>& positional_args) {
Expand Down Expand Up @@ -300,6 +321,15 @@ void RunTask(const std::shared_ptr<InitializationResultImpl>& result,
return;
}

// With no command (e.g. bare `node --run`), list the available scripts to
// stdout and exit successfully, mirroring `npm run`.
if (command_id.empty()) {
fprintf(stdout, "Available scripts are:\n");
PrintScripts(stdout, scripts_object);
result->exit_code_ = ExitCode::kNoFailure;
return;
}

// If the command_id is not found in the scripts object, throw an error.
std::string_view command;
if (auto command_error =
Expand All @@ -317,23 +347,7 @@ void RunTask(const std::shared_ptr<InitializationResultImpl>& result,
command_id.data(),
path.string().c_str());
fprintf(stderr, "Available scripts are:\n");

// Reset the object to iterate over it again
scripts_object.reset();
simdjson::ondemand::value value;
for (auto field : scripts_object) {
std::string_view key_str;
std::string_view value_str;
if (!field.unescaped_key().get(key_str) && !field.value().get(value) &&
!value.get_string().get(value_str)) {
fprintf(stderr,
" %.*s: %.*s\n",
static_cast<int>(key_str.size()),
key_str.data(),
static_cast<int>(value_str.size()),
value_str.data());
}
}
PrintScripts(stderr, scripts_object);
}
result->exit_code_ = ExitCode::kGenericUserError;
return;
Expand Down
14 changes: 14 additions & 0 deletions test/message/node_run_list.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
'use strict';

require('../common');
const assert = require('node:assert/strict');
const childProcess = require('node:child_process');
const fixtures = require('../common/fixtures');

const child = childProcess.spawnSync(
process.execPath,
[ '--no-warnings', '--run'],
{ cwd: fixtures.path('run-script'), encoding: 'utf8' },
);
assert.strictEqual(child.status, 0);
console.log(child.stdout);
14 changes: 14 additions & 0 deletions test/message/node_run_list.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
Available scripts are:
test: echo "Error: no test specified" && exit 1
ada: ada
ada-windows: ada.bat
positional-args: positional-args
positional-args-windows: positional-args.bat
custom-env: custom-env
custom-env-windows: custom-env.bat
path-env: path-env
path-env-windows: path-env.bat
special-env-variables: special-env-variables
special-env-variables-windows: special-env-variables.bat
pwd: pwd
pwd-windows: cd
35 changes: 35 additions & 0 deletions test/parallel/test-node-run.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,4 +222,39 @@ describe('node --run [command]', () => {
assert.strictEqual(child.stdout, '');
assert.strictEqual(child.code, 1);
});

it('lists available scripts to stdout when no command is given', async () => {
const child = await common.spawnPromisified(
process.execPath,
[ '--run'],
{ cwd: fixtures.path('run-script') },
);
assert.match(child.stdout, /Available scripts are:/);
assert.match(child.stdout, /test: echo "Error: no test specified" && exit 1/);
assert.strictEqual(child.stderr, '');
assert.strictEqual(child.code, 0);
});

it('does not consume a following flag as the script name', async () => {
// `--run` followed by a flag lists scripts rather than treating the flag
// as a script name.
const child = await common.spawnPromisified(
process.execPath,
[ '--run', '--no-warnings'],
{ cwd: fixtures.path('run-script') },
);
assert.match(child.stdout, /Available scripts are:/);
assert.strictEqual(child.code, 0);
});

it('errors when listing scripts without a package.json', async () => {
const child = await common.spawnPromisified(
process.execPath,
[ '--run'],
{ cwd: __dirname },
);
assert.match(child.stderr, /Can't find package\.json/);
assert.strictEqual(child.stdout, '');
assert.strictEqual(child.code, 1);
});
});