Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support pnpm link and file protocols for external packages #1328

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

twheys
Copy link

@twheys twheys commented Oct 24, 2023

Type of change

  • New feature or functionality (change which adds functionality)

Test plan

  • Covered by existing test cases (workspace: protocol tests first-party packages in workspace)
  • Need a solution for testing external packages
  • Manual testing:

I've been testing manually by setting file:/abs/path/to/3rdparty/package/dist and link:/abs/path/to/3rdparty/package/dist in my package.json. The file: protocol works without any additional stpes, but the link protocol requires that node_modules can be found in the dist folder.

Notes

This works towards #1165

Added support for using the file: and link: protocols when setting package versions in package.json and using paths to projects external to the Bazel workspace.

I've extended the npm_import rule to accept path attributes (in addition to url).

When path is set, a repository is created with the same structure as when url is set, only the external package is symlinked to it's file location, instead of downloaded.

In order to support existing behavior with the workspace: protocol, I've also added a src arg to the npm_import macro which will only create the npm import links repository, which is similar how the targets were set up previously, except the links are created in a links repository instead of the workspace node_modules targets. This isn't a necessary change, it just made the logic for generating defs.bzl and repositories.bzl a little easier to implement.

This is a draft. This change still needs some polish before it is ready to be implemented. The following are some use cases that I think would help make this change more robust:

  1. Using relative paths to external packages, ie file:../../path/to/package
    Since the package.json is copied to the npm repository before running pnpm install --lockfile-only, the paths would have to be adapted to be relative to the source repository or made absolute

  2. The end goal here would be able to support a command similar to pnpm link
    It seems to me that pnpm link is meant to be used after running pnpm install, since it modifies the lock file and the installed node_modules package. Maybe an equivalent rules_js executable could be implemented which would write some output file that is read while generating repositories.bzl and know which packages should be linked to where.

  3. The new behavior in this PR still needs documentation and tests

I'm a bit short on time for the next couple of weeks. I'll do my best to help get it to a stage where it's ready to be merged, but it might just take me a little while to make big changes. Any help and/or feedback would be greatly appreciated. I think this change would be a great improvement for rules_js because at the present time it's quite difficult for developers to work with first party dependencies outside of the workspace (in particular, those not use a Bazel build).


…col for package.json versions that point to packages external to the workspace
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

1 participant