Skip to content

Commit

Permalink
chore: Remove npm bin from scripts (#25913)
Browse files Browse the repository at this point in the history
This change removes the expression `export PATH=$(npm bin):$PATH`, which had formerly been used in scripts to ensure `node_modules` is in `PATH`.

`npm bin` was [removed in npm 9](npm/cli#5459). `npm exec` or `npx` should be used instead. `build.sh` already uses `npx`. This change revises `scripts/gen.sh` to use `npx` as well.

Prior to this change, within shells executing `build.sh` or `scripts/gen.sh`, `PATH` actually contains error text if npm 9+ is used.

```
~/repos/aws-cdk $ docker run --rm -v $PWD:$PWD -w $PWD node:hydrogen-alpine sh -c 'node --version && npm --version && export PATH=$(npm bin):$PATH && echo $PATH' # output when npm bin is unavailable
v18.16.0
9.5.1
Unknown command: "bin" To see a list of supported npm commands, run: npm help:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin

~/repos/aws-cdk $ docker run --rm -v $PWD:$PWD -w $PWD node:gallium-alpine sh -c 'node --version && npm --version && export PATH=$(npm bin):$PATH && echo $PATH' # output when npm bin is available
v16.20.0
8.19.4
/Users/douglasnaphas/repos/aws-cdk/node_modules/.bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
```

It didn't make `build.sh` fail, because `lerna` has been run via `npx` since #24217, and `build.sh` doesn't need anything from `node_modules` to be added to `PATH`. `export PATH=$(npm bin):$PATH` succeeds even though `npm bin` fails, per `export`'s normal behavior.

Prior to this change, `scripts/gen.sh` failed with

```
./scripts/gen.sh: line 18: lerna: command not found
```

when I ran it. After this change, `scripts/gen.sh` ran successfully.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
douglasnaphas authored Jun 12, 2023
1 parent 23a84d3 commit 12fe60e
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 4 deletions.
1 change: 0 additions & 1 deletion build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ while [[ "${1:-}" != "" ]]; do
shift
done

export PATH=$(npm bin):$PATH
export NODE_OPTIONS="--max-old-space-size=8196 --experimental-worker ${NODE_OPTIONS:-}"

# Temporary log memory for long builds (this may mess with tests that check stderr)
Expand Down
5 changes: 2 additions & 3 deletions scripts/gen.sh
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#!/bin/bash
set -euo pipefail

export PATH=$(npm bin):$PATH
export NODE_OPTIONS="--max-old-space-size=8196 ${NODE_OPTIONS:-}"

echo "============================================================================================="
Expand All @@ -15,8 +14,8 @@ fail() {

echo "============================================================================================="
echo "building required build tools..."
time lerna run --stream build --scope @aws-cdk/cfn2ts --scope @aws-cdk/ubergen --include-dependencies || fail
time npx lerna run --stream build --scope @aws-cdk/cfn2ts --scope @aws-cdk/ubergen --include-dependencies || fail

echo "============================================================================================="
echo "executing gen..."
time lerna run --stream gen || fail
time npx lerna run --stream gen || fail

0 comments on commit 12fe60e

Please sign in to comment.