npm packages fail if package.json's type is set to "module"

[paullewis]

Building on #446, the value of "type" in package.json interacts badly with translated npm packages like mocha, e.g.,

load("@npm//:mocha/package_json.bzl", mocha_bin = "bin")

mocha_bin.mocha_test(
    name = "src_test",
    args = [
        "\"$(rootpath :src.test.js)\"",
    ],
    data = [
        ":src.test.js",
        "//:node_modules/mocha",
    ],
)

Will fail:

ERROR: BUILD.bazel:19:22: Running lifecycle hooks on npm package fsevents@2.3.2 failed: (Exit 1): lifecycle-hooks.sh failed: error executing command bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/external/aspect_rules_js/npm/private/lifecycle/lifecycle-hooks.sh fsevents ../../../external/npm__fsevents__2.3.2/package ... (remaining 1 argument skipped)
file:///path/__main__/bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/external/aspect_rules_js/npm/private/lifecycle/lifecycle-hooks.sh.runfiles/aspect_rules_js/npm/private/lifecycle/min/index.min.js:73

[Removed some output]

SyntaxError: Unexpected strict mode reserved word
    at ESMLoader.moduleStrategy (node:internal/modules/esm/translators:117:18)
    at ESMLoader.moduleProvider (node:internal/modules/esm/loader:337:14)
    at async link (node:internal/modules/esm/module_job:70:21)

I think something is anticipating CJS in the translated Mocha, yet if package.json's "type" value is set to "module" it gets

[gregmagolan]

Failing on lifecycle hooks? ERROR: BUILD.bazel:19:22: Running lifecycle hooks on npm package fsevents@2.3.2 failed How are lifecycle hooks involved here?

[paullewis]

That's a bit of a mystery to me - shall I make a repro?

[gregmagolan]

Sure. Looks like an interesting one.

[paullewis]

https://github.com/paullewis/repro-rules-js-issue-447

[gregmagolan]

Found a sec to look at your repro @paullewis . Thanks for putting it up.

It's an interesting one. I can get the build working with package.json's type is set to "module" in two ways. Either set

lifecycle_hooks_exclude = ["fsevents"],

on npm_translate_lock, which turns off the failing lifecycle hook, or set

lifecycle_hooks_no_sandbox = False

on npm_translate_lock, which turns on sandboxing for lifecycle hooks (it is off by default since the symlinks node_modules structure is sandbox-like and sandboxing make a build with a lot of lifecycle hooks slower.

So that leads to the question of why the fsevents lifecycle hooks fails in this mysterious way if it sees the "type": "module", in the package.json of the project, which is can outside of the sandbox.

[gregmagolan]

I also looked back at some previous work and I found that I hit a very similar issue before in the angular-ngc example we wrote https://github.com/aspect-build/bazel-examples/blob/861a7c5ab7e7073b879939b571e0420ab6c49dab/angular-ngc/WORKSPACE.bazel#L44. In that case I went with the lifecycle_hooks_no_sandbox = False work-around and didn't figure out the root cause and I didn't make the connection to "type" = "module" there.

[gregmagolan]

lifecycle_hooks_exclude = ["fsevents"] is probably the better work-around out of the two since it doesn't slow down the build and fsevents post-install seems to be optional fwict

[gregmagolan]

Hmmm. The callstack on the failure comes from

file:///private/var/tmp/_bazel_greg/8dea966c5e06921975c7e20d638b69b5/execroot/__main__/bazel-out/darwin-opt-exec-2B5CBBC6/bin/external/aspect_rules_js/npm/private/lifecycle/lifecycle-hooks.sh.runfiles/aspect_rules_js/npm/private/lifecycle/min/index.min.js

and from that point, the nearest package.json when outside of the sandbox would indeed be the package.json at the root of the execroot. This differs from lifecycle hooks under pnpm which run from within the pnpm lifecycle npm package and will hit a package.json in that package instead of finding the user's package.json. In rules_js we bundle up pnpm's lifecycle hooks library into the npm/private/lifecycle/min/index.min.js bundle and run it from there.

[gregmagolan]

Yup. That was it. The fix in rules_js is as simple as,

$ gd
diff --git a/npm/private/lifecycle/BUILD.bazel b/npm/private/lifecycle/BUILD.bazel
index 9a6bd01..a3821e2 100644
--- a/npm/private/lifecycle/BUILD.bazel
+++ b/npm/private/lifecycle/BUILD.bazel
@@ -4,7 +4,7 @@ load("//js:defs.bzl", "js_binary")
 
 js_binary(
     name = "lifecycle-hooks",
-    data = glob(["min/**"]),
+    data = ["package.json"] + glob(["min/**"]),
     entry_point = "min/index.min.js",
     visibility = ["//visibility:public"],
 )

which adds a package.json in the runfile of the lifecycle hook bundle binary. 🤷

I'll put up a PR with a regression case.

[paullewis]

Ah so the fix is to present the "correct" package.json to the lifecycle hooks node process? Makes sense. I had an unfounded hunch that it was picking up the source tree package.json from the symlink'd tree in bazel-bin or somewhere like that, but I doubt I would have found it myself!

Thanks so much for the explanation!

[gregmagolan]

Yeah. It was a sneaky one.

Outside of the sandbox, node was looking all the way up the file tree from

file:///path/__main__/bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/external/aspect_rules_js/npm/private/lifecycle/lifecycle-hooks.sh.runfiles/aspect_rules_js/npm/private/lifecycle/min/index.min.js

to

file:///path/__main__/package.json

to find the root user package.json.

With the fix it now finds one without "type" = "module" at

file:///path/__main__/bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/external/aspect_rules_js/npm/private/lifecycle/lifecycle-hooks.sh.runfiles/aspect_rules_js/npm/private/lifecycle/package.json

Fix is landed so you can pick it up from the HEAD rules_js commit.