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]: Transitive dependencies are not present in js_image_layer's node_modules #677

Closed
flolu opened this issue Dec 2, 2022 · 14 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@flolu
Copy link

flolu commented Dec 2, 2022

What happened?

I'm using js_image_layer to build Docker images. But currently, transitive dependencies of node_modules are not passed on to it.

Version

Development (host) and target OS/architectures: Linux (Fedora)

Output of bazel --version:

Bazelisk version: v1.13.2
Build label: 7.0.0-pre.20221102.3
Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Fri Nov 11 18:06:13 2022 (1668189973)
Build timestamp: 1668189973
Build timestamp as int: 1668189973

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

  • rules_js-1.8.0
  • rules_ts-1.0.2
  • rules_swc-0.19.3
  • rules_pkg-0.8.0
  • rules_docker-0.25.0

How to reproduce

git clone https://github.com/flolu/rules-ts-transitive-deps
cd rules-ts-transitive-deps
pnpm i
bazelisk run //:image && docker run --rm -it bazel:image
@flolu flolu added the bug Something isn't working label Dec 2, 2022
@flolu
Copy link
Author

flolu commented Dec 2, 2022

@thesayyn do you know how to prevent having to specify transitive dpendencies?

@thesayyn thesayyn self-assigned this Dec 2, 2022
@thesayyn
Copy link
Member

thesayyn commented Dec 5, 2022

they should be included automatically. Is this happening with all packages or single one?

@flolu
Copy link
Author

flolu commented Dec 5, 2022

@thesayyn it's happening with all packages. you can check the repro: https://github.com/flolu/rules-ts-transitive-deps

@matthewjh
Copy link

matthewjh commented Dec 7, 2022

I am also seeing this issue, but only with certain transitive deps. Bazel run'ing the binary itself works fine; the problem is introduced somehow by the js_image packaging. Will reply when/if I gather some more info.

@flolu
Copy link
Author

flolu commented Jan 3, 2023

@thesayyn any updates?

@matthewjh
Copy link

Update: my issue turned out to be caused by something else. There was some difference between my local pnpm-installed node_modules and the rules_js-managed ones (perhaps because I didn't set hoist=false in my .npmrc?) that generated a difference in runtime module resolution between my local bazel run and running the built binary inside the docker image. Modules should not be resolved out of the runfiles, anyway, but that is slightly broken due to #656 and #362.

However, I have separately noticed ts_project deps not being propagated in an all or nothing way in sandboxed build actions, so I still think this issue is valid.

@flolu
Copy link
Author

flolu commented Jan 9, 2023

@thesayyn do you think rules_ts#280 is the solution to this issue?

@flolu
Copy link
Author

flolu commented Jan 11, 2023

@mvukov I've tried to patch my repro from this issue with the fix from your pull request on this branch.

But the issue still persists. Did I mess up the patching?

@thesayyn
Copy link
Member

thesayyn commented Jan 13, 2023

However, I have separately noticed ts_project deps not being propagated in an all or nothing way in sandboxed build actions, so I still think this issue is valid.

i can confirm that the cause of this issue is indeed ts_project.

@flolu yes that's the solution to this particular problem.

for what is worth, you may workaround this problem by listing your deps in npm_package. until

index d3bebbe..677e317 100644
--- a/lib1/BUILD.bazel
+++ b/lib1/BUILD.bazel
@@ -20,5 +20,6 @@ npm_package(
     name = "lib1",
     srcs = [":lib1_project"],
     package = "@org/lib1",
+    data = ["//:node_modules/@org/lib2"],
     visibility = ["//visibility:public"],
 )
diff --git a/lib2/BUILD.bazel b/lib2/BUILD.bazel
index f6c6199..46d807e 100644
--- a/lib2/BUILD.bazel
+++ b/lib2/BUILD.bazel
@@ -19,6 +19,7 @@ ts_project(
 npm_package(
     name = "lib2",
     srcs = [":lib2_project"],
+    data = ["//:node_modules/@faker-js/faker"],
     package = "@org/lib2",
     visibility = ["//visibility:public"],
 )

output of image: { email: '[email protected]' }

@flolu
Copy link
Author

flolu commented Jan 13, 2023

@thesayyn the patch you provided doesn't seem to work for me:

ERROR: /home/flolu/Desktop/rules-ts-transitive-deps/BUILD.bazel:86:16: in tars attribute of container_layer_ rule //:node_modules_layer: '//:node_modules_tar' does not produce any container_layer_ tars files (expected .tar, .tar.xz, .tar.gz or .tgz). Since this rule was created by the macro 'container_layer', the error might have been caused by the macro implementation
ERROR: /home/flolu/Desktop/rules-ts-transitive-deps/BUILD.bazel:86:16: Analysis of target '//:node_modules_layer' failed
ERROR: Analysis of target '//:image' failed; build aborted

@thesayyn
Copy link
Member

the patch you provided doesn't seem to work for me:

sorry. updated it.

@flolu
Copy link
Author

flolu commented Jan 13, 2023

@thesayyn thanks, now the workaround works.

@flolu
Copy link
Author

flolu commented Jan 13, 2023

@thesayyn do you have any idea why this branch doesn't work even though I've patched ts_project?

@gregmagolan
Copy link
Member

gregmagolan commented Jan 24, 2023

Marking this one as a dup of aspect-build/rules_ts#268. Closing this to keep the discussion in a single place Similar pattern to here aspect-build/rules_jest#72 with a ts_project being depended on via a linked npm_package. In this case the targets are:

ts_project(
    name = "lib2_project",
    srcs = ["index.ts"],
    composite = True,
    declaration = True,
    transpiler = partial.make(
        swc,
        swcrc = "//:swcrc",
    ),
    tsconfig = "//:tsconfig",
    deps = ["//:node_modules/@faker-js/faker"],
)

npm_package(
    name = "lib2",
    srcs = [":lib2_project"],
    package = "@org/lib2",
    visibility = ["//visibility:public"],
)

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

4 participants