[Bug]: rules_js 2.0 pnpm 9 fails to load pnpm-lock.yaml

[hjellek]

What happened?

Thanks for all your hard work!

After upgrading our monorepo to rules_js 2.0.0-alpha.7 and testing the pnpm9 support, npm_translate_lock seems to have problem parsing:

ERROR: Traceback (most recent call last):
	File "/private/var/tmp/_bazel_knut.hjelle/5c024f616e5754a9bdc69a64d7cdfdce/external/aspect_rules_js~/npm/extensions.bzl", line 29, column 41, in _npm_extension_impl
		_npm_lock_imports_bzlmod(module_ctx, attr)
	File "/private/var/tmp/_bazel_knut.hjelle/5c024f616e5754a9bdc69a64d7cdfdce/external/aspect_rules_js~/npm/extensions.bzl", line 83, column 58, in _npm_lock_imports_bzlmod
		importers, packages = translate_to_transitive_closure(
	File "/private/var/tmp/_bazel_knut.hjelle/5c024f616e5754a9bdc69a64d7cdfdce/external/aspect_rules_js~/npm/private/transitive_closure.bzl", line 197, column 71, in translate_to_transitive_closure
		package_info["transitive_closure"] = gather_transitive_closure(
	File "/private/var/tmp/_bazel_knut.hjelle/5c024f616e5754a9bdc69a64d7cdfdce/external/aspect_rules_js~/npm/private/transitive_closure.bzl", line 67, column 21, in gather_transitive_closure
		fail(msg)
Error in fail: Unknown package key: @docusaurus/[email protected]_react_18.2.0 in ["@algolia/autocomplete-core/1.9.3_755392440", .....

I see that the package is listed in the list with a slightly different format:

"@docusaurus/react-loadable/5.5.2_react_18.2.0"

vs

@docusaurus/[email protected]_react_18.2.0

I created a demo-branch in a separate repository that also shows the same error here: https://github.com/hjellek/rules_js_alpha_test/compare/pnpm_9

Version

Development (host) and target OS/architectures:
macos, arm64

Output of bazel --version:
bazel 7.1.2

Version of the Aspect rules, or other relevant rules from your
WORKSPACE or MODULE.bazel file:
rules_js 2.0.0-alpha.7

Language(s) and/or frameworks involved:
JavaScript, Docusaurus

How to reproduce

I created a repo which reproduces the problem in the pnpm_9 branch
https://github.com/hjellek/rules_js_alpha_test

On `main`, `bazel build //apps/docusaurus:app --spawn_strategy=local` works as expected (docusaurus currently hangs on sandbox)

On the branch `pnpm_9` where the lockfile format is updated to v9.0 (updated with `bazel run -- @pnpm//:pnpm --dir "$PWD" i --lockfile-only`), build fails due to missing packages while resolving the lockfile with the error listed

Any other information?

No response

[gregmagolan]

Thanks for trying out the alpha release and the bug report. @jbedard is currently working on a refactor on the lock file parsing code (#1735) that will hopefully fix this. The changes in the pnpm v9 lockfile format were significant enough that a refactor and better test coverage was warranted. After that lands we'll cut another alpha release.

[jbedard]

@hjellek would you be able to try again in the next alpha after #1735 merges? That fixes some edge cases with pnpm9 which might include this issue.

[gregmagolan]

@hjellek Can you try again with https://github.com/aspect-build/rules_js/releases/tag/v2.0.0-alpha.8?

[hjellek]

@gregmagolan / @jbedard I've tried with 2.0.0-rc0 and now it seems to work as expected! Thanks for handling it so quickly.

[jbedard]

Great 👍

I'll close this for now then. But please log another or comment here if you see anything similar.

[hjellek]

But I might just have found another and probably related bug.

In our main repo, npm_translate_lock now resolves just fine. But one of our applications running Docusaurus fails building due to:

Module not found: Error: Can't resolve 'react-loadable' in '/private/var/tmp/_bazel_knut.hjelle/e8e018c965c1b1ff18d8534c832243ca/execroot/_main/bazel-out/darwin_arm64-fastbuild/bin/node_modules/.aspect_rules_js/@[email protected]_-122611188/node_modules/@docusaurus/core/lib/client/exports'

I see that react-loadable is listed in @docusaurus/core as "react-loadable": "npm:@docusaurus/[email protected]",

bazel-out/darwin_arm64-fastbuild/bin/node_modules/.aspect_rules_js/@[email protected]_react_18.2.0 exists, but not bazel-out/darwin_arm64-fastbuild/bin/node_modules/.aspect_rules_js/[email protected]_react_18.2.0 which I guess is what Docusaurus expects.

I don't see the same error in the example repo but that might be because the app itself is empty - I will try to reproduce it there as well.

[jbedard]

Yeah, that sounds related to pnpm9 and how those deps are processed in rules_js.

I was having issues with npm: even with plain pnpm9 though so I haven't yet added proper tests for it yet within rules_js either.

If you can put a minimal repro together that would be great 👍

[hjellek]

I have updated the case in https://github.com/hjellek/rules_js_alpha_test/compare/pnpm_9 now.

It did not fail until I ran bazel clean first, probably leftover symlinks from the pnpm 8 install locally on my machine.

[gregmagolan]

@hjellek I cloned down your repo to try to repro and the build hung indefinitely on the first bazel build:

DEBUG: /private/var/tmp/_bazel_greg/35c5b8f055f6df6a32439a05008ada9c/external/aspect_rules_js~/npm/extensions.bzl:272:18: NOTE: repo 'pnpm' has multiple versions ["9.1.1", "8.6.7"]; selected 9.1.1
INFO: Analyzed 6 targets (1184 packages loaded, 12675 targets configured).
[14,505 / 14,506] Docusaurus apps/docusaurus/build; 6721s darwin-sandbox

Did you ever observe that in your testing?

[hjellek]

@gregmagolan yeah, there is something probably traversing symlinks in one of the webpack plugins for docusaurus - I have never been able to find it unfortunately. I mentioned it in how to reproduce, as we build it with --spawn_strategy=local

[hjellek]

I think I managed to reproduce it with a more local setup in the branch pnpm_9_npm_rename_package here: hjellek/rules_js_alpha_test@pnpm_9...pnpm_9_npm_rename_package

ERROR: /[....]/rules_js_alpha_test/BUILD.bazel:3:22: in deps attribute of npm_package_store rule //:.aspect_rules_js/node_modules/[email protected]: target '//:.aspect_rules_js/node_modules/local-lpad@[email protected]' does not exist. Since this rule was created by the macro 'npm_link_all_packages', the error might have been caused by the macro implementation

lpad seems to be correctly synced up, but local-lpad is not

bazel query //... | grep lpad
Loading: 0 packages loaded
//:.aspect_rules_js/node_modules/[email protected]
//:.aspect_rules_js/node_modules/[email protected]/dir
//:.aspect_rules_js/node_modules/[email protected]/pkg
//:.aspect_rules_js/node_modules/[email protected]/ref
//apps/package-dep-override:node_modules/local-lpad
//apps/package-dep-override:node_modules/local-lpad/dir

[jbedard]

The npm: case is one I'm working on atm. I've listed the known issues currently being worked on in #1753

[hjellek]

@jbedard I think I have stumbled across another edge case with 2.0.0-rc2 😬
Our Remix and NextJS apps fail with a similiar dependency error:

ERROR: /..../rules_js_alpha_test/BUILD.bazel:3:22: in deps attribute of npm_package_store_internal rule //:.aspect_rules_js/node_modules/@[email protected]/pkg: target '//:.aspect_rules_js/node_modules/string-width-cjs@[email protected]/ref' does not exist. Since this rule was created by the macro 'npm_link_all_packages', the error might have been caused by the macro implementation

I have recreated the problem in https://github.com/hjellek/rules_js_alpha_test/tree/pnpm_9_remix, it should be verifiable with bazel build //apps/remix:build

The package in question lists the same dependency in different versions using the npm: rename strategy https://github.com/isaacs/cliui/blob/main/package.json#L53 but I noticed that the entry in the lockfile does not contain the npm: prefix:

  '@isaacs/[email protected]':
    dependencies:
      string-width: 5.1.2
      string-width-cjs: [email protected]

Not sure if that is related though.

Please let me know it you want me to file this as a separate issue, and again - thanks for your work!

[jbedard]

That same package keeps coming up as an interesting test case, and I swear I fixed it. I'll take another look. Thanks for the info 👍

[jbedard]

See 3e08f8e and the latest rc

[hjellek]

@jbedard the remix-example now works as expected with 2.0.0-rc3!

Unfortunately I am back to what looks close to the original error for the docusaurus example I provided:

Error in fail: Unknown package key: @docusaurus/[email protected]([email protected]) (@docusaurus/[email protected](react @ 18.2.0)) in [...,"@docusaurus/[email protected]_react_18.2.0",...]

See https://github.com/hjellek/rules_js_alpha_test/tree/pnpm_9 and bazel build //apps/docusaurus:all --spawn_strategy=local

This example builds as expected with 2.0.0-rc2.

[hjellek]

I updated the main branch in my example repo with all of the testcases so far.

All except remix builds in 2.0.0-rc2, but after upgrading to 2.0.0-rc3 the lock translation fails as mentioned above.

[jbedard]

Thank you once again @hjellek. I think this fixes it: #1776

More tests every time to (I hope) not break these things in the future :|

[hjellek]

Great work, I will test it when the next rc lands!