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

feat: support pnpm.onlyBuiltDependencies #1723

Merged
merged 1 commit into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions docs/pnpm.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,13 @@ See our [blog post](https://blog.aspect.dev/easier-merges-on-lockfiles) for a lo

First, mark the `npm_translate_lock_<hash>` file (with `<hash>` replaced with the hash generated in your workspace)
to use a custom custom merge driver, in this example named `ours`:

```
.aspect/rules/external_repository_action_cache/npm_translate_lock_<hash>= merge=ours
```

Second, developers must define the `ours` custom merge driver in their git configuration to always accept local change:

```
git config --global merge.ours.driver true
```
Expand Down Expand Up @@ -258,8 +260,16 @@ npm packages have "lifecycle scripts" such as `postinstall` which are documented

We refer to these as "lifecycle hooks".

> You can disable this feature completely by setting all packages to have no hooks, using
> `lifecycle_hooks = { "*": [] }` in `npm_translate_lock`.
The lifecycle hooks of a package are determined by the `package.json` [`pnpm.onlyBuiltDependencies` attribute](https://pnpm.io/package_json#pnpmonlybuiltdependencies).

If `pnpm.onlyBuiltDependencies` is unspecified `npm_translate_lock` will fallback to the legacy pnpm lockfile `requiresBuild` attribute.
This attribute is only available in pnpm _before_ v9, see [pnpm #7707](https://github.com/pnpm/pnpm/issues/7707) for reasons why this attribute was removed.

When a package has lifecycle hooks the `lifecycle_*` attributes are applied to filter which hooks are run and how they are run.

For example, you can restrict lifecycle hooks across all packages to only run `postinstall`:

> `lifecycle_hooks = { "*": ["postinstall"] }` in `npm_translate_lock`.
gregmagolan marked this conversation as resolved.
Show resolved Hide resolved

Because rules_js models the execution of these hooks as build actions, rather than repository rules,
the result can be stored in the remote cache and shared between developers.
Expand Down
12 changes: 3 additions & 9 deletions e2e/pnpm_lockfiles/.bazelignore
Original file line number Diff line number Diff line change
@@ -1,13 +1,7 @@
node_modules/
projects/a/node_modules
projects/b/node_modules
projects/c/node_modules
v54/node_modules/
v60/node_modules/
v61/node_modules/
v54/projects/a/node_modules
v54/projects/b/node_modules
v54/projects/c/node_modules
v60/projects/a/node_modules
v60/projects/b/node_modules
v60/projects/c/node_modules
v61/projects/a/node_modules
v61/projects/b/node_modules
v61/projects/c/node_modules
2 changes: 1 addition & 1 deletion e2e/pnpm_lockfiles/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1 +1 @@
exports_files(["patched-dependencies-test.js"])
exports_files(["base/patched-dependencies-test.js"])
5 changes: 1 addition & 4 deletions e2e/pnpm_lockfiles/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,9 @@ npm = use_extension(
npm.npm_translate_lock(
name = "lock-%s" % version,
data = [
"//:package.json",
"//%s:package.json" % version,
"//%s:patches/meaning-of-life@1.0.0-pnpm.patch" % version,
"//:pnpm-workspace.yaml",
],
npmrc = "//:.npmrc",
patch_args = {"*": ["-p1"]},
pnpm_lock = "//%s:pnpm-lock.yaml" % version,
verify_node_modules_ignored = "//:.bazelignore",
)
Expand Down
5 changes: 1 addition & 4 deletions e2e/pnpm_lockfiles/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,9 @@ load("@aspect_rules_js//npm:repositories.bzl", "npm_translate_lock")
npm_translate_lock(
name = "lock-%s" % version,
data = [
"//:package.json",
"//%s:package.json" % version,
"//%s:patches/meaning-of-life@1.0.0-pnpm.patch" % version,
"//:pnpm-workspace.yaml",
],
npmrc = "//:.npmrc",
patch_args = {"*": ["-p1"]},
pnpm_lock = "//%s:pnpm-lock.yaml" % version,
verify_node_modules_ignored = "//:.bazelignore",
)
Expand Down
File renamed without changes.
File renamed without changes.
3 changes: 3 additions & 0 deletions e2e/pnpm_lockfiles/base/pnpm-workspace.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
packages:
- '.'
- '../projects/*'
30 changes: 14 additions & 16 deletions e2e/pnpm_lockfiles/lockfile-test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
Test utils for lockfiles
"""

load("@bazel_skylib//rules:build_test.bzl", "build_test")
load("@aspect_rules_js//js:defs.bzl", "js_test")
load("@aspect_bazel_lib//lib:copy_file.bzl", "copy_file")
load("@aspect_rules_js//js:defs.bzl", "js_test")
load("@bazel_skylib//rules:build_test.bzl", "build_test")

def lockfile_test(name = "lockfile", node_modules = "node_modules"):
"""
Expand All @@ -15,21 +15,19 @@ def lockfile_test(name = "lockfile", node_modules = "node_modules"):
node_modules: name of the tested 'node_modules' directory
"""

# TODO: lockfile v53 does not support patching?
if native.package_name() != "v53":
copy_file(
name = "copy-tests",
src = "//:patched-dependencies-test.js",
out = "patched-dependencies-test.js",
)
copy_file(
name = "copy-tests",
src = "//:base/patched-dependencies-test.js",
out = "patched-dependencies-test.js",
)

js_test(
name = "patch-test",
data = [
":%s/meaning-of-life" % node_modules,
],
entry_point = "patched-dependencies-test.js",
)
js_test(
name = "patch-test",
data = [
":%s/meaning-of-life" % node_modules,
],
entry_point = "patched-dependencies-test.js",
)

build_test(
name = name,
Expand Down
3 changes: 0 additions & 3 deletions e2e/pnpm_lockfiles/pnpm-workspace.yaml

This file was deleted.

6 changes: 3 additions & 3 deletions e2e/pnpm_lockfiles/setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
# 6.0 - pnpm v8.0.0 bumped the lockfile version to 6.0; this included breaking changes
# 6.1 - pnpm v8.6.0 bumped the lockfile version to 6.1

mv v54/pnpm-lock.yaml . && npx pnpm@^7.0 install --lockfile-only && mv pnpm-lock.yaml v54/
mv v54/pnpm-lock.yaml base && pushd base && npx pnpm@^7.0 install --lockfile-only && mv pnpm-lock.yaml ../v54/ && popd

# pnpm v8.0.0 bumped the lockfile version to 6.0, 8.6.0 bumped it to 6.1 which was then reverted to 6.0
# while still presenting minor differences from <8.6.0.
mv v60/pnpm-lock.yaml . && npx pnpm@8.5.1 install --lockfile-only && mv pnpm-lock.yaml v60/
mv v61/pnpm-lock.yaml . && npx pnpm@8.6.0 install --lockfile-only && mv pnpm-lock.yaml v61/
mv v60/pnpm-lock.yaml base && pushd base && npx pnpm@8.5.1 install --lockfile-only && mv pnpm-lock.yaml ../v60/ && popd
mv v61/pnpm-lock.yaml base && pushd base && npx pnpm@8.6.0 install --lockfile-only && mv pnpm-lock.yaml ../v61/ && popd
1 change: 1 addition & 0 deletions e2e/pnpm_lockfiles/v54/package.json
2 changes: 1 addition & 1 deletion e2e/pnpm_lockfiles/v54/patches
6 changes: 3 additions & 3 deletions e2e/pnpm_lockfiles/v54/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions e2e/pnpm_lockfiles/v60/package.json
2 changes: 1 addition & 1 deletion e2e/pnpm_lockfiles/v60/patches
6 changes: 3 additions & 3 deletions e2e/pnpm_lockfiles/v60/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions e2e/pnpm_lockfiles/v61/package.json
2 changes: 1 addition & 1 deletion e2e/pnpm_lockfiles/v61/patches
6 changes: 3 additions & 3 deletions e2e/pnpm_lockfiles/v61/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions npm/extensions.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ WARNING: Cannot determine home directory in order to load home `.npmrc` file in
importers = importers,
packages = packages,
patched_dependencies = state.patched_dependencies(),
only_built_dependencies = state.only_built_dependencies(),
root_package = attr.pnpm_lock.package,
rctx_name = attr.name,
attr = attr,
Expand Down
1 change: 1 addition & 0 deletions npm/private/npm_translate_lock.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ See https://github.com/aspect-build/rules_js/issues/1445
importers,
packages,
state.patched_dependencies(),
state.only_built_dependencies(),
state.root_package(),
state.default_registry(),
state.npm_registries(),
Expand Down
4 changes: 2 additions & 2 deletions npm/private/npm_translate_lock_generate.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,11 @@ _PACKAGE_JSON_BZL_FILENAME = "package_json.bzl"
_RESOLVED_JSON_FILENAME = "resolved.json"

# buildifier: disable=function-docstring
def generate_repository_files(rctx, pnpm_lock_label, importers, packages, patched_dependencies, root_package, default_registry, npm_registries, npm_auth, link_workspace):
def generate_repository_files(rctx, pnpm_lock_label, importers, packages, patched_dependencies, only_built_dependencies, root_package, default_registry, npm_registries, npm_auth, link_workspace):
# empty line after bzl docstring since buildifier expects this if this file is vendored in
generated_by_prefix = "\"\"\"@generated by npm_translate_lock(name = \"{}\", pnpm_lock = \"{}\")\"\"\"\n".format(helpers.to_apparent_repo_name(rctx.name), str(pnpm_lock_label))

npm_imports = helpers.get_npm_imports(importers, packages, patched_dependencies, root_package, rctx.name, rctx.attr, rctx.attr.lifecycle_hooks, rctx.attr.lifecycle_hooks_execution_requirements, rctx.attr.lifecycle_hooks_use_default_shell_env, npm_registries, default_registry, npm_auth)
npm_imports = helpers.get_npm_imports(importers, packages, patched_dependencies, only_built_dependencies, root_package, rctx.name, rctx.attr, rctx.attr.lifecycle_hooks, rctx.attr.lifecycle_hooks_execution_requirements, rctx.attr.lifecycle_hooks_use_default_shell_env, npm_registries, default_registry, npm_auth)

link_packages = [helpers.link_package(root_package, import_path) for import_path in importers.keys()]

Expand Down
5 changes: 3 additions & 2 deletions npm/private/npm_translate_lock_helpers.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ def _select_npm_auth(url, npm_auth):
return npm_auth_bearer, npm_auth_basic, npm_auth_username, npm_auth_password

################################################################################
def _get_npm_imports(importers, packages, patched_dependencies, root_package, rctx_name, attr, all_lifecycle_hooks, all_lifecycle_hooks_execution_requirements, all_lifecycle_hooks_use_default_shell_env, registries, default_registry, npm_auth):
def _get_npm_imports(importers, packages, patched_dependencies, only_built_dependencies, root_package, rctx_name, attr, all_lifecycle_hooks, all_lifecycle_hooks_execution_requirements, all_lifecycle_hooks_use_default_shell_env, registries, default_registry, npm_auth):
"Converts packages from the lockfile to a struct of attributes for npm_import"
if attr.prod and attr.dev:
fail("prod and dev attributes cannot both be set to true")
Expand Down Expand Up @@ -393,7 +393,8 @@ ERROR: can not apply both `pnpm.patchedDependencies` and `npm_translate_lock(pat
elif name not in link_packages[public_hoist_package]:
link_packages[public_hoist_package].append(name)

if requires_build:
run_lifecycle_hooks = name in only_built_dependencies if only_built_dependencies != None else requires_build
if run_lifecycle_hooks:
lifecycle_hooks, _ = _gather_values_from_matching_names(False, all_lifecycle_hooks, "*", name, friendly_name, unfriendly_name)
lifecycle_hooks_env, _ = _gather_values_from_matching_names(True, attr.lifecycle_hooks_envs, "*", name, friendly_name, unfriendly_name)
lifecycle_hooks_execution_requirements, _ = _gather_values_from_matching_names(False, all_lifecycle_hooks_execution_requirements, "*", name, friendly_name, unfriendly_name)
Expand Down
13 changes: 11 additions & 2 deletions npm/private/npm_translate_lock_state.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ WARNING: `update_pnpm_lock` attribute in `npm_translate_lock(name = "{rctx_name}

_init_patches_labels(priv, rctx, attr, label_store)

_init_root_package(priv, rctx, attr, label_store)

if _should_update_pnpm_lock(priv) or not attr.pnpm_lock:
# labels only needed when updating or bootstrapping the pnpm lock file
_init_pnpm_labels(priv, rctx, attr, label_store)
Expand All @@ -56,6 +54,9 @@ WARNING: `update_pnpm_lock` attribute in `npm_translate_lock(name = "{rctx_name}
_load_lockfile(priv, rctx, attr, label_store)
_init_patched_dependencies_labels(priv, rctx, attr, label_store)

# May depend on lockfile state
_init_root_package(priv, rctx, attr, label_store)

if _should_update_pnpm_lock(priv):
_init_importer_labels(priv, label_store)

Expand Down Expand Up @@ -531,6 +532,9 @@ def _packages(priv):
def _patched_dependencies(priv):
return priv["patched_dependencies"]

def _only_built_dependencies(priv):
return _root_package_json(priv).get("pnpm", {}).get("onlyBuildDependencies", None)

def _num_patches(priv):
return priv["num_patches"]

Expand All @@ -543,6 +547,9 @@ def _npm_auth(priv):
def _root_package(priv):
return priv["root_package"]

def _root_package_json(priv):
return priv["root_package_json"]

################################################################################
def _new(rctx_name, rctx, attr, bzlmod):
label_store = repository_label_store.new(rctx.path)
Expand All @@ -564,6 +571,7 @@ def _new(rctx_name, rctx, attr, bzlmod):
"npm_registries": {},
"packages": {},
"root_package": None,
"root_package_json": {},
"patched_dependencies": {},
"should_update_pnpm_lock": should_update_pnpm_lock,
}
Expand All @@ -578,6 +586,7 @@ def _new(rctx_name, rctx, attr, bzlmod):
importers = lambda: _importers(priv),
packages = lambda: _packages(priv),
patched_dependencies = lambda: _patched_dependencies(priv),
only_built_dependencies = lambda: _only_built_dependencies(priv),
npm_registries = lambda: _npm_registries(priv),
npm_auth = lambda: _npm_auth(priv),
num_patches = lambda: _num_patches(priv),
Expand Down
Loading