rules_nodejs icon indicating copy to clipboard operation
rules_nodejs copied to clipboard

yarn_versions helper only includes v1

Open Karibash opened this issue 4 years ago • 6 comments

🚀 Yarn Berry Support

Relevant Rules

yarn_install

Description

When trying to use Yarn berry with the yarn_install rule, an error occurs because it implicitly sets the --mutex network, which is not supported by Yarn Berry. The following implementation shows that the mutex option can be disabled by setting the use_global_yarn_cache to false, but the --cache-folder is implicitly set next. https://github.com/bazelbuild/rules_nodejs/blob/aa9882b9d5afaeb9815a3630017fa2911eeedb0a/internal/npm_install/npm_install.bzl#L825-L834

In Yarn Berry, setting the --cache-folder in the CLI is deprecated, so an error will occur in this case as well. https://github.com/yarnpkg/berry/blob/83311e1d4dec7fb6c0ead21f37efa9b687be58b9/packages/plugin-essentials/sources/commands/install.ts#L211-L219

The following is an example of WORKSPACE.bazel for using Yarn Berry.

http_archive(
    name = "vendored_yarn_3_1_0",
    build_file_content = """exports_files(["vendored_yarn_3_1_0/berry--yarnpkg-cli-3.1.0/packages/yarnpkg-cli/bin/yarn.js"])""",
    urls = ["https://github.com/yarnpkg/berry/archive/refs/tags/@yarnpkg/cli/3.1.0.tar.gz"],
)

load("@build_bazel_rules_nodejs//:index.bzl", "node_repositories")
node_repositories(
    vendored_yarn = "@vendored_yarn_3_1_0//:berry--yarnpkg-cli-3.1.0/packages/yarnpkg-cli",
)

load("@build_bazel_rules_nodejs//:index.bzl", "yarn_install")
yarn_install(
    name = "npm",
    args = ["--immutable"],
    frozen_lockfile = False,
    use_global_yarn_cache = False,
    data = [
      "@//:.yarnrc.yml",
      "@vendored_yarn_3_1_0//:berry--yarnpkg-cli-3.1.0/packages/yarnpkg-cli/bin/yarn.js",
    ],
    package_json = "//:package.json",
    yarn_lock = "//:yarn.lock",
)

Describe the solution you'd like

The ideas suggested in the issue comments below are helpful.

  1. add a flag to remove the automatic addition of --mutex network
  2. add a bool attribute yarn2_compatible that does not use --mutex network and replaces --frozen-lockfile with --immutable
  3. detect that yarn2 is used and behave accordingly

https://github.com/bazelbuild/rules_nodejs/issues/1599#issuecomment-816480044

Personally, I think the 1st idea is better, as it seems to be easy to implement without destructive changes. An example is shown below.

yarn_install(
    name = "npm",
    args = ["--immutable"],
    frozen_lockfile = False,
-   use_global_yarn_cache = False,
+   use_mutex = False,
    data = [
      "@//:.yarnrc.yml",
      "@vendored_yarn_3_1_0//:berry--yarnpkg-cli-3.1.0/packages/yarnpkg-cli/bin/yarn.js",
    ],
    package_json = "//:package.json",
    yarn_lock = "//:yarn.lock",
)

Karibash avatar Nov 10 '21 15:11 Karibash

https://github.com/bazelbuild/rules_nodejs/blob/d2f9c4137543c3ed8477ca3aa73b2aa0fcbd7fe9/nodejs/private/yarn_versions.bzl while that comment still refers to an open issue, the list of supported yarn versions could be extended now.

But that could require modifications of the download functions, as yarn berry is available as https://github.com/yarnpkg/berry/raw/%40yarnpkg/cli/{version}/packages/yarnpkg-cli/bin/yarn.js as a single *.js file.

lukasoyen avatar Jan 14 '22 18:01 lukasoyen

I noticed from the example above that https://github.com/yarnpkg/berry/archive/refs/tags/@yarnpkg/cli/3.1.0.tar.gz is over 150MB file so I don't think that's quite the right workaround either.

alexeagle avatar Jan 14 '22 20:01 alexeagle

Re-opening this issue to get the yarn_versions builtin to support more versions.

alexeagle avatar Jan 14 '22 20:01 alexeagle

@alexeagle I see you made a PR for this and closed it. Would you mind explaining the current status on this please?

friendly ping?

sgammon avatar May 05 '22 09:05 sgammon

I checked out that pr again, and commented there. It doesn't actually work, and I don't have a client who needs it. Contributions welcome of course 😁

alexeagle avatar May 05 '22 15:05 alexeagle

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 Nov 05 '22 03:11 github-actions[bot]

Just to wrap this up, rules_nodejs is no longer maintained and rules_js standardized on pnpm because it's lockfile allows us to port the node_modules layout to starlark, which allows lazy install of only needed packages for the requested build and test targets

alexeagle avatar Nov 05 '22 16:11 alexeagle

dumb question but that means pnpm is the way to go and yarn berry won't be supported, right? @alexeagle

sgammon avatar Nov 08 '22 03:11 sgammon

that does sound like a major gain

sgammon avatar Nov 08 '22 03:11 sgammon

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 May 12 '23 02:05 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 Jun 11 '23 02:06 github-actions[bot]