Repository navigation
fix(build): prune devDependencies before shrinkwrapping - #1781
Merged
kriszyp merged 1 commit intoJul 13, 2026
Merged
Conversation
Follow-up to HarperFast#1622. npm shrinkwrap runs after a full npm install in build-tools/build.sh, so the generated npm-shrinkwrap.json captures devDependencies as well as runtime ones. Since HarperFast#1622 started shipping npm-shrinkwrap.json in the published tarball, consumers now get harper's entire dev tree installed under node_modules/harper/node_modules/ -- including esbuild (pulled in transitively via tsx) and its ~30 platform-specific optional binaries. Those nested @esbuild/<platform> entries land in the consumer's package-lock.json as "extraneous" rather than resolved-optional, because they aren't reachable from any declared dependency edge in the consumer's graph -- they're just mirrored in from harper's bundled shrinkwrap. Extraneous entries don't get the "optional": true flag written, so npm ci in consumer projects fails with EBADPLATFORM on platforms other than the shrinkwrap's original build machine (e.g. @esbuild/aix-ppc64 on darwin/arm64). Prune devDependencies before shrinkwrapping so the pinned tree only contains what harper actually needs at runtime.
github-actions
Bot
requested review from
Ethan-Arrowood,
cb1kenobi,
heskew and
kriszyp
July 13, 2026 22:42
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds a step to prune devDependencies using npm prune --omit=dev before creating a shrinkwrap in the build script. The reviewer points out that running this locally can leave the developer's environment in a broken state by removing necessary devDependencies and renaming package-lock.json. It is recommended to add a cleanup block at the end of the script to restore the local environment.
Comment on lines
+11
to
+12
| echo -e "\n📦 Pruning devDependencies" | ||
| npm prune --omit=dev |
Contributor
There was a problem hiding this comment.
Running npm prune --omit=dev and npm shrinkwrap locally (e.g., via npm run package) prunes all devDependencies from the local node_modules and renames package-lock.json to npm-shrinkwrap.json. This leaves the developer's local environment in a broken state where they can no longer run tests, linting, or typechecking without manually running npm install and restoring package-lock.json.
To improve the developer experience, we can restore package-lock.json and reinstall the pruned devDependencies at the end of the script after npm pack has completed.
For example, you can add the following cleanup block at the end of build-tools/build.sh:
# Restore package-lock.json and devDependencies for local development
if [ -f npm-shrinkwrap.json ]; then
mv npm-shrinkwrap.json package-lock.json
npm install
fi
Member
|
Nice, this presumably resolves #1780 |
Contributor
Patch cherry-pick: mergedCherry-picked onto |
kriszyp
approved these changes
Jul 13, 2026
kriszyp
left a comment
kriszyp
left a comment
Member
There was a problem hiding this comment.
Approving — correct minimal fix for #1780 (devDependencies were leaking into the published artifact's npm-shrinkwrap). Build-time only, no runtime path touched. The failing Unit Test (Node 26) is a pre-existing rocksdb flake unrelated to this change. — KrAIs (Claude Opus 4.8), on Kris's behalf
This was referenced Jul 13, 2026
github-actions Bot
pushed a commit
that referenced
this pull request
Jul 14, 2026
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>
kriszyp
added a commit
that referenced
this pull request
Jul 14, 2026
* fix(build): strip devDependencies from published shrinkwrap 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> * fix(build): require exactly lockfileVersion 3 in shrinkwrap guard 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> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions Bot
pushed a commit
that referenced
this pull request
Jul 14, 2026
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1622.
npm shrinkwrap runs after a full npm install in build-tools/build.sh, so the generated npm-shrinkwrap.json captures devDependencies as well as runtime ones. Since #1622 started shipping npm-shrinkwrap.json in the published tarball, consumers now get harper's entire dev tree installed under node_modules/harper/node_modules/ -- including esbuild (pulled in transitively via tsx) and its ~30 platform-specific optional binaries.
Those nested @esbuild/ entries land in the consumer's package-lock.json as "extraneous" rather than resolved-optional, because they aren't reachable from any declared dependency edge in the consumer's graph -- they're just mirrored in from harper's bundled shrinkwrap. Extraneous entries don't get the "optional": true flag written, so npm ci in consumer projects fails with EBADPLATFORM on platforms other than the shrinkwrap's original build machine (e.g. @esbuild/aix-ppc64 on darwin/arm64).
Prune devDependencies before shrinkwrapping so the pinned tree only contains what harper actually needs at runtime.