Project build files are deleted when running under yarn workspaces

[Zetten]

🚀 feature request

Relevant Rules

yarn_install / npm_install, tested in revision v0.31.1 and d7e4554.

Description

I'm trying to add Bazel rules to an existing monorepo of npm packages, currently managed with lerna (i.e. yarn workspaces). Some of these packages have dependencies on each other, which results in symlinks inside node_modules like:

node_modules/@myorg/package1 -> ../../packages/package1
node_modules/@myorg/package2 -> ../../packages/package2
node_modules/@myorg/package3 -> ../../packages/subdir/package3

When rules_node comes along and generates BUILD.bazel files for all the contents of node_modules, it follows these symlinks and flattens any config I write myself (and leaves a _bazel marker). Not only that, but it generates broken config. Any subsequent build throws errors like:

no such package 'node_modules/object-assign': Package is considered deleted due to --deleted_packages and referenced by '//packages/package1:package1__pkg'

Describe the solution you'd like

Maybe I'm just doing something wrong, but I would expect the automatic generation of BUILD targets in the managed_directories to not touch my own workspace contents.

Describe alternatives you've considered

In a crude workaround, I've added an attribute to yarn_install/npm_install: excluded_scopes, which gets passed to generate_build_file.js. In my case all the local modules are namespaced in a single scope, so I can easily filter this whole scope out. I can file this as a PR if the approach is useful.

Another option would be to (somehow) do some dereferencing and walk back up the tree to find out if a node_modules package is linked to a directory in the original workspace. This may be more complex than a manual exclusion though.

[alexeagle]

I think the core issue is we just don't know the right way to support yarn workspaces right now. /cc @gregmagolan

[Zetten]

Ok, this might explain the next issue I'm facing 😁

ts_library targets can't resolve absolute imports to workspace-local dependencies:

ts_library(
    name = "@myorg/package1",
    ...
)

ts_library(
    name = "@myorg/package2",
    deps = ["//packages/package1:@myorg/package1"],
    ...
)

The first library compiles fine, including dependencies to external stuff from node_modules. The second throws:

packages/package2/js/Package2.tsx:11:23 - error TS2307: Cannot find module '@myorg/package1'.

11 import { Package1 } from "@myorg/package1";

A relative import of ../../pacakges/package1 works fine.

Edit: However I see that a relative path is added to paths in the generated tsconfig.json, so perhaps it's not that far from working:

      "@myorg/package1": [
        "packages/package1",
        "bazel-out/k8-fastbuild/bin/packages/package1",
        "bazel-out/k8-fastbuild/bin/packages/package1"
      ],
      "@myorg/package1/*": [
        "packages/package1/*",
        "bazel-out/k8-fastbuild/bin/packages/package1/*",
        "bazel-out/k8-fastbuild/bin/packages/package1/*"
      ],

[gregmagolan]

BUILD files are no longer generated in node_modules as part of the fix for #802. Does this solve you issue?

[Zetten]

Unfortunately, while BUILD files may not be generated, the default symlink_node_modules/always_hide_bazel_files values still cause a failure because the local workspace modules - symlinked in node_modules - still contain BUILD files:

generate_build_file.js failed: 
STDOUT:

STDERR:
npm package '@myorg/package1' from @npm yarn_install rule
has a Bazel BUILD file 'BUILD.bazel'. Use the @bazel/hide-bazel-files utility to hide these files.
See https://github.com/bazelbuild/rules_nodejs/blob/master/packages/hide-bazel-files/README.md
for installation instructions.

Adding @bazel/hide-bazel-files (latest) or setting always_hide_bazel_files = True then still renames all the workspace BUILD files to _BUILD.bazel (so the build fails).

[gregmagolan]

Can you explain your use case some more?

Are you building targets in node_modules/@myorg/package1 or is it that the BUILD are being renamed in their original location via the node_modules symlink? Are you using local file: links in your package.json? I ran into a similar symlinking BUILD file rename issue in the rules_nodejs repo with npm but not with yarn. Have you tried using both yarn_install and npm_install to see if the behavior is the same?

Also, regarding your earlier comment about the ts_library issue you're facing, you should be able to use the module_name attribute for that purpose as done here: https://github.com/bazelbuild/rules_nodejs/blob/master/e2e/ts_library/some_library/BUILD.bazel#L5

[Zetten]

The node_modules is in .bazelignore, so I'm trying to bazel build //packages/package1:package1.

The project is a monorepo comprising a bunch of React components. We'd ultimately like to be able to both 1) import the project to other bazel workspaces to assemble actual applications, and 2) build independently to push the npm_packages to a registry. Currently it's all set up with lerna to build and publish. There's a top level package.json with "workspaces": ["packages/**/*"], and each component has its own package.json with various dependencies - some local to this workspace, some external.

Because of the internal workspace dependencies, during yarn install these paths are set up as symlinks in node_modules, e.g.: node_modules/@myorg/package1 -> ../../packages/package1. I guess this is where bazel gets confused, since it thinks it's an external dependency and does its generate_build_file.js stuff.

I'll work on getting a smaller reproducing project set up, and also give it a test with npm_install.

[Zetten]

