aspect-build / aspect-build/rules_js
[Bug]: `public_hoist_pattern` behaves different to `public_hoist_pattern` in `pnpm`
- Dominant language
- Starlark
- Stars
- 378
- Forks
- 183
- Avg merge
- 1d 9h
- Merged PRs (30d)
- 32
Description
### What happened?
`public_hoist_pattern` behaves different to `public_hoist_pattern` in `pnpm`.
### rules_js behavior
Take examples in `rules_js` repository as an example (I'm at acb0f7efe9be99b4bb4766b3bd13064b6f891b59), where `ms` is listed in the `public_hoist_pattern`.
What left in the sandbox becomes the following after `bazel test examples/npm_deps:test6 --sandbox_debug` is ran.
It's clear that `ms` is hoisted to `/examples/npm_deps/node_modules`. However, it's not hoisted to `/node_modules`. (I tried set `"ms@2.1.3"` to `["examples/npm_deps", ""]` or just `[""]` in `public_hoist_packages` but that doesn't make any difference).
### pnpm behavior
Running `pnpm install` in `rules_js` repository root gives the following `node_modules` structure:
It hoists the `ms` packages under `/node_modules/ms` instead of `/examples/npm_deps/node_modules`.
### Config to mimic the pnpm behavior
After a few try&error when writing this issue, I finally managed to get what I want, so in order to mimic `node_modules` hoisting behavior in `pnpm`, `"ms@2.1.3"` need to be configured as an array that contains `[""]`, **and use `//:node_modules/ms` instead of `//examples/npm_deps:node_modules/ms`**.
### And here is what confuse me
The problem I have is the `node_modules` resolution when a plugin comes into play and hence I got [the doc](https://docs.aspect.build/rulesets/aspect_rules_js/docs/troubleshooting/#its-a-plugin).
> Note that public_hoist_packages affects the layout of the node_modules tree, but you still need to depend on that hoisted package, e.g. with deps = [":node_modules/hoisted_pkg"]. Continuing the example:
Since it says `:node_modules/hoisted_pkg` instead of `//:node_modules/hoisted_pkg`, I believe it will do the hoisting just like what `pnpm` does, and the doc of `public_hoisted_packages` implies so as well.
https://docs.aspect.build/rulesets/aspect_rules_js/docs/npm_translate_lock#public_hoist_packages
> This is similar to setting https://pnpm.io/npmrc#public-hoist-pattern in an .npmrc file outside of Bazel, however, wild-cards are not yet supported and npm_translate_lock will fail if there are multiple versions of a package that are to be hoisted.
### Question
So first of all, I'm not sure what's the expected behavior of `public_hoist_packages`? Should it do what `pnpm` does, to hoist the dependency into the root `node_modules`, or like what it is now, to hoist the dependency to the specific package?
If the later is the case, maybe the doc can be improved to be less confusing?
### Version
Development (host) and target OS/architectures: MacOS (pretty irrelevant I think)
Output of `bazel --version`:
```
> bazel --version
aspect 5.9.25
```
Version of the Aspect rules, or other relevant rules from your
`WORKSPACE` or `MODULE.bazel` file: `"0.0.0"`, as I managed to reproduce it in `rules_js`'s repository. It's latest main at acb0f7efe9be99b4bb4766b3bd13064b6f891b59
Language(s) and/or frameworks involved: Nothing.
### How to reproduce
_No response_
### Any other information?
_No response_
Contributor guide
Research direction
Reproduce the discrepancy in rules_js with `bazel test examples/npm_deps:test6 --sandbox_debug`, comparing `public_hoist_pattern` and `public_hoist_packages` configurations. Read the linked `npm_translate_lock` and troubleshooting documentation, then determine which node_modules layout is intended. Done means the behavior and documentation clearly agree about root versus package-level hoisting.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, node.js
- Domain
- build-system, tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100