aspect-build / rules_js

High-performance Bazel rules for running Node.js tools and building JavaScript projects
https://docs.aspect.build/rules/aspect_rules_js
Apache License 2.0
310 stars 107 forks source link

[Bug]: Using bazel-bin/external/pnpm to run a helper shell file (via package.json script) include unnecessary environment variables breaks tooling #2012

Open rh-meowsa opened 2 weeks ago

rh-meowsa commented 2 weeks ago

What happened?

We have multiple shell files which helps locate build target and build specific arguments to run against bazel test and bazel run. These shell files run bazel commands. To trigger these shell scripts, we have added them to package.json scripts--so we can just pnpm run <script name> <args>.

While trying to add further Playwright integration to update snapshots, we are noticing that Playwright is reporting "missing tests". Upon investigating, we found that bazel-bin/external/pnpm includes a bunch of NPM_ and JSBINARY environment variables to the shell script, which in turn make available to the bazel run call in the script and confuses Playwright. If we were to bazel run directly from the terminal, Playwright can locate the test. It seems that the issue is from these environment variables being included as part of running pnpm as unsetting all of the JSBINARY* variables inside the shell script before calling bazel run fixes the issue.

Overall, the problem seem to stem from bazel-bin/external/pnpm as it should not be "leaking" these JSBINARY* environment variables.

Version

Development (host) and target OS/architectures:

Output of bazel --version: 7.1.1

Version of the Aspect rules, or other relevant rules from your WORKSPACE or MODULE.bazel file: 1.42.3

Language(s) and/or frameworks involved: Playwright

How to reproduce

No response

Any other information?

No response

Aghassi commented 2 weeks ago

The one thing I want to add here is that for us the reason this is occurring is because we have folks running pnpm via an alias that uses the binary fetched by bazel. E.g. bazel run @pnpm//:pnpm -- -C "$PWD" install

We are on rules_js .141.0

Aghassi commented 2 weeks ago

A workaround that we came up with temporarily is our shel scripts do this if they detect pnpm is running under bazel

# Unset any environment variables that start with JS_BINARY__
for var in $(env | grep -o '^JS_BINARY__[^=]*'); do
  if [ "$var" != "JS_BINARY__NODE_BINARY" ] && [ "$var" != "JS_BINARY__NODE_PATCHES" ]; then
    unset "$var"
  fi
done