Support yarn/npm7 workspaces

[bugzpodder]

Would yarn workspaces work with this rule?
I was playing around with 0.10.0 recently but didn't get very far in trying to setup yarn_install. Would a future version support this?

[alexeagle]

@clydin from our team has been looking at yarn workspaces.

As I understand it, we have the same features as yarn workspaces without actually using it. You can have multiple directories with a package.json file, run a command to install all their dependencies, and publish individual directories to npm. But to have one package depend on another, you don't need package.json for this, as the deps of the consuming target can point directly to the output of the providing target.

[tristanz]

@alexeagle I think you'll still run into problem supporting both yarn workspaces (e.g. for interactive development with livereload) and Bazel. To support a local yarn install, you need the subpackages in your package.json, but this will break the yarn_install in WORKSPACE since those private subpackages won't be published to npm.

Do you have advice on how to support both a native yarn workflow and Bazel workflow from a monorepo with multiple packages?

[Toxicable]

@tristanz I think the idea here is that you don't isntall your sub packages with a yarn install you just reference their outputs directly

[trin0491]

If bazel supports yarn workspaces, could that be used to solve the following issue:

Assume a monorepo has two 'top-level' projects that are separately deploy able, e.g. 'app1' and 'app2' and they both depend upon a library 'lib1'. Lib1 is typescript library with a package.json that has a dependency on a third party library, e.g. lodash. Each of the applications also have package.json files with lodash but they also add another dependency, e.g. angular. The two apps must have a consistent version for lodash but they may want to have different versions for angular at times, e.g. so they can upgrade independently. Bazel supports supports multiple yarn_install/npm_install calls to create the workspaces but ts_library from app1 would prevent a dependency on lib1 because it will throw the 'All npm dependencies need to come from a single workspace" error and I don't think a single npm_install/yarn_install' workspace can contain two versions of angular so we can't use one package.json?

Is there some other solution to this or would we need the yarn workspace capability? Thanks.

[DylanVann]

There's probably a lot of people using Lerna / Yarn workspaces currently that would be interested in Bazel but need to keep workspaces working while using Bazel for some things.

This is currently blocked.

[DylanVann]

I think you'll still run into problem supporting both yarn workspaces (e.g. for interactive development with livereload) and Bazel. To support a local yarn install, you need the subpackages in your package.json, but this will break the yarn_install in WORKSPACE since those private subpackages won't be published to npm.

Assuming Bazel is delegating to Yarn how it says it is Yarn handles doing a symlink for packages within the monorepo instead of trying to download them.

[migueloller]

It looks like currently Bazel doesn't blow up when using Yarn workspaces, but the generated @npm repository will only have the top-level node_modules packages included. If there's a nested node_modules folder in a yarn workspace package, it will be ignored. For example, given the directory structure below, if app and lib both depend on a different version of some external library, Yarn will properly have the two versions, one at the top-level node_modules and one inside app or lib. The runfiles set up with the @npm repository, though, will always point to the top-level node_modules, regardless if the runfiles are declared in the packages/app Bazel package or packages/lib Bazel package. If each Bazel package got runfiles set up the same way Yarn does and let Node module resolution take care of the rest, then it should work as expected.

project
├── BUILD
├── WORKSPACE
├── package.json
├── packages
│   ├── app
│   │   ├── BUILD
│   │   ├── index.ts
│   │   └── package.json
│   └── lib
│       ├── BUILD
│       ├── index.ts
│       └── package.json
└── yarn.lock

[migueloller]

Could the Node linker be reworked so that instead of having a single node_modules in the runfiles, it has one per package? I guess this is what it would mean to "add Yarn workspaces support". It would be nice if rules_nodejs could do this by itself and use of Yarn workspaces wasn't required. Delegating to the package manager, in this case Yarn, seems more Bazel "native" though, since Bazel stays away from having multiple versions of dependencies.

@alexeagle, if there's a need for contributors I'll gladly help with getting Yarn workspaces support. My team is trying to move from Lerna + Yarn workspaces to Bazel but it's imperative that if two different apps in our repo have two different versions of some transitive dependency, it resolves properly. There's two reasons for this. First, because it would break the application if the wrong version is used of course. And second, because while keeping the same version of a shared library the same across a monorepo is definitely ideal, specially if the library is in the monorepo itself, it's sometimes not possible to make a change across the entire repo.

