Enhancement: Check for shell builtin before wrapping
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 10.1k
- Forks
- 4.7k
- Avg merge
- 2d 2h
- Merged PRs (30d)
- 19
Description
The introduction of wrapping commands given the possibility it is a shell builtin keyword on *nix envs results in potential issues in projects relying on npm_lifecycle_script env var given the value has now changed.
This was introduced in https://github.com/npm/cli/commit/fdc3413019c2f34f1fde35449e5f3a6b0fb51ba2
Instead of wrapping all cmds it would be beneficial to first check if the command is a builtin prior to wrapping.
Something such as ...
if (!isWindowsShell) {
if (args.length > 0) {
let builtin = false;
exec(`type ${args[0]}`, (error, stdout, stderr) => { if(stdout.includes('keyword')) { builtin = true; } });
if(builtin) {
args[0] = '"' + args[0] + '"';
}
}
}
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by tracing the command-wrapping behavior introduced in commit fdc3413019c2f34f1fde35449e5f3a6b0fb51ba2, then inspect how npm_lifecycle_script changes for shell builtins on Unix environments. Done means builtin commands are checked before wrapping and projects relying on the original environment variable value no longer regress.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, nodejs
- Domain
- cli
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100