Sorry for the delay, but I've finally managed to set up the reproducing project: https://github.com/Zetten/bazelbuild_rules_nodejs_issue_871

As you suggested, I tried with npm_install, which worked fine.

The example project's WORKSPACE contains equivalent config to test both install rules (un/comment accordingly), and a detailed example in the readme.

The difference indeed seems to be that with a yarn workspace, the local packages are installed to node_modules as symlinks. Unfortunately I'm told the yarn-based workflow is essential to our UI developers, who are working with lerna and storybooks and such things which are built around the yarn workspaces feature.

(The ts_library issue was solved by module_name - I had found that on another project already, but thanks for the the reminder!)

[bjovanovic84]

I second this, yarn workspaces are essential for our UI developers as well. This used to work fine up until version 0.29.

[thesayyn]

@gregmagolan I'm running into the same issue with @Zetten faces.

Here is the use case

we have a directory with a bunch of ts files and a ts_library that builds them all with module_name attribute.
at the same time, we have another ts_library target that depends on the first ts_library that has module_name attribute.

Note: we don't add the first ts_library as a dependency in package.json

Strangely, when we run yarn_install it responds back with an error like so

npm package '@spica/client' from @npm yarn_install rule
has a Bazel BUILD file 'BUILD.bazel'. Use the @bazel/hide-bazel-files utility to hide these files.
See https://github.com/bazelbuild/rules_nodejs/blob/master/packages/hide-bazel-files/README.md

Important Note: the ts_library targets are also specified in package.json's workspaces property.

npm_install works because it has nothing to do with workspaces.

[thesayyn]

Is there any workaround for this @Zetten @gregmagolan ? Except downgrade the version

[jerome-nelson]

https://github.com/bazelbuild/rules_nodejs/blob/master/internal/npm_install/generate_build_file.js

Generates BUILD targets for all node_module entries. Since Yarn Workspace is symlinking local directories we have a boundary conflict with Bazel.

@gregmagolan Is it valid to remove workspace specific scopes from being generated as BUILD targets - since we include them as local dependencies, not external ones

FYI, @thesayyn @Zetten to test this on your own repos remove your subpackage files from:
<bazel cache dir>\external\npm\BUILD.bazel -> export_files().

[alexeagle]

We already have some logic to detect symlinks, seems easy enough to detect a symlink that points into the workspace.

[jerome-nelson]

I'm happy to take a stab at this, assuming that:

You want to edit npm_install.bzl:_create_build_files() to filter out symlinked subfolders in node_modules before calling the execute context.

[alexeagle]

yes, that sounds right.
We have this problem now with the new linker I introduced too. It symlinks into the node_modules and so our project files are getting moved around.
@jerome-nelson do you think you'll have time to do it soonish, or should someone else take it?

[bryanjlee]

@soldair Can this be merged and closed?

[Zetten]

I've finally found some time to give this another try, and while the change works to preserve BUILD(.bazel) files, it doesn't quite enable building with nested yarn workspaces.

The use of the hide flag in https://github.com/bazelbuild/rules_nodejs/blob/master/internal/npm_install/generate_build_file.ts#L478 prevents mangling the workspace-local BUILD files, but the generated @npm exports_files rule still includes every file in the local modules:

exports_files([
...
    "node_modules/@myorg/component1/BUILD.bazel",
    "node_modules/@myorg/component1/index.js",
    "node_modules/@myorg/component1/js/Component1.tsx",
    "node_modules/@myorg/component1/package.json",
    "node_modules/@myorg/component2/BUILD.bazel",
    "node_modules/@myorg/component2/index.js",
    "node_modules/@myorg/component2/js/Component2.tsx",
    "node_modules/@myorg/component2/package.json",
...

This means that there's now a package overlap. As tested with my repro project, updated to rules_nodejs 0.39.0 (and bazel 1.1.0, if that has any bearing):

ERROR: /home/pvzetten/.cache/bazel/_bazel_pvzetten/ac41c3c6cebb66363f797a83c01d882d/external/npm/BUILD.bazel:9:1: Label '@npm//:node_modules/@myorg/component1/index.js' is invalid because '@npm//node_modules/@myorg/component1' is a subpackage; perhaps you meant to put the colon here: '@npm//node_modules/@myorg/component1:index.js'?
ERROR: /home/pvzetten/.cache/bazel/_bazel_pvzetten/ac41c3c6cebb66363f797a83c01d882d/external/npm/BUILD.bazel:9:1: Label '@npm//:node_modules/@myorg/component1/js/Component1.tsx' is invalid because '@npm//node_modules/@myorg/component1' is a subpackage; perhaps you meant to put the colon here: '@npm//node_modules/@myorg/component1:js/Component1.tsx'?
...

A crude workaround would be to completely prevent indexing local symlink directories as packages, by adding another filter condition after https://github.com/bazelbuild/rules_nodejs/blob/master/internal/npm_install/generate_build_file.ts#L476.

[DylanVann]

On a project I'm working on we are using Yarn workspaces currently. We'd like to incrementally migrate some of the build to Bazel in parallel with the existing setup, but it's not possible because of this issue (the issue mentioned above by @Zetten).