I more or less understand the source so I might be able to experiment with it if I'm pointed in the right direction.

[migueloller]

I looked more into this. It seems the current rules are pretty close.

# WORKSPACE

# ... prior setup snipped

yarn_install(
  name = "npm",
  package_json = "//:package.json",
  yarn_lock = "//:yarn.lock",
)

When using the repository rule above, one can depend on dependencies as follows...

# packages/lib/BUILD.bazel

ts_library(
    name = "lib",
    srcs = ["index.ts"],
    module_name = "lib",
    deps = [
        "@npm//lodash",
    ],
)

# packages/app/BUILD.bazel

nodejs_binary(
  name = "bin",
  deps = [
    "//packages/lib",
    "@npm//lodash",
  ],
  entry_point = "index.ts",
)

If the Yarn workspace "app" depends on lodash@4 but "lib" dependes on lodash@3, both packages will get v4. That being said, the runfiles at bin.sh.runfiles do contain both versions, buried under the @npm repository. All that's left is either for bin.runfiles/packages/lib/node_modules to be symlinked to bin.runfiles/npm/node_modules/packages/lib/node_modules or for the Node.js linker's require patch to do the proper mapping depending on whether the import is coming from "app" or "lib".

---

Before coming to this conclusion, I tried some things.

I tried making following change to BUILD.bazel but no luck.

 # packages/app/BUILD.bazel

 nodejs_binary(
   name = "bin",
   deps = [
+    "@npm//lib",
     "@npm//lodash",
   ],
   entry_point = "index.ts",
 )

Also, I played with the generated BUILD file's visibility to expose the @npm//lib:lib_contents target since that one includes the @npm//lib:lib__nested_node_modules target and then doing something like this:

# packages/lib/BUILD.bazel

 ts_library(
     name = "lib",
     srcs = ["index.ts"],
     module_name = "lib",
     deps = [
-        "@npm//lodash",
+        "@npm//lib:lib__contents",
     ],
 )

This will properly add the desired contents to @npm but they still won't be available in bin.sh.runfiles.

One more thing, you can always get to the right lodash version from "lib" by doing something like this:

console.log(require.resolve('../../..')) // resolves to "bin.sh.runfiles"

require('../../../npm/node_modules/lib/node_modules/lodash')

[mistic]

@alexeagle @clydin do you have any news on that topic? I have a pretty similar use case to the one mentioned by @migueloller and considering what he described in his findings it looks like what we currently have is only some step ways from what is needed. It would be really nice to finally have support for this 😃

[alexeagle]

Sorry, no update. We support multiple nested package.json but they each need to be self-contained right now. We don't even have any design thoughts yet for how we would do yarn workspaces, and it might need changes in Bazel to support properly (like with the managed_directories feature)

…

On Mon, Jun 15, 2020 at 1:47 PM Tiago Costa ***@***.***> wrote: @alexeagle <https://github.com/alexeagle> do you have any news on that topic? I have a pretty similar use case to the one mentioned by @migueloller <https://github.com/migueloller> and considering what he described in his findings it looks like what we currently have is only some step ways from what is needed. It would be really nice to finally have support for this 😃 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#266 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAALSI2XJVP7YCHSR2LH3Z3RW2CGJANCNFSM4FNOURJQ> .

[SGudbrandsson]

Sorry, no update. We support multiple nested package.json but they each need to be self-contained right now.

@alexeagle do you have a link to an example?
I'm looking to solve a similar issue and I'd love to see how smarter people solved it

[alexeagle]

Sure, this repo is an example. Multiple calls to yarn_install in the WORKSPACE file.

[migueloller]

I've spent a very long time attempting to implement a workspace rule for Yarn workspaces. My conclusion is that because with Bazel we want to be explicit about dependencies (i.e., fine-grained deps), and a yarn install with workspaces can result in multiple versions of the same package (i.e., require('lodash') could be a different version depending on the file that calls that), there have to be potentially multiple labels for each dependency. For example, if there are 2 Yarn workspaces foo and bar:

# //foo/BUILD

js_library(
  name = "foo",
  srcs = ["index.js", "package.json"],
  entry_point = "index.js",
  deps = ["@npm//foo/lodash"],
)

# //bar/BUILD

