feat: support pnpm.patchedDependencies

.aspect/rules/external_repository_action_cache/npm_translate_lock_LTE4Nzc1MDcwNjU=

@@ -1,20 +1,21 @@
 # Input hashes for repository rule npm_translate_lock(name = "npm", pnpm_lock = "//:pnpm-lock.yaml").
 # This file should be checked into version control along with the pnpm-lock.yaml file.
 .npmrc=-2065072158
-pnpm-lock.yaml=-824957789
-package.json=-2091148127
+pnpm-lock.yaml=-194549367
+package.json=325733595
 pnpm-workspace.yaml=-1988748742
+examples/npm_deps/patches/[email protected]=-442666336
 examples/js_binary/package.json=-41174383
 examples/macro/package.json=-696073599
-examples/npm_deps/package.json=-462506805
+examples/npm_deps/package.json=283109008
 examples/npm_package/libs/lib_a/package.json=-1377103079
 examples/npm_package/packages/pkg_a/package.json=-1053875011
 examples/npm_package/packages/pkg_b/package.json=-994654274
 examples/webpack_cli/package.json=1775843160
 js/private/coverage/bundle/package.json=-975746706
 js/private/worker/src/package.json=-1936685546
 npm/private/test/package.json=1727477721
-npm/private/test/npm_package/package.json=-1377103079
 npm/private/test/vendored/lodash-4.17.21.tgz=-1206623349
+npm/private/test/npm_package/package.json=-1377103079
 npm/private/test/vendored/is-odd/package.json=1041695223
 npm/private/test/vendored/semver-max/package.json=578664053

WORKSPACE

@@ -74,6 +74,7 @@ npm_translate_lock(
         "@aspect-test/[email protected]": "echo mooo >> cow.txt",
     },
     data = [
+        "//:examples/npm_deps/patches/[email protected]",
         "//:package.json",
         "//:pnpm-workspace.yaml",
         "//examples/js_binary:package.json",
@@ -141,6 +142,7 @@ npm_translate_lock(
         "@gregmagolan/test-a": ["//examples/npm_deps:patches/test-a.patch"],
         "@gregmagolan/[email protected]": ["//examples/npm_deps:patches/[email protected]"],
         "@gregmagolan/test-b": ["//examples/npm_deps:patches/test-b.patch"],
+        "[email protected]": ["//examples/npm_deps:patches/[email protected]_pnpm.patch"],
     },
     pnpm_lock = "//:pnpm-lock.yaml",
     pnpm_version = "7.25.0",

docs/pnpm.md

@@ -132,10 +132,36 @@ It is recommended to set this environment variable on CI when `update_pnpm_lock`
 
 ## Working with packages
 
-### Patching
+### Patching via pnpm.patchedDependencies
+
+Patches included in [pnpm.patchedDependencies](https://pnpm.io/next/package_json#pnpmpatcheddependencies) are automatically applied. These patches must be included in the `data` attribute of `npm_translate_lock`, for example:
+
+```json
+{
+    ...
+    "pnpm": {
+        "patchedDependencies": {
+            "[email protected]": "patches/[email protected]"
+        }
+    }
+}
+```
+
+```starlark
+npm_translate_lock(
+    ...
+    data = [
+        "//:patches/[email protected]",
+    ],
+)
+```
+
+### Patching via `patches` attribute
+
+We recommend patching via [pnpm.patchedDependencies](#patching-via-pnpmpatcheddependencies) as above, but if you are importing
+a yarn or npm lockfile and do not have this field in your package.json, you can apply additional
+patches using the `patches` and `patch_args` attributes of `npm_translate_lock`.
 
-You can apply patches to packages you fetch remotely such as from npm.
-Use the `patches` and `patch_args` attributes of `npm_translate_lock`.
 These are designed to be similar to the same-named attributes of
 [http_archive](https://bazel.build/rules/lib/repo/http#http_archive-patch_args).
 
@@ -148,10 +174,12 @@ In case multiple entries in `patches` match, the list of patches are additive.
 (More specific matches are appended to previous matches.)
 However if multiple entries in `patch_args` match, then the more specific name matches take precedence.
 
+Patches in `patches` are applied after any patches included in `pnpm.patchedDependencies`.
+
 For example,
 
 ```starlark
-npm_translate_lock (
+npm_translate_lock(
     ...
     patches = {
         "@foo/bar": ["//:patches/foo+bar.patch"],

examples/npm_deps/BUILD.bazel

@@ -252,3 +252,13 @@ js_test(
     ],
     entry_point = "case7.js",
 )
+
+#######################################
+# Case 8: patch applied via `pnpm.patchesDependencies` then by `patches`
+js_test(
+    name = "test8",
+    data = [
+        ":node_modules/meaning-of-life",
+    ],
+    entry_point = "patched-dependencies-test.js",
+)

examples/npm_deps/package.json

@@ -7,6 +7,7 @@
         "@mycorp/pkg-a": "workspace:*",
         "@rollup/plugin-commonjs": "21.1.0",
         "debug": "3.2.7",
+        "meaning-of-life": "1.0.0",
         "mobx-react": "7.3.0",
         "mobx": "6.3.0",
         "react": "17.0.2",

examples/npm_deps/patched-dependencies-test.js

@@ -0,0 +1,19 @@
+const meaningOfLife = require('meaning-of-life')
+
+// meaning-of-life should have been patched twice:
+//
+// First, by the `pnpm.patchedDependencies` patch:
+//  examples/npm_deps/patches/[email protected]
+// 42 => "forty two"
+//
+// Then by the the following patch in the `patches` attr:
+//  examples/npm_deps/patches/[email protected]_pnpm.patch
+// "forty two" => 32
+
+if (meaningOfLife === 42) {
+    throw new Error('Patches were not applied!')
+} else if (meaningOfLife === 'forty two') {
+    throw new Error('Only `pnpm.patchedDependencies` patch was applied!')
+} else if (meaningOfLife !== 32) {
+    throw new Error('Patch in `patches` was not applied!')
+}

examples/npm_deps/patches/meaning-of-life@1.0.0-after_pnpm.patch

@@ -0,0 +1,7 @@
+diff --git a/index.js b/index.js
+index a8653a9c9264ca1ac9fd2acb6c523a321912ab33..ae967de93c7c12074a686e1e8437b7b2344108be 100644
+--- a/index.js
++++ b/index.js
+@@ -1 +1 @@
+-module.exports = "forty two"
++module.exports = 32

examples/npm_deps/patches/meaning-of-life@1.0.0-pnpm.patch

@@ -0,0 +1,7 @@
+diff --git a/index.js b/index.js
+index a8653a9c9264ca1ac9fd2acb6c523a321912ab33..ae967de93c7c12074a686e1e8437b7b2344108be 100644
+--- a/index.js
++++ b/index.js
+@@ -1 +1 @@
+-module.exports = 42
++module.exports = "forty two"

examples/npm_deps/patches/patches

@@ -1,3 +1,5 @@
+examples/npm_deps/patches/[email protected]_pnpm.patch
+examples/npm_deps/patches/[email protected]
 examples/npm_deps/patches/test-a.patch
 examples/npm_deps/patches/[email protected]
 examples/npm_deps/patches/test-b.patch

npm/extensions.bzl

@@ -44,14 +44,14 @@ def _extension_impl(module_ctx):
             if not attr.pnpm_lock:
                 continue
 
-            lock_importers, lock_packages = utils.parse_pnpm_lock(module_ctx.read(attr.pnpm_lock))
+            lock_importers, lock_packages, lock_patched_dependencies = utils.parse_pnpm_lock(module_ctx.read(attr.pnpm_lock))
             importers, packages = translate_to_transitive_closure(lock_importers, lock_packages, attr.prod, attr.dev, attr.no_optional)
             registries = {}
             npm_auth = {}
             if attr.npmrc:
                 npmrc = parse_npmrc(module_ctx.read(attr.npmrc))
                 (registries, npm_auth) = npm_translate_lock_helpers.get_npm_auth(npmrc, module_ctx.path(attr.npmrc), module_ctx.os.environ)
-            imports = npm_translate_lock_helpers.gen_npm_imports(importers, packages, attr.pnpm_lock.package, attr.name, attr, registries, utils.default_registry(), npm_auth)
+            imports = npm_translate_lock_helpers.gen_npm_imports(importers, packages, lock_patched_dependencies, attr.pnpm_lock.package, attr.name, attr, registries, utils.default_registry(), npm_auth)
             for i in imports:
                 npm_import(
                     name = i.name,

npm/npm_import.bzl

@@ -180,6 +180,8 @@ def npm_translate_lock(
         patches: A map of package names or package names with their version (e.g., "my-package" or "[email protected]")
             to a label list of patches to apply to the downloaded npm package. Multiple matches are additive.
 
+            These patches are applied after any patches in [pnpm.patchedDependencies](https://pnpm.io/next/package_json#pnpmpatcheddependencies).
+
             Read more: [patching](/docs/pnpm.md#patching)
 
         patch_args: A map of package names or package names with their version (e.g., "my-package" or "[email protected]")

npm/private/npm_translate_lock.bzl

@@ -69,7 +69,7 @@ def _impl(rctx):
 
     gen_helpers.verify_node_modules_ignored(rctx, state.importers(), state.root_package())
 
-    helpers.verify_patches(rctx, state.label_store)
+    helpers.verify_patches(rctx, state)
 
     rctx.report_progress("Translating {}".format(state.label_store.relative_path("pnpm_lock")))
 
@@ -88,6 +88,7 @@ def _impl(rctx):
         state.label_store.label("pnpm_lock"),
         importers,
         packages,
+        state.patched_dependencies(),
         state.root_package(),
         state.default_registry(),
         state.npm_registries(),

npm/private/npm_translate_lock_generate.bzl

@@ -252,7 +252,7 @@ def _get_npm_auth(npmrc, npmrc_path, environ):
     return (registries, auth)
 
 ################################################################################
-def _gen_npm_imports(importers, packages, root_package, rctx_name, attr, registries, default_registry, npm_auth):
+def _gen_npm_imports(importers, packages, patched_dependencies, root_package, rctx_name, attr, registries, default_registry, npm_auth):
     "Converts packages from the lockfile to a struct of attributes for npm_import"
 
     if attr.prod and attr.dev:
@@ -295,6 +295,7 @@ def _gen_npm_imports(importers, packages, root_package, rctx_name, attr, registr
         optional_deps = package_info.get("optional_dependencies")
         dev = package_info.get("dev")
         optional = package_info.get("optional")
+        pnpm_patched = package_info.get("patched")
         requires_build = package_info.get("requires_build")
         transitive_closure = package_info.get("transitive_closure")
         resolution = package_info.get("resolution")
@@ -344,13 +345,34 @@ def _gen_npm_imports(importers, packages, root_package, rctx_name, attr, registr
         # gather patches & patch args
         patches, patches_keys = _gather_values_from_matching_names(True, attr.patches, name, friendly_name, unfriendly_name)
 
+        # Apply patch from `pnpm.patchedDependencies` first
+        if pnpm_patched:
+            patch_path = "//:%s" % patched_dependencies.get(friendly_name).get("path")
+            patches.insert(0, patch_path)
+
         # Prevent the patch string labels from going through further repo mapping:
         # https://docs.google.com/document/d/1N81qfCa8oskCk5LqTW-LNthy6EBrDot7bdUsjz6JFC4/
         # Further, prepend the optional '@' for earlier versions of Bazel so that checked in
         # repositories.bzl files don't fail diff tests when run under multiple versions of Bazel.
         patches = [("" if utils.bzlmod_supported else "@") + str(attr.pnpm_lock.relative(patch)) for patch in patches]
 
         patch_args, _ = _gather_values_from_matching_names(False, attr.patch_args, "*", name, friendly_name, unfriendly_name)
+
+        # Patches in `pnpm.patchedDependencies` must have the -p1 format. Therefore any
+        # patches applied via `patches` must also use -p1 since we don't support different
+        # patch args for different patches.
+        if pnpm_patched and not _has_strip_prefix_arg(patch_args, 1):
+            if _has_strip_prefix_arg(patch_args):
+                msg = """\
+ERROR: patch_args for package {package} contains a strip prefix that is incompatible with a patch applied via `pnpm.patchedDependencies`.
+
+`pnpm.patchedDependencies` requires a strip prefix of `-p1`. All applied patches must use the same strip prefix.
+
+""".format(package = friendly_name)
+                fail(msg)
+            patch_args = patch_args[:]
+            patch_args.append("-p1")
+
         patches_used.extend(patches_keys)
 
         # gather custom postinstalls
@@ -576,7 +598,7 @@ or disable this check by setting `verify_node_modules_ignored = None` in `npm_tr
             fail(msg)
 
 ################################################################################
-def _generate_repository_files(rctx, pnpm_lock_label, importers, packages, root_package, default_registry, npm_registries, npm_auth, link_workspace):
+def _generate_repository_files(rctx, pnpm_lock_label, importers, packages, patched_dependencies, root_package, default_registry, npm_registries, npm_auth, link_workspace):
     generated_by_lines = [
         "\"\"\"@generated by npm_translate_lock(name = \"{}\", pnpm_lock = \"{}\")\"\"\"".format(_to_apparent_repo_name(rctx.name), utils.consistent_label_str(pnpm_lock_label)),
         "",  # empty line after bzl docstring since buildifier expects this if this file is vendored in
@@ -594,7 +616,7 @@ def _generate_repository_files(rctx, pnpm_lock_label, importers, packages, root_
     defs_bzl_header = ["""# buildifier: disable=bzl-visibility
 load("@aspect_rules_js//js:defs.bzl", _js_library = "js_library")"""]
 
-    npm_imports = _gen_npm_imports(importers, packages, root_package, rctx.name, rctx.attr, npm_registries, default_registry, npm_auth)
+    npm_imports = _gen_npm_imports(importers, packages, patched_dependencies, root_package, rctx.name, rctx.attr, npm_registries, default_registry, npm_auth)
 
     fp_links = {}
     rctx_files = {
@@ -1004,6 +1026,14 @@ load("@aspect_rules_js//npm/private:npm_package_store.bzl", _npm_package_store =
     for filename, contents in rctx_files.items():
         rctx.file(filename, "\n".join(generated_by_lines + contents))
 
+def _has_strip_prefix_arg(patch_args, strip_num = None):
+    if strip_num != None:
+        return "-p%d" % strip_num in patch_args or "--strip=%d" % strip_num in patch_args
+    for arg in patch_args:
+        if arg.startswith("-p") or arg.startswith("--strip="):
+            return True
+    return False
+
 helpers = struct(
     # TODO(cleanup): move non-generation helpers out of this file in a follow-up PR
     to_apparent_repo_name = _to_apparent_repo_name,

npm/private/npm_translate_lock_helpers.bzl

@@ -2,34 +2,31 @@
 
 load("@bazel_skylib//lib:new_sets.bzl", "sets")
 
-def _verify_patches(rctx, label_store):
+def _verify_patches(rctx, state):
     if rctx.attr.verify_patches and rctx.attr.patches != None:
-        rctx.report_progress("Verifying patches in {}".format(label_store.relative_path("verify_patches")))
+        rctx.report_progress("Verifying patches in {}".format(state.label_store.relative_path("verify_patches")))
 
         # Patches in the patch list verification file
-        verify_patches_content = rctx.read(label_store.label("verify_patches")).strip(" \t\n\r")
+        verify_patches_content = rctx.read(state.label_store.label("verify_patches")).strip(" \t\n\r")
         verify_patches = sets.make(verify_patches_content.split("\n"))
 
-        # Patches declared in the `patches` attr
-        declared_patch_count = 0
-        for pkg_patches in rctx.attr.patches.values():
-            declared_patch_count += len(pkg_patches)
-        declared_patches = sets.make([label_store.relative_path("patches_%d" % i) for i in range(declared_patch_count)])
+        # Patches in `patches` or `pnpm.patchedDependencies`
+        declared_patches = sets.make([state.label_store.relative_path("patches_%d" % i) for i in range(state.num_patches())])
 
         if not sets.is_subset(verify_patches, declared_patches):
             missing_patches = sets.to_list(sets.difference(verify_patches, declared_patches))
             missing_patches_formatted = "\n".join(["- %s" % path for path in missing_patches])
             fail("""
 ERROR: in verify_patches:
 
-The following patches were found in {patches_list} but were not listed
-in the `patches` attribute of `npm_translate_lock`.
+The following patches were found in {patches_list} but were not listed in the 
+`patches` attribute of `npm_translate_lock` or in `pnpm.patchedDependencies`.
 
 {missing_patches}
 
 To disable this check, remove the `verify_patches` attribute from `npm_translate_lock`.
 
-""".format(patches_list = label_store.relative_path("verify_patches"), missing_patches = missing_patches_formatted))
+""".format(patches_list = state.label_store.relative_path("verify_patches"), missing_patches = missing_patches_formatted))
 
 helpers = struct(
     verify_patches = _verify_patches,