rules_nodejs icon indicating copy to clipboard operation
rules_nodejs copied to clipboard

Add yarn_run/npm_run rules / Add node_modules to execroot

Open elado opened this issue 3 years ago • 10 comments

🚀 feature request

Relevant Rules

Add new rules - yarn_run and npm_run

Description

I would like to revive https://github.com/bazelbuild/rules_nodejs/issues/362 and suggest adding yarn_run / npm_run rules, and/or symlink node_modules in execroot sandbox.

While this may not be "the bazel way", I believe it'll be tremendously helpful with migration of existing projects that rely heavily on package scripts.

The major gap I see currently is the lack of presence of node_modules in the current execution directory.

Describe the solution you'd like

In a genrule, I can access yarn binary using $(execpath @nodejs//:yarn), and execute package scripts like

genrule(
    name = "foo",
    outs = [...],
    cmd = """
        $(execpath @nodejs//:yarn) script-from-package-json
    """,
    tools = [
        "@nodejs//:yarn",
        "@npm//:node_modules",
        "package.json",
        ...
    ],
)

However, any binary access that my package.json scripts rely on cannot be found because the CWD where yarn is running is in a sandbox execroot which doesn't contain node_modules:

[tmpdir]/sandbox/[os]-sandbox/[id]/execroot/[project]

So scripts like "build": "tsc index.ts --outDir .." can't find tsc.

The node_modules are symlinked in [tmpdir]/external/npm/node_modules.

I can think of two possible solutions for yarn_run/npm_run rules:

  1. Add external/npm/node_modules/.bin to the PATH before executing the script.
  2. Symlink node_modules in the sandbox. I believe this is more of a bulletproof solution, because I've seen some node modules that rely on file system and try to access the closes node_modules in the tree. Also, with workspaces, there could be multiple nested node_modules, so the .bin depends on where a script is executed from (chdir usage), so if this is the approach the tree need to be rebuilt accordingly.

As a POC, manual symlink of node_modules into the sandbox execroot works (is there a more resilient way to get to external/npm/node_modules?)

genrule(
    name = "build",
    outs = [..],
    cmd = """
        ln -s ../../../../../external/npm/node_modules .

        $(execpath @nodejs//:yarn) build
    """,
    tools = [
        "@nodejs//:yarn",
        "@npm//:node_modules",
        "package.json",
        "index.ts",
    ],
)

Missing node_modules/.bin in sh_test sandbox

Additionally, when running a test (sh_test), it's sandboxed in a different folder. When specifying packages (rather than @npm//:node_modules) they are symlinked individually, but the .bin folder is missing.

sandbox/darwin-sandbox/[id]/execroot/[project]/bazel-out/darwin-fastbuild/bin/[test_name].runfiles/npm/node_modules

Ideally .bin is also available for yarn (maybe @npm//.bin/tsc) so it can execute scripts by name.

Passing locations to scripts

With all that, another thing that *_run requires is to pass the location of the expected outs. Env vars could help, but it's still not ideal:

// package.json
"build": "tsc index.ts --outDir ${TS_OUT_DIR:-lib}"
yarn_run(
    name = "build",
    outs = ["lib/index.js"],
    script_name = "build",
    env = [{ "TS_OUT_DIR": "$(location lib/index.js)" }],
    tools = [
        "@nodejs//:yarn",
        "@npm//:node_modules",
        "package.json",
        "index.ts",
    ],
)

Describe alternatives you've considered

  • Create a genrule and symlink node_modules into sandbox execroot
  • Add node_modules/.bin to PATH with something like export PATH=path/to/node_modules/.bin:$$PATH

elado avatar Mar 05 '21 18:03 elado

I totally agree we should have this - we aren't opinionated about "the bazel way" in rules_nodejs.

alexeagle avatar Mar 07 '21 01:03 alexeagle

This issue has been automatically marked as stale because it has not had any activity for 90 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

github-actions[bot] avatar Jun 08 '21 03:06 github-actions[bot]

bump

ljharb avatar Jun 08 '21 04:06 ljharb

This issue has been automatically marked as stale because it has not had any activity for 90 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

github-actions[bot] avatar Sep 08 '21 02:09 github-actions[bot]

bump

ljharb avatar Sep 08 '21 02:09 ljharb

This issue has been automatically marked as stale because it has not had any activity for 90 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

github-actions[bot] avatar Dec 08 '21 02:12 github-actions[bot]

bump

ljharb avatar Dec 08 '21 02:12 ljharb

can we use this rule?

softshipper avatar Feb 11 '22 21:02 softshipper

@softshipper this is a feature request, no such rule exists. Help wanted :)

alexeagle avatar Feb 13 '22 21:02 alexeagle

+1 to this issue I think it would be massive to be able to do this

shadogar avatar Apr 05 '22 22:04 shadogar

This issue has been automatically marked as stale because it has not had any activity for 6 months. It will be closed if no further activity occurs in 30 days. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

github-actions[bot] avatar Oct 06 '22 03:10 github-actions[bot]

This issue was automatically closed because it went 30 days without any activity since it was labeled "Can Close?"

github-actions[bot] avatar Nov 06 '22 03:11 github-actions[bot]