aspect-build / rules_js

High-performance Bazel rules for running Node.js tools and building JavaScript projects
https://docs.aspect.build/rules/aspect_rules_js
Apache License 2.0
306 stars 106 forks source link

[FR]: Document smooth process for modifying the source code of an NPM dependency #826

Open gonzojive opened 1 year ago

gonzojive commented 1 year ago

What is the current behavior?

I'm using angular-plotly.js (an npm package) that needs some changes. Making the changes to the dependency is easy (example). There isn't an obvious, canonical process for getting those changes into a project that uses rules_js.


The way I'm doing it is by updating the npm dependency, copying all the patched files into a third_party/npm/angular-plotly.js directory, then writing some changesBUILD.bazel files:

# contents of third_party/npm/angular-plotly.js/BUILD.bazel
load("@aspect_rules_js//npm:defs.bzl", "npm_package")

# This is probably wrong. e.g., it has no deps.
npm_package(
    name = "angular-plotly.js",
    srcs = glob([
      "**/*.json",
      "**/*.d.ts",
      "**/*.js",
      "**/*.map",
    ]),
    # This is a perf improvement; the default will be flipped to False in rules_js 2.0
    include_runfiles = False,
    visibility = ["//visibility:public"],
)

The root BUILD.bazel file is just

load("@npm//:defs.bzl", "npm_link_all_packages")

npm_link_all_packages(name = "node_modules")

pnpm-workspace.yaml:

packages:
    - 'third_party/npm/angular-plotly.js'

and package.json:

{
  "dependencies": {
    "angular-plotly.js": "workspace:^4.0.5"
  }
}

Some aspects I wish were better about what I did above:

1) There are some apparent differences between the BUILD.bazel file generated by npm_import and the BUILD.bazel file one would write if defining a local NPM package. Is that right? bazel-myproj/external/myproj_npm__grpc-web__1.4.2/ uses private rules

"@generated by @aspect_rules_js//npm/private:npm_import.bzl for npm package grpc-web@1.4.2"

load("@aspect_rules_js//npm/private:npm_package_internal.bzl", _npm_package_internal = "npm_package_internal")
load("@bazel_skylib//:bzl_library.bzl", "bzl_library")

_npm_package_internal(
    name = "source_directory",
    src = ":package",
    package = "grpc-web",
    version = "1.4.2",
    visibility = ["//visibility:public"],
)

_npm_package_internal(
    name = "pkg",
    src = ":package",
    package = "grpc-web",
    version = "1.4.2",
    visibility = ["//visibility:public"],
)

I would prefer if I could copy this generated BUILD.bazel file to use as the basis of the patched up dependency.

2) I'm not sure if the change to package.json is the right way to do this. I don't personally care about pnpm compatibility as long as the project builds properly with Bazel and the IDE is fully functional. What seems more straightforward to me is adding an "overrides" feature to npm_link_all_packages that allows replacing a dependency with some local npm_package target that builds the patched-up version of the package. Less to learn for me that way.

3) I'd rather store the updated version of the library in a different repository rather than within third_party/... - likely a fork of the dependency. I'm not sure how to do this while remaining compatible with pnpm workspaces.

Describe the feature

Document a good way to make changes to an npm dependency.

I like the workflow in rules_go. A set of go_repositories is generated by gazelle. If I want to fix a bug, I'll fork the github project for the dep and update the go_repository rule to point to my fork. Pretty simple, easy to understand, not a lot of boilterplate at all. Can the process be made more like that? There is already some experimental support for gazelle-like behavior by checking in repositories.bzl. Perhaps that could be used, and an override could be made by changing some lines of a npm_import call.

Fund our work

alexeagle commented 1 year ago

The top-level workflow is just https://pnpm.io/cli/patch - but we should document this, and I think there's also a bit where rules_js needs to know about the patch files that maybe we can make smooth.

alexeagle commented 1 year ago

Possible that Aspect CLI could have patch command, which would apply to bazel modules as well (https://github.com/aspect-build/silo/issues/828 for Aspect ppl who can see that repo)