js_library(
  name = "bar",
  srcs = ["index.js", "package.json"],
  entry_point = "index.js",
  deps = ["@npm//bar/lodash"],
)

It is necessary to namespace lodash because foo and bar might depend on different versions of it. Each label can then reference the correct files, something that would not be possible otherwise (i.e., @npm//lodash isn't specific enough if there's 2 versions of lodash).

Now here's problem, though. When using js_binary the Node.js linker that patches require would have to determine which lodash to provide and this is where I get stuck. For example, Go Modules enforces the import specifier to include the major version to solve this diamond dependency issue and Rust's name mangler includes crate versions to do the same. Node.js gives you nothing, just "lodash". And unless the require patch script can somehow detect which file is calling require and then provide the appropriate version, I'm not sure how this issue can be solved. It is 100% solvable, though, because Yarn PnP does it. Perhaps the answer is in the RFC or white paper.

It might be worth mentioning that this isn't an issue with Node.js because it relies in the filesystem for its module resolution. That wouldn't be idiomatic in Bazel, though, so it doesn't seem like a viable alternative.

EDIT: Module._resolveFilename provides the parent module and that can be used to determine which version to provide. So if the module map used by require_patch.js keeps information about which paths require which versions, it's possible to solve the diamond dependency issue and make multiple version work!

@alexeagle, did you have any ideas yet for the design of a potential implementation?

[alexeagle]

Hi @migueloller
I was actually thinking about this last night and wrote a bit in #1977 about it.

I don't think we want to get into the business of understanding multiple versions nor the node APIs needed to resolve them. Rather, we want to be 100% mechanical about:

• let the user run the package manager under Bazel
• with a light touch, control the package manager environment so the stuff it installs goes into an external repository
• provide BUILD files for that repository so you can transport a subset of the files into action's execroot / test's runfiles
• make sure that by the time the node program runs, the node_modules directories on disk look just like they would have if there was no Bazel

So this means our responsibility should just be about locations on disk where the packages go, first making them different from node idiom (external repo rather than in the source tree) and then making them the same again (linker).

My theory is that if the linker should not always link a single node_modules tree into the $pwd/node_modules. Instead we should remember what subdirectory of your project had the package.json file, and link the node_modules there.
Then we are free to have multiple npm/yarn installed external repositories in the deps[] because each will get linked to a different place.
And then when node runs, it does the same thing under Bazel as it would outside of Bazel.

Does that make sense? It feels doable but it's a pretty tricky area of the code.

[migueloller]

@alexeagle, yeah that makes sense. If I'm understanding correctly: option 1 would be to patch Node.js require so that nodejs_binary can find the right dependencies at runtime. Isn't there some of that going on right now? Option 2 would be to instead put the node_modules folders in the correct places in the filesystem — likely under runfiles — to achieve the same result.

I agree with you that option 2 is probably the best way to go about this. I took another look at the linker, and if I'm understanding it correctly, does it do the filesystem changes when the binary is executed? Or said a different way, when running a nodejs_binary, before Node.js starts, there's filesystem I/O to either move or symlink node_modules into the right places?

Also, regarding the ability to reference npm/yarn dependencies from multiple external repositories, this would in a way solve the same problem that Yarn workspaces solves and is something that I personally prefer even over Yarn workspaces. It makes sense that if support for multiple package.json is added to the linker, then the node_modules folder could be put in the right place, thus allowing references to different external repositories created by npm_install/yarn_install to be deterministic.

Would you say that these are two separate features, though? One is adding support for Yarn workspaces and the other is adding support for referencing different external repositories for the npm_install and yarn_install rules. Although probably the implementations will overlap.

[alexeagle]

You understand the linker right. It adapts from Bazel semantics of separate sources/outputs/external repo back to node semantics of those all being together.

Yarn workspaces is a special case of multiple repositories and should be done second. The hard part is calling yarn install and then getting the layout in the right places for Bazel.

We should discuss in the next team meeting, ping me on slack if you want to attend

[github-actions]

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!

[alexeagle]

Now that Workspaces in npm has shipped it's more clear that this is a standard and we should understand it better, probably implement support via a new *_install repository rule.

[Aghassi]

@adam-thomas-privitar I'll discuss with @gregmagolan on monday and see what we can put out since technically it's company code at the moment 😛

[tcarrio]

I have started working on a change to support this by updating the require patch, but I see that the linker is the suggested way to manage modules now. I would like to support both options, so I'm working on this now. I would like to clarify a couple things here around the design prior to moving to far along with the implementation.

How would you see module resolution working with multiple node_modules roots? Bazel is all about hermetic builds and reproducibility is inherent with this, so there needs to be some standardization around how duplicates will be handled (allowed, prioritization, etc.).

Example

For an example. Let's say we want to define a test target to use Stencil. You could have the following in your WORKSPACE file:

# setup stencil spec test dependencies
yarn_install(
    name = "npm_stencil",
    exports_directories_only = True,
    frozen_lockfile = True,
    package_json = "//tools//stencil:dependencies/package.json",
    package_path = "sub_node_modules/stencil",
    yarn_lock = "//tools/stencil:dependencies/yarn.lock",
)

yarn_install(
    name = "npm",
    data = ["//:.npmrc"],
    exports_directories_only = True,
    frozen_lockfile = True,
    package_json = "//:package.json",
    yarn_lock = "//:yarn.lock",
)

And the following in your BUILD file to define a test using the Stencil CLI:

nodejs_test(
    name = "test",
    args = ["test", "--spec"],
    chdir = native.package_name(),
    data = [
        "@npm//rxjs",
        "@npm_stencil//@stencil/core",
        "@npm_stencil//jest",
        "@npm_stencil//jest-cli",
        "@npm_stencil//@types/jest",
        "@npm_stencil//yargs",
        ":src",
        ":stencil.config.ts",
    ],
    entry_point = stencil_cli_path,
)

Here, we will need to pull in certain libraries to test with the Stencil CLI. However, the rest of my project may be standardized on a different version of jest than Stencil requires. In tools/stencil, we have defined a package manifest for Stencil, including the core libraries for building and any required testing libraries, and installed required libraries for my web component from the root package.json as well as the stencil libraries from tools/stencil/dependencies/package.json.

Design

In this example scenario, I would expect either the linker or require_patch methodology to allow me to use these modules from separate roots.

No Duplicates

So long as there aren't duplicates, the solution is fairly straightforward.

For the module linker: Link each of the targets modules into a single root regardless of separate workspaces (which is currently checked for a failed).

For the require patch: The require_patch would simply check both roots for a module, resolving the first one found, and throwing an error in a similar fashion if the module can't be found in any workspace.

Duplicates

With duplicates, the design must make certain decisions which will have impacts to the reproducibility of a build. Supposing that we have defined a dependency, e.g. rxjs, from both @npm and @npm_stencil, how these modules will resolve is another matter. Another scenario might be dependencies of dependencies- where the child dependencies make use of varying versions of the package. Any version change can break compatibility outside of semantic versioning, and there's little control to be had over these child dependencies.

If duplicates are allowed, in order to be reproducible the operation to link these must be a bijective function, and some manner of defining order of precedence will be required. This could be inferred by order of declaration, e.g. the last dependency always wins (⍺, β -> β).

This is somewhat implicit with those using rules that may provide certain dependencies on their own, but reproducible nonetheless, based on the order of modules as given in the data for the target.

Another option is to simply disallow duplicates, which may be a more destructive or limiting approach in the case of child dependencies.

Feedback

I'm hoping to get feedback on what logic would be preferred for this. Let me know what you think of the handling of duplicates.

[molszanski]

@Aghassi and @gregmagolan, thank you for the all the tips. Will start exploring the possibilities.

[github-actions]

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!

[tony-scio]

Still humbly hoping for workspaces support and that this doesn't get auto closed after 4 years.

[Aghassi]

So my previous comment about copying back to the source tree still stands as how we do things. I'm still working with @gregmagolan @alexeagle and @joeljeske about next steps, but there is a strong possibility my team will not remain on yarn long term as we pursue things like pnpm. I'm not sure how that changes the equation, but the concept of workspaces doesn't really apply under the bazel graph it's mostly a source tree concept while you are in an in-between state as far as I know. For now, our preinstall script method has served us well, it's just cumbersome to use unfortunately.

[github-actions]

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]

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

[alexeagle]

this is available in https://github.com/aspect-build/rules_js

[adam-thomas-privitar]

Yep! And just as one data point, I switched over and it resolved everything mentioned in this thread perfectly. I would encourage everyone to switch.