fix(build): strip devDependencies from published shrinkwrap by kriszyp · Pull Request #1783 · HarperFast/harper

kriszyp · 2026-07-14T01:10:28Z

Follow-up to harper#1781 — fix(build): prune devDependencies before shrinkwrap, which was merged but is a no-op.

Why #1781 didn't work

npm shrinkwrap regenerates the lockfile from the package.json manifest, not from the physical node_modules tree. npm prune --omit=dev only removes packages from node_modules — it does not touch the lockfile. So every devDependency (marked "dev": true) survives into the published npm-shrinkwrap.json. Verified: the shrinkwrap produced with the prune step is byte-for-byte identical to the one produced without it. (Thanks to @cb1kenobi's Barber AI review on the harper-pro twin for flagging this.)

The actual bug (#1780 / #1782)

The published shrinkwrap vendors the 26 @esbuild/* platform packages that arrive via tsx (a devDependency), each marked { "dev": true, "optional": true }. When a consumer generates their lockfile with npm 11, npm folds harper's shrinkwrap subtree into their package-lock.json and drops the "optional": true flag. npm ci then treats @esbuild/aix-ppc64 as required on every platform:

npm error code EBADPLATFORM
npm error notsup Unsupported platform for @esbuild/aix-ppc64@0.28.1:
  wanted {"os":"aix","cpu":"ppc64"} (current: {"os":"linux","cpu":"x64"})

Fix

Prune the shrinkwrap after it is generated. build-tools/prune-shrinkwrap-dev.mjs (zero-dependency) removes every package marked "dev": true plus the root devDependencies block, enforcing the invariant that the published shrinkwrap describes only the production tree a consumer installs. No npm version can fold dev entries that aren't there.

 echo -e "\n📦 Creating shrinkwrap"
 npm shrinkwrap

+echo -e "\n📦 Pruning devDependencies from shrinkwrap"
+node build-tools/prune-shrinkwrap-dev.mjs npm-shrinkwrap.json

Validation

Run against the real published harper@5.1.19 shrinkwrap:

417 dev-only entries removed — all 26 @esbuild/* gone, zero "dev": true remaining.
Production deps (794 packages) and prod-optional natives (@lmdb/*, @harperfast/rocksdb-js-*, @msgpackr-extract/*) untouched, each keeping optional: true.
Stripped shrinkwrap installs cleanly and npm ci passes (validated via a local verdaccio registry).

Labeled patch to match the original.

🤖 Generated with Claude Code

The prior `npm prune --omit=dev` before `npm shrinkwrap` (#1781) is a no-op: npm generates the lockfile from the package.json manifest, so prune (which only touches node_modules) leaves every devDependency in npm-shrinkwrap.json. The published shrinkwrap therefore vendors dev-only platform packages (@esbuild/* via tsx); when a consumer's lockfile is generated with npm 11, npm folds that subtree in and drops the "optional" flag, so `npm ci` fails everywhere with EBADPLATFORM @esbuild/aix-ppc64. Prune the shrinkwrap after it is generated instead: remove every package marked "dev": true plus the root devDependencies block, so the published shrinkwrap describes only the production tree a consumer installs. Verified against the published 5.1.19 shrinkwrap — 417 dev-only entries removed (all @esbuild/*), production and prod-optional (lmdb/rocksdb/msgpackr) entries untouched. Fixes #1780 Fixes #1782 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

github-actions · 2026-07-14T01:11:11Z

Patch cherry-pick: merged

Cherry-picked onto v5.1.

gemini-code-assist

Code Review

This pull request replaces the npm prune step in the build process with a custom script, prune-shrinkwrap-dev.mjs, which removes dev-only packages from the generated npm-shrinkwrap.json file. Feedback on this change highlights that for lockfile version 2, the script should also recursively prune the legacy dependencies tree to prevent inconsistencies between packages and dependencies maps.

Ethan-Arrowood

Nice — this is the real fix. #1781 ran the prune after the shrinkwrap, so it was effectively a no-op; moving it before is exactly right. The one red check is the unrelated RocksDB Blob flake, so not blocking. Good to merge.

Follow-up worth considering: a CI/publish assertion that the shipped npm-shrinkwrap.json contains no devDependencies, so this can't silently regress again.

_{sent with Claude Opus 4.8}

Harper's engines field (^22.18.0 || >=24.0.0) only ever produces npm 10/11, which only ever writes lockfileVersion 3 — the committed package-lock.json is already v3. The v2-with-legacy-`dependencies` case the old `>= 2` guard nominally accepted is unreachable in this repo's build, so tighten the check instead of adding untestable pruning logic for a lockfile shape that can't occur. Addresses PR #1783 review comment from cb1kenobi. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

claude · 2026-07-14T17:39:04Z

Reviewed; no blockers found.

Harper's engines field (^22.18.0 || >=24.0.0) only ever produces npm 10/11, which only ever writes lockfileVersion 3 — the committed package-lock.json is already v3. The v2-with-legacy-`dependencies` case the old `>= 2` guard nominally accepted is unreachable in this repo's build, so tighten the check instead of adding untestable pruning logic for a lockfile shape that can't occur. Addresses PR #1783 review comment from cb1kenobi. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

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>

github-actions Bot requested review from Ethan-Arrowood, cb1kenobi and heskew July 14, 2026 01:10

kriszyp added the patch label Jul 14, 2026

kriszyp mentioned this pull request Jul 14, 2026

fix(build): prune devDependencies before shrinkwrap HarperFast/harper-pro#571

Merged

gemini-code-assist Bot reviewed Jul 14, 2026

View reviewed changes

Comment thread build-tools/prune-shrinkwrap-dev.mjs

cb1kenobi reviewed Jul 14, 2026

View reviewed changes

Comment thread build-tools/prune-shrinkwrap-dev.mjs Outdated

Ethan-Arrowood approved these changes Jul 14, 2026

View reviewed changes

kriszyp merged commit 72cc38d into main Jul 14, 2026
57 checks passed

kriszyp deleted the kris/strip-dev-shrinkwrap branch July 14, 2026 18:09

This was referenced Jul 27, 2026

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

Merged

Prune the react-native tree from the published shrinkwrap #1962

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(build): strip devDependencies from published shrinkwrap - #1783

fix(build): strip devDependencies from published shrinkwrap#1783
kriszyp merged 2 commits into
mainfrom
kris/strip-dev-shrinkwrap

kriszyp commented Jul 14, 2026

Uh oh!

github-actions Bot commented Jul 14, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Ethan-Arrowood left a comment

Uh oh!

claude Bot commented Jul 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

kriszyp commented Jul 14, 2026

Why #1781 didn't work

The actual bug (#1780 / #1782)

Fix

Validation

Uh oh!

github-actions Bot commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Patch cherry-pick: merged

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Ethan-Arrowood left a comment

Choose a reason for hiding this comment

Uh oh!

claude Bot commented Jul 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Jul 14, 2026 •

edited

Loading