Skip to content

fix(build): strip the react-native tree from the published shrinkwrap - #1959

Merged
kriszyp merged 5 commits into
mainfrom
fix/prune-react-native-from-shrinkwrap
Jul 28, 2026
Merged

kriszyp merged 5 commits into
mainfrom
fix/prune-react-native-from-shrinkwrap

Conversation

@heskew

@heskew heskew commented Jul 27, 2026 •

Copy link
Copy Markdown
Contributor

Removes the react-native tree from what harper publishes. Part 1 of #1937.## What and whyalasql declares optionalDependencies: { "react-native-fs": "^2.20.0" }, and react-native-fs peer-depends on react-native without marking it optional. npm 7+ auto-installs peer deps, so resolving alasql drags in react-native, react, hermes, metro and react-devtools-core — ~140MB, ~200 packages. None of it can execute: every require('react-native-fs') in alasql/dist/alasql.fs.js sits behind an isReactNative guard, and Harper only uses alasql.parse plus its function extensions. The published 5.1.23 shrinkwrap pins 794 packages, 64 of them react-native/hermes/metro.This runs alongside prune-shrinkwrap-dev.mjs, which already enforces the same invariant — the published shrinkwrap should describe only the production tree a consumer installs (#1783).## Where to look**build-tools/prune-shrinkwrap-react-native.mjs** is the whole change; the rest is a build step, a doc entry, and tests.The reachability logic is what deserves scrutiny. It computes the set of packages reachable from the root with the react-native-fs edge and without it, and deletes only the difference — so a package still reachable by any other path cannot be removed, and the removed set is derived rather than a hardcoded list of directory names that would silently rot as alasql's tree shifts. resolve() reimplements node_modules lookup (check <from>/node_modules/<name>, then each ancestor); that's the part most likely to be subtly wrong, though a mistake there fails safe — an unresolvable name simply means nothing gets marked reachable through it.On the current tree: 419 dev entries pruned, then 262 react-native entries, 1202 → 521. Zero unresolved required edges afterwards; the only new unresolved optional edge is alasql -> react-native-fs, the one being severed. (The published 5.1.23 shrinkwrap already carries 11 such unresolved optional/peer edges, so this is a pre-existing, normal pattern.)## Why not overridesnpm honours overrides only for the root project — an override in harper's package.json does nothing for anyone installing harper. The published shrinkwrap, by contrast, is authoritative for registry installs.I verified that rather than assuming it, because the premise matters and one plausible failure mode would have sunk it: npm could have re-resolved the pruned optional dependency and pulled the subtree back. Serving a package with a pruned shrinkwrap from a local static registry (so npm takes the real packument + _hasShrinkwrap code path, with the dependency still resolving from npmjs):shrinkwrap before prune: 331 entries, react-native present: yes shrinkwrap after prune: 23 entries, react-native present: no installed: 23 packages react-native tree in consumer tree: NO react-native-fs node: ABSENTConsequence: no stub package, no third-party empty module, and no change to package.json or the committed lockfile.## Scope — what this does not fix- The Docker image. npm install <tarball> has no packument, so npm never learns the shrinkwrap exists and resolves the tree fresh from package.json. Confirmed on the same published 5.1.23 artifact: installed via the registry it honours the pin (fastify 5.8.5), installed from the tarball it resolves fresh (fastify 5.10.0, latest) and the react-native tree returns. The image is therefore also not reproducible — two builds of one commit weeks apart can ship different trees. Follow-up PR, which builds on this one; it has to land after this, since honouring an unpruned shrinkwrap would pin the react-native tree in.

  • Contributor / CI installs, which use package-lock.json and are unaffected. Deliberately left alone: fixing it means either a stub package or a cryptic override, and it costs disk rather than shipped bytes.
  • The root cause. Worth reporting upstream to alasql (active repo, no existing issue found) — RN-only deps are normally lazy-required without being declared. When that lands, this script gets deleted; it reports "nothing to prune" rather than failing, so it degrades quietly.## TestingunitTests/buildTools/pruneShrinkwrapReactNative.test.js — 6 tests, the load-bearing one being that a package pulled in by react-native and depended on elsewhere survives. Note the sibling prune-shrinkwrap-dev.mjs has no tests; I added these because a wrong prune ships a broken shrinkwrap to every consumer.test:unit:main: 2795 passing, 2 failing — globalIsolation and a uWS 413 case. Both reproduce identically on pristine origin/main (12/1 and 6/1 in isolation), and this diff only touches build-tools/, unitTests/buildTools/ and dependencies.md.## DocsNo companion documentation PR: this changes no public API, config option, schema directive, CLI surface or user-visible default — it only removes dead weight from what gets published. The behavioural note for future work lives in DESIGN.md and dependencies.md.Closes Prune the react-native tree from the published shrinkwrap #1962

🤖 Generated with Claude Code

alasql declares `optionalDependencies: { "react-native-fs": "^2.20.0" }`, and
react-native-fs peer-depends on react-native without marking it optional. npm 7+
auto-installs peer deps, so resolving alasql drags in react-native, react,
hermes, metro and react-devtools-core: ~140MB and ~200 packages that cannot
execute under Node, since every require('react-native-fs') in alasql sits behind
an isReactNative guard. Harper only uses alasql.parse and its function
extensions. The published 5.1.23 shrinkwrap pins 794 packages, 64 of them
react-native/hermes/metro.

Pruned from the published shrinkwrap rather than via an `overrides` entry,
because npm honours overrides only for the root project — they do nothing for
anyone installing harper. The published shrinkwrap is authoritative for registry
installs: npm learns it exists from the packument's `_hasShrinkwrap` flag and
installs exactly the tree it describes, without re-resolving pruned optional
dependencies. Verified against npm 11 by serving a package with a pruned
shrinkwrap from a local registry — 331 packages became 23, with no react-native
tree and no react-native-fs node. So this needs no stub package and no
third-party empty module.

The prune is surgical by construction: it computes the packages reachable with
and without the react-native-fs edge and removes only the difference, so nothing
reachable by another path can be removed, and the set is derived rather than a
hardcoded list of directory names that would rot as alasql's tree shifts. On the
current tree it removes 262 entries and leaves zero unresolved required edges.

Runs alongside prune-shrinkwrap-dev.mjs, which already enforces the same
invariant (#1783).

Refs #1937

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a build tool script prune-shrinkwrap-react-native.mjs along with its corresponding unit tests and documentation in dependencies.md. The script surgically prunes the react-native-fs dependency tree from the published npm-shrinkwrap.json to prevent transitive peer dependencies like react-native and react from being installed by consumers. There are no review comments, and I have no feedback to provide.

@claude

claude Bot commented Jul 27, 2026 •

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found.

@heskew
heskew marked this pull request as ready for review July 28, 2026 03:28
heskew and others added 2 commits July 27, 2026 20:30
Captures why the react-native prune lives in build-tools rather than in
`overrides`, and why the Docker image gets none of it: npm honours a bundled
npm-shrinkwrap.json only for registry installs, keyed off the packument's
`_hasShrinkwrap` flag, and re-resolves from package.json for tarball installs.

Refs #1937, #1960

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… result

Cross-model review (Codex) caught that severing every edge named
react-native-fs, rather than only the ones a dependent declared optional, would
delete the package even when another dependent hard-requires it — leaving that
requirement dangling in the published shrinkwrap and breaking installs for every
consumer. Reproduced with a two-parent fixture before fixing: the validator
reported `other-pkg -> react-native-fs` unresolved.

Only `optionalDependencies` edges are severed now, so a real requirement keeps
the package alive for the whole tree. As a backstop the pruned result is scanned
for required edges that resolve before the prune but not after, and the script
throws rather than writing the file. Edges already unresolved in the input are
ignored — a published shrinkwrap can legitimately carry some, and those are not
ours to fail the build over.

With the sever restricted to optional edges the backstop should be unreachable,
since anything a surviving package requires is reachable without the severed
edge. It stays because the cost of being wrong is every consumer's install.

Also corrects the over-broad "untouchable by construction" claim in the script
comment and dependencies.md.

No change to the real tree: still 262 entries pruned, zero react-native entries
left, zero unresolved required edges.

Refs #1937

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread unitTests/buildTools/pruneShrinkwrapReactNative.test.js
heskew and others added 2 commits July 27, 2026 21:23
…une order

Follow-ups from the cross-model review adjudication.

The backstop is not unreachable as the previous comment claimed. Root-reachable
packages cannot trip it, but entries the production walk never visits can — most
usefully dev entries pointing into the react-native subtree, which is exactly
what happens if this runs before the dev prune. Corrected the comment, added a
test that pins the throw, and had the error point at prune-shrinkwrap-dev.mjs so
the failure explains itself.

That ordering was implicit in build.sh; noted it there so a future reorder does
not produce a surprising build failure.

Also added a fixture covering resolve()'s nested-first lookup and scoped names.
Both occur in the real tree (@react-native/*) but every prior fixture was flat
and unscoped, so that branch was only exercised incidentally.

12 unit tests. Real pipeline unchanged: 419 dev entries then 262 react-native
entries pruned, zero unresolved required edges.

Refs #1937

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… walk

Review feedback from cb1kenobi. A prior commit added a nested/scoped fixture ten
minutes after that review landed, which covers nested-first resolution, but not
the case actually suggested: a react-native-fs whose only resolution path is a
copy nested under its dependent, and a dependency of that copy reachable only by
walking up two segments to the root.

Both are now pinned. Verified the coverage is real rather than incidental by
breaking the ancestor walk (clamping the loop to the deepest segment) and
confirming 8 tests fail; restored, 13 pass.

Real pipeline unchanged: 262 react-native entries pruned, zero unresolved
required edges.

Refs #1937

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prune the react-native tree from the published shrinkwrap

3 participants