rules_js icon indicating copy to clipboard operation
rules_js copied to clipboard

[FR]: link outside WORKSPACE for dependencies during local development (~= `pnpm link`)

Open sjbarag opened this issue 1 year ago • 10 comments

What is the current behavior?

Currently, rules_js fetches packages from either:

  1. A remote location (https://registry.nodejs.org/..., git://github.com/...) with the Bazel downloader
  2. The user workspace (//pkg/left-pad), which requires the package to be pulled into the workspace and its dependencies readable by npm_translate_lock.

Attempts to link outside the Bazel workspace to an NPM package located elsewhere on-disk get rejected with invalid link_package outside of the WORKSPACE.

Describe the feature

The npm link workflow is pretty common in the node community, as it allows a dev to modify a third-party package and use it without having to vendor that package.

As a contrived example motivation (replace left-pad with any other package):

  1. I've found a pathological case in left-pad performance that my application highlights
  2. I'd like to contribute a fix back to upstream left-pad. While working on that fix, I want to use the locally-changed copy in my application as a sort of "real world" gut-check in addition to the left-pad test suite. Just to make sure my pathological case is indeed getting fixed.
  3. I don't want to vendor left-pad for any number of reasons:
    1. left-pad has a complicated/obscure build system that makes it non-trivial to vendor and build with rules_js.
    2. left-pad takes bugfixes incredibly fast, so my vendored copy would only exist for a few hours after my PR goes out.

In the current state, there appears to be no way to build left-pad in a separate directory and use it in my rules_js-built package(s).

With vanilla pnpm link (or npm link or …), the workflow is roughly:

  1. pnpm --dir ~/bitbucket.org/my-org/my-app link ~/github.com/left-pad/left-pad to use a custom left-pad
  2. Change and rebuild left-pad.
  3. Change and rebuild my-app. It includes my left-pad changes.

Ideally, the workflow would be pretty similar under Bazel + rules_js. For local development it feels reasonable to break hermeticity, akin to npm_translate_lock's current use_home_npmrc), but I'd be happy to add an extra flag or two to achieve this.


Relevant slack conversation: https://bazelbuild.slack.com/archives/CEZUUKQ6P/p1689358470077099

sjbarag avatar Jul 17 '23 17:07 sjbarag

For a non-contrived motivation, included here to avoid cluttering the main description:

  1. My large Bazel-built open-source application $app has a major release every six months.
  2. That application shares UI components with a closed-source hosted webapp.
  3. That hosted webapp supports multiple major-versions of the OSS $app. I'd like those shared UI components to be versioned independently of the OSS $app, such that the shared UI components themselves support multiple major versions of $app.
  4. Keeping those shared components in a separate repo with separate history feels the most natural to achieve that, but makes local development of $app and the shared UI components very difficult.

sjbarag avatar Jul 17 '23 17:07 sjbarag

If you link a package link pnpm link lodash, the dependencies in the pnpm.lock file get modified to something like

      lodash:
        specifier: 4.17.4
        version: link:node_modules/lodash

However, rules_js doesn't like this and errors out

ERROR: .../BUILD.bazel:6:22: no such package 'node_modules/lodash': Package is considered deleted due to --deleted_packages and referenced by '//:.aspect_rules_js/node_modules/[email protected]'

Seems like this could work hermetically since linking changes the lock file. Would be really nice if we could get support for this feature, since it's a very common workflow for developers who maintain 3rd party packages in their workspaces. I'd be happy to make a PR if someone from aspect could give me a little guidance of how this might be implemented. Please let me know.

twheys avatar Jul 18 '23 15:07 twheys

I think it makes sense to allow you to do this.

We've also discussed a bazel wrapper (like Aspect CLI) could give an easier way to apply patches to third-party libraries - imagine you have edits in anything that bazel downloader fetches and could easily go from the repo where you made the edits to a patch file Bazel applies. (issue 828 in our internal monorepo)

alexeagle avatar Aug 03 '23 20:08 alexeagle

@sjbarag you should be able to use the pnpm.overrides field in the root package.json (next to pnpm-lock.yaml) with a falues like file:/some/path and then after re-running the pnpm install so that path appears in the lockfile, it should work.

I'm not sure if that actually resolves the request, or if the typical pnpm link workflow is actually more convenient than this, and so there's still something we should change in rules_js?

alexeagle avatar Aug 03 '23 23:08 alexeagle

@sjbarag you should be able to use the pnpm.overrides field in the root package.json (next to pnpm-lock.yaml) with a falues like file:/some/path and then after re-running the pnpm install so that path appears in the lockfile, it should work.

I'm not sure if that actually resolves the request, or if the typical pnpm link workflow is actually more convenient than this, and so there's still something we should change in rules_js?

Thanks, that gets pretty close! It does risk someone committing that change, but it'd of course fail in CI. Perhaps that's not as big a deal as I'd originally thought.

The main benefits for pnpm link (and anything in the npm link family are:

  1. It doesn't change any source files files, so there's no risk of accidentally committing / submitting to CI a broken package.json
  2. Changes in the host project (the Bazel one, in this case) are picked up automatically without having to rerun pnpm install because of the symlinks involved

Of course, those same symlinks can become a nightmare with Node's resolution algorithm. One can easily end up with multiple copies of react a bundle (react expects to be a singleton), etc. So it's reasonable if y'all don't want to support it for those reasons :)

sjbarag avatar Aug 04 '23 00:08 sjbarag

@sjbarag you should be able to use the pnpm.overrides field in the root package.json (next to pnpm-lock.yaml) with a falues like file:/some/path and then after re-running the pnpm install so that path appears in the lockfile, it should work.

I'm not sure if that actually resolves the request, or if the typical pnpm link workflow is actually more convenient than this, and so there's still something we should change in rules_js?

Maybe I'm misunderstanding, but I still get the "Invalid link_package outside of the WORKSPACE" error when I try this.

For context, I add the absolute path to the root of the external module in the pnpm overrides:

    "ts-client-generator": "file:/home/jacob/projects/ts-client-generator",

And then when I do pnpm install, it appears to change the absolute path into a relative one with link inside the pnpm-lock.yaml:

      ts-client-generator: link:../../../../projects/ts-client-generator

jacobgardner avatar Oct 11 '23 02:10 jacobgardner

I think maybe https://github.com/aspect-build/bazel-examples/pull/290 illustrates a solution for this? @gregmagolan is it the same issue?

alexeagle avatar Oct 11 '23 13:10 alexeagle

One big constraint here is that Bazel can't reference targets outside of the WORKSPACE so you can't just put another repo on disk and pull it into the Bazel graph. That is why an absolute path such as "ts-client-generator": "file:/home/jacob/projects/ts-client-generator" won't work under Bazel since /home/jacob/projects/ts-client-generator is presumably outside of the Bazel WORKSPACE you're building in.

You can bring other repositories on disk into a Bazel graph with local_repository in your WORKSPACE but when you do Bazel will put them under $(bazel info output_base)/external/other_repository_name. That doesn't play nicely with pnpm link since the path on disk where that lives is Bazel managed and the labels to that repository start with @other_repository_name// which can't be expressed in a package.json files or lock file since pnpm deals with paths only. In theory, you could tell rules_js that a certain relative path corresponds to an external repository such as @other_repository_name// but then you still run into issues with referencing npm_package targets in an external repository since the BUILD file there will have references to the @npm repository for that WORKSPACE to link its deps into that package which breaks things. And there is also the problem of transitive deps of that npm_package not being in the virtual store of the workspace you're building in.

Long story short, it is tricky to reproduce the pnpm link workflow under Bazel since external deps have to be brought in via the WORKSPACE with Bazel. https://github.com/aspect-build/bazel-examples/pull/290 is a POC of bringing in npm_packages from other repositories, although the use case is geared towards a permanent reference to the npm_package from the other repo vs. a temporary change for local development.

gregmagolan avatar Oct 11 '23 17:10 gregmagolan

There may be a pattern where you pnpm link a .gitignored but not .bazelignored within your WORKSPACE.

In this use case, is the dep being pnpm linked one that is built with Bazel and has a npm_package target for rules_js to reference?

gregmagolan avatar Oct 11 '23 17:10 gregmagolan