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]: Local NPM package don't transitively propagate node_modules #1654

Closed
pat-trunk-io opened this issue Apr 17, 2024 · 1 comment
Closed
Labels
bug Something isn't working untriaged Requires traige

Comments

@pat-trunk-io
Copy link

pat-trunk-io commented Apr 17, 2024

What happened?

When creating a package with the rule npm_package, if it gets included in another package, the node_modules dependencies do not get propagated correctly (I think).

For example, have a package A with dependency typescript, and a package B with dependency on A, bazel-bin/packages/B/node_modules/A/node_modules does not exits, yet bazel-bin/packages/A/node_modules exists

Version

Development (host) and target OS/architectures: linux/amd64

Output of bazel --version: bazel 7.1.1

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

Language(s) and/or frameworks involved: typescript, skylark

How to reproduce

Created a minimal repro here: https://github.com/pat-trunk-io/npm_package_bug_repro

Success when:

pnpm i
node packages/B/main.mjs

Failure when:

bazel run //packages/B:script

It is entirely possible I am using the npm_package rule wrong, but I am converting quite a large codebase that access things through import.meta.url into the node_modules.

Any other information?

With a little guidance, I would be ready to help and implement the fix.

My mental model is that in @npm/defs.bzl something is missing to transitively propagate the node_modules deps.

@pat-trunk-io pat-trunk-io added the bug Something isn't working label Apr 17, 2024
@github-actions github-actions bot added the untriaged Requires traige label Apr 17, 2024
@pat-trunk-io
Copy link
Author

So looking at https://nodejs.org/api/modules.html#modules_loading_from_node_modules_folders
It seems aspect_rules depend on the behavior that it will look upwards for directory having a node_modules (which it does in the package_store)

Therefore using require.resolve(...) is the correct way to make the code behave correctly.

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

No branches or pull requests

1 participant