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

[Bug]: dev = True support missing from pnpm version 9 lockfiles #2013

Open
sin-ack opened this issue Nov 7, 2024 · 1 comment
Open

[Bug]: dev = True support missing from pnpm version 9 lockfiles #2013

sin-ack opened this issue Nov 7, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@sin-ack
Copy link

sin-ack commented Nov 7, 2024

What happened?

I added the following to my MODULE.bazel:

npm = use_extension("@aspect_rules_js//npm:extensions.bzl", "npm")
npm.npm_translate_lock(
    name = "npm_dev",
    npmrc = "//:.npmrc",
    pnpm_lock = "//:pnpm-lock.yaml",
    dev = True,
)
use_repo(npm, "npm_dev")

The intention here is to use this together with ts_project because right now it's including dev dependencies in its output (which might actually be related to this issue...).

However, when I add dev = True, no dependencies actually get included in the resulting linked packages, and I get errors related to missing type declarations.

Version

Development (host) and target OS/architectures: Host+target: Gentoo Linux

Output of bazel --version: 7.4.0

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

Language(s) and/or frameworks involved: JavaScript

How to reproduce

  1. Create a new directory
  2. pnpm init
  3. pnpm i -D left-pad (any package)
  4. echo 'require("left-pad"); require("fs").writeFileSync("dummy.txt", "");' > index.js
  5. Add the following to MODULE.bazel:
module(
    name = "repro",
    version = "0.0.0",
)

bazel_dep(name = "aspect_rules_js", version = "2.1.0")

npm = use_extension("@aspect_rules_js//npm:extensions.bzl", "npm")
npm.npm_translate_lock(
    name = "npm_dev",
    pnpm_lock = "//:pnpm-lock.yaml",
    dev = True,
)
use_repo(npm, "npm_dev")
  1. Add the following to BUILD.bazel:
load("@npm_dev//:defs.bzl", npm_dev_link = "npm_link_all_packages")
load("@aspect_rules_js//js:defs.bzl", "js_binary", "js_run_binary")

npm_dev_link(name = "node_modules_dev")

js_binary(
    name = "repro_bin",
    data = [":node_modules_dev"],
    entry_point = "index.js",
)

# Required to get a sandboxed environment (i.e. avoid local node_modules)
js_run_binary(
    name = "repro",
    tool = ":repro_bin",
    outs = ["dummy.txt"],
)
  1. Add "pnpm": {"onlyBuiltDependencies": []} to package.json
  2. Run bazel build repro. Get the following error:
ERROR: /home/.../tmp/repro/BUILD.bazel:12:14: JsRunBinary result.txt failed: (Exit 1): repro_bin failed: error executing JsRunBinary command (from target //:repro) bazel-out/k8-opt-exec-ST-d57f47055a04/bin/repro_bin_/repro_bin

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
node:internal/modules/cjs/loader:1143
  throw err;
  ^

Error: Cannot find module 'left-pad'
Require stack:
- /home/.../.cache/bazel/_bazel_.../4c58cc4ead2e23f682ce8e1d4c0dcef1/sandbox/linux-sandbox/614/execroot/_main/bazel-out/k8-fastbuild/bin/index.js
...

You can remove dev = True from the translate_lock call to have it working.

Any other information?

The offending line seems to be:

dev = None, # TODO(pnpm9): must inspect importers.*.devDependencies?

@sin-ack sin-ack added the bug Something isn't working label Nov 7, 2024
@sin-ack
Copy link
Author

sin-ack commented Nov 7, 2024

I tried to solve this locally, but I'm kind of confused about how this should interact with importers. My understanding is that a package can be a development dependency in one workspace and a production dependency on another. How do workspaces interact with rules_js? Do we care about maintaining separate identities for dev and non-dev packages?

Also, it seems dev packages are filtered out twice, once during the transitive closure generation and another in helpers.get_npm_imports. The latter IMO is invalid because it's possible that a package marked as dev in our package.json is depended on by a production package or vice-versa (and in fact I did hit this while testing, where production packages depend on filtered out dev packages). We already culled the roots based on the user's request during the closure generation, we shouldn't cull individual packages from the graph.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant