fix: opt out of node-gyp on install via gypfile:false#367
Open
BridgeAR wants to merge 1 commit into
Open
Conversation
This comment has been minimized.
This comment has been minimized.
Overall package sizeSelf size: 2.4 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | pprof-format | 2.2.2 | 500.53 kB | 500.53 kB | | source-map | 0.7.6 | 185.63 kB | 185.63 kB | | node-gyp-build | 4.8.4 | 13.86 kB | 13.86 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
szegedi
approved these changes
Jul 7, 2026
szegedi
requested changes
Jul 7, 2026
szegedi
left a comment
There was a problem hiding this comment.
The base branch should be main, not v5.x
Dropping the no-op install script in #363 stopped Yarn Berry's YN0007 warning, but it let npm's publish-time normalization take over: with binding.gyp present in the dev tree (and excluded from the tarball) and no install/preinstall script, npm synthesizes "gypfile": true and "install": "node-gyp rebuild" into the published manifest. Consumers on npm then run node-gyp rebuild against a binding.gyp that isn't in the package and the install fails, which shipped broken in 5.16.0. Setting "gypfile": false suppresses that implicit hook while keeping the manifest free of lifecycle scripts, so both npm and Yarn Berry install the prebuilt binaries without a build step. The regression test now also pins gypfile:false alongside the no-lifecycle-scripts assertion. Fixes: #364 (review)
d0d9efd to
6b67b97
Compare
szegedi
approved these changes
Jul 7, 2026
IlyasShabi
approved these changes
Jul 7, 2026
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
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.
Summary
Dropping the no-op install script in #363 stopped Yarn Berry's
YN0007warning, but it handed control to npm's publish-time normalization. Withbinding.gypin the dev tree (excluded from the tarball) and noinstall/preinstallscript, npm synthesizes"gypfile": trueand"install": "node-gyp rebuild"into the published manifest. npm consumers then runnode-gyp rebuildagainst abinding.gypthat isn't in the package, and the install fails — this shipped broken in5.16.0(see the linked review, e.g. the pprof-it CI failure).Setting
"gypfile": falsesuppresses that implicit hook while keeping the manifest free of lifecycle scripts, so npm and Yarn Berry both install the prebuilt binaries without a build step. The regression test now pinsgypfile:falsealongside the existing no-lifecycle-scripts assertion.Why
"gypfile": falseis package metadata rather than a lifecycle script, so it does not reintroduce theYN0007warning #363 removed. Verified against npm's own normalization (@npmcli/package-json): withbinding.gypstill on disk, the normalized manifest keepsgypfile: falseand no synthesizedinstallscript — the exact metadata that was broken in5.16.0.A follow-up patch release is needed since
latest(5.16.0) is currently broken for npm consumers.Fixes: #364 (review)