feat(npm): experimental Yarn PnP zero-install support - #2957
Conversation
e216450 to
9bc6ffc
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9bc6ffccd1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Why? Does modern yarn provide a strict lockfile describing the exact structure of |
dc7115a to
2748ff5
Compare
|
You’re right on the core distinction: modern Yarn pins the logical resolution graph, not the exact physical This PR compensates by running pinned Yarn to export a stricter graph and verified archives, which explains the adapter size and creates a larger maintenance boundary than the original description made clear. I don’t think this broad exporter is appropriate to merge upstream as-is, so I’ve updated the description to say that explicitly and will keep this implementation in the fork. If Yarn support is potentially in scope, I can instead open a small design proposal for a PnP-focused importer based on Yarn’s generated PnP representation and checked-in cache, without asking rules_js to maintain this exporter. |
|
rules_js should never run yarn, running pnpm to convert yarn to pnpm-lock is an edge case we handle but not part of the standard workflow If we support yarn the lockfile must be deterministic without any rules_js logic deciding the node_modules layout which I assume is what a lot of this PR is doing |
0bc86ee to
f8c36bc
Compare
|
@jbedard Took your last comment as the spec and replaced this PR's content entirely — What's here now is a PnP zero-install importer: rules_js never executes yarn, Authored surface is ~1,100 lines; the diff looks bigger because two Apologies for the force-push over the old review context — the previous |
Consume a checked-in Yarn PnP zero-install project without ever running Yarn. The pnp module extension reads the user-generated .pnp.data.json and yarn.lock, cross-validates them (every hard-linked package must resolve to an offline-cache archive covered by a lockfile checksum), and generates pnp_js_binary / pnp_js_test wrappers that load the checked-in .pnp.cjs resolver via --require. A generated pnp_verify_test recomputes the sha512 of every cache archive against yarn.lock and rejects stale or foreign archives. All resolution and layout decisions are made by Yarn on the developer's machine and are fully described by the checked-in files; rules_js only parses and consumes them, mirroring the pnpm-lock.yaml review model. Tests: - npm/private/test: 4 Starlark suites covering the Berry lockfile parser (incl. multi-descriptor keys), PnP data validation, virtual (peer) locator devirtualization, and error paths - all passing - e2e/pnp_zero_install: a real Yarn 4.12.0 zero-install fixture; main_test resolves transitive deps out of the cache zips under Bazel and verify_cache_test validates all archives - both passing Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hsnpEj9uwyqyFo4W9swPQ
Extend the Berry lockfile parser to metadata version 6 (Yarn 3.x): bare sha512 checksums without a cache-key prefix and unquoted dependency ranges, validated against artifacts generated by a real Yarn 3.8.7 install in the new e2e/pnp_zero_install_yarn3 fixture. Yarn 3 support is import/verification only: pnp_verify_test passes against the Yarn 3 cache, but the Yarn 3 .pnp.cjs runtime loads a second instance of itself when the project is reachable through more than one path spelling (as under Bazel's runfiles and output trees) and recurses until the stack overflows, so runtime execution stays Yarn 4+ and the limitation is documented in the fixture. The Yarn 4 e2e (runtime + verification) also passes on Bazel 8.4.2 in addition to 7.7.1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hsnpEj9uwyqyFo4W9swPQ
b048d13 to
40014c2
Compare
Summary
This is an experimental Yarn PnP zero-install importer.
rules_jsnever runsYarn and never synthesizes a
node_modulestree. The caller supplies labelsfor the checked-in
yarn.lock,.pnp.data.json,.pnp.cjs,.pnp.integrity.json,.yarnrc.yml, cache archives, and any unplugged runtimefiles.
Repository analysis parses Yarn Berry lockfile versions 6, 8, and 10 and
cross-validates the generated PnP graph: roots and top-level fallback state,
dependency edges and aliases, virtual-to-physical locators, link types, cache
and unplugged locations, and lockfile bindings. It generates
pnp_js_binary/pnp_js_testwrappers pluspnp_verify_test.The integrity manifest binds file type, executable mode, and sha512 for the
resolver, graph, config, lockfile, every cache archive, and every referenced
unplugged file. Runtime verification happens before
.pnp.cjsis loaded,rejects noncanonical or escaping paths and source symlinks, checks exact cache
and unplugged tree membership, and independently checks lockfile checksums when
present. Valid lock entries without a checksum remain protected by the
integrity manifest; an active unplugged package may retain one matching
lock-backed source archive.
Version support
covered by
e2e/pnp_zero_installwith real peer-virtualized React packagesand an unplugged package.
e2e/pnp_zero_install_yarn3. Yarn 3 runtime execution under Bazel is notclaimed; the fixture README documents the generated runtime's multi-path
recursion limitation.
Known limitations: CommonJS PnP only (
.pnp.loader.mjs/ ESM is not yetsupported), Yarn-generated cache and unplugged path conventions are recognized
rather than invented, and these wrappers do not compose with rules that
require a linked
node_moduleslayout.Changes are visible to end-users: yes
READMEs and inline API documentation)
Suggested release note: experimental support for consuming integrity-bound
Yarn PnP zero-install projects in Bazel without executing Yarn or creating a
node_modulestree.Test plan
Fresh direct validation on the final reconstructed tree:
Starlark/BUILD files.
node --test npm/private/pnp/pnp_verify_test.mjs: 14/14 passed, includingcorrupted archives, graph/config mutation, executable-mode binding, path
traversal, source symlinks, and exact unplugged-tree controls.
passes for the Yarn 4 and Yarn 3 fixtures.
install --immutable --immutable-cache --check-cachepassed;yarn node main.jsprintedpnp-zero-install-ok.install --immutable --immutable-cache --check-cachepassed(import/verification support only).
git diff --check, Git object connectivity, and clean-worktree checkspassed. Read-only self-review found and fixed a standalone-verifier
..path traversal mismatch; no P0/P1/P2 findings remain.
Earlier focused, forced-uncached Bazel 7.7.1 validation executed 13/13 focused
tests (126 packages, 7,555 configured targets, 51.009s). That run predates the
final formatting, copy-mode, and path-traversal hardening, so it is supporting
evidence rather than a final-head Bazel result.
The full Yarn 4
//...gate is not claimed green. After repairing thelocal Node cache artifact, the final authorized warmed run on the HFS-backed
workspace emitted zero package, target, action, or test progress for 30
minutes and was stopped by its exact process handle (exit 143). This is
qualified as a local Bazel/filesystem infrastructure block; the direct gates
above are the current final-head evidence.