feat: trim unwanted platform prebuilds, rename to NativePlatformManager - #11
Merged
Conversation
node-gyp-build/prebuildify-style packages (e.g. bare-fs) ship every supported platform's binary bundled together in one package, unconditionally - unlike sharp/koffi's one-package-per-platform approach, there's no npm-level mechanism that already limits this to what's actually wanted. A driver bundling any dependency that pulls one of these in (e.g. archiver -> tar-stream -> bare-fs) ends up shipping iOS/Android/etc. prebuilds no appium driver could ever use. Adds NativePlatformManager#trim(), run unconditionally (unlike #inflate(), which only runs when NATIVE_PLATFORMS is set): deletes every prebuilds/<platform> subdirectory, anywhere under ROOT/node_modules, that isn't the CI runner's own platform or a configured NATIVE_PLATFORMS target. Renamed the class from NativePlatformInflator since it now owns both directions of platform selection for the bundle. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V7n3xs33VgEN4uiDjGGSnR
Replace trim()'s unbounded Promise.all fan-outs (one per directory entry, plus one per unwanted prebuilds/<platform> dir) with mapWithConcurrency at RESOLVE_CONCURRENCY, matching inflate()'s existing concurrency pattern for its own installs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LfMmxaegNBiPTotHyZCFHw
Track each trimmed package/platform pair instead of just a count, so the log line names exactly what got removed - derived from each prebuilds directory's parent, the innermost segment after "node_modules/" (scope included). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LfMmxaegNBiPTotHyZCFHw
Staging is fully delegated to npm pack, which already handles a missing/empty "files" field correctly via its own default ignore-based inclusion, same as any ordinary npm publish. The check dates back to when staging manually approximated files-field selection with cp, where an absent files field really did produce an incomplete copy - that's no longer how staging works. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LfMmxaegNBiPTotHyZCFHw
trim() is a no-op unless nativePlatforms.isEnabled, which is already one arm of this block's condition - move it in alongside inflate() so native-platform work is grouped together and the common case (neither unbundled packages nor native platforms configured) skips the call entirely instead of relying on trim()'s internal guard. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LfMmxaegNBiPTotHyZCFHw
Group trimmed platforms by package instead of listing every package/platform pair as a flat comma-joined string, which got hard to read once several packages or platforms were trimmed at once. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LfMmxaegNBiPTotHyZCFHw
The exclusion-warning and native-platform paragraphs duplicated detail already covered by warnAboutUnhonorableExclusions and NativePlatformManager's own doc comments. Point there instead of repeating it at the top of the file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LfMmxaegNBiPTotHyZCFHw
Add NativePlatformManager#stripUnbundled: when a package listed in unbundled-packages still ends up bundled as someone else's transitive dependency (bundleDependencies filtering only reaches direct dependencies), delete its platform-locked native sibling packages (e.g. sharp's @img/sharp-*) from every such copy, wherever nested in the tree. Its own JavaScript and optionalDependencies declaration are left untouched, so a normal npm install still fetches the right native package for the consumer's platform - this is what actually shrinks the bundle when excluding something like sharp, rather than just avoiding a redundant top-level copy. Reused the existing platform-suffix matcher and mapWithConcurrency helpers; verified end-to-end with a root -> support -> sharp -> @img/sharp-<platform> fixture that the native binary is stripped while sharp's own manifest and optionalDependencies survive in the packed tarball. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LfMmxaegNBiPTotHyZCFHw
KazuCocoa
reviewed
Sep 11, 2026
KazuCocoa
left a comment
KazuCocoa
left a comment
Member
There was a problem hiding this comment.
Review findings reproduced locally by Codex.
- stripUnbundled(): skip dependency-graph nodes without an installed path (uninstalled optional deps, e.g. sharp's native packages for other platforms) before recursing/deleting - fixes rm(undefined) throwing ERR_INVALID_ARG_TYPE and aborting the release. - inflate()/#findMissing(): accept unbundledNames and skip collecting optional dependencies declared by an unbundled package, so a native sibling stripUnbundled() already removed doesn't get silently reinstalled when native-platforms is also configured. - trim(): match prebuilds/<dirName> against node-gyp-build's actual "<os>-<cpu1>[+<cpu2>...]" convention instead of exact string comparison, so a universal build like "darwin-x64+arm64" survives when it covers a wanted target instead of being deleted outright. Verified each with a dedicated fixture: an uninstalled platform variant no longer crashes the run, a stripped native package stays stripped after inflation runs, and a multi-arch prebuild directory covering a wanted platform is kept while unwanted ones are still trimmed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LfMmxaegNBiPTotHyZCFHw
KazuCocoa
reviewed
Sep 12, 2026
| if (!node.path || visitedPaths.has(node.path)) { | ||
| return; | ||
| } | ||
| visitedPaths.add(node.path); |
Member
There was a problem hiding this comment.
[P2] Do not mark a truncated dependency reference as fully visited
npm ls --all --long --json can report the same installed sharp path both as a deduplicated reference without dependencies and as a fully expanded node. With root → a → sharp and root → sharp, traversal reaches the truncated reference under a first and adds its path to visitedPaths. The subsequent root-level sharp node, which contains the native dependencies, is then skipped. Codex reproduced this by running the complete script with UNBUNDLED_PACKAGES=sharp: the resulting tarball still contained node_modules/@img/sharp-darwin-arm64/package.json. This leaves native binaries bundled in the direct-plus-transitive dependency case this feature is intended to handle. Please merge dependency information for nodes sharing an installed path before traversal, or otherwise ensure a truncated reference does not suppress processing of the expanded node.
Contributor
Author
There was a problem hiding this comment.
Fixed in c971738: added normalizeDedupedNodes(), run once inside getInstalledDependencyGraph() right after fetching the tree. It collects the fully-expanded node for each installed path and splices its dependencies into every bare reference sharing that path, before any traversal begins - so collectTransitiveNames, stripUnbundled, and inflate's #findMissing all see the same complete data regardless of which occurrence they reach first. Reproduced your exact scenario (root depending on the same package both transitively via another dependency and directly) and confirmed the packed tarball no longer retains the native platform package after excluding it.
KazuCocoa
approved these changes
Sep 12, 2026
KazuCocoa
left a comment
KazuCocoa
left a comment
Member
There was a problem hiding this comment.
once the last comment is fixed, lgtm
npm ls --long only fully expands a package's "dependencies" the first time it encounters that installed path; every other occurrence of the same instance (e.g. both a direct and a transitive dependency) comes back as a bare reference sharing the same path but without its own "dependencies". Every traversal in this file dedupes further visits to an already-seen path, so whichever occurrence is reached first wins - if that's the bare reference, the one occurrence that actually carries the recursive data (native optional dependencies included) gets silently skipped. Add normalizeDedupedNodes(), run once inside getInstalledDependencyGraph() right after fetching the graph: collect the fully-expanded node for each installed path, then splice its "dependencies" into every bare reference sharing that path. All three downstream traversals (collectTransitiveNames, stripUnbundled, inflate's #findMissing) get the fix for free since they all consume the same graph. Verified with the reported root -> a -> sharp / root -> sharp scenario (root depending on the same package both transitively and directly): the packed tarball no longer retains sharp's native platform package after excluding it via UNBUNDLED_PACKAGES. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LfMmxaegNBiPTotHyZCFHw
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.
node-gyp-build/prebuildify-style packages (e.g. bare-fs) ship every supported platform's binary bundled together in one package, unconditionally - unlike sharp/koffi's one-package-per-platform approach, there's no npm-level mechanism that already limits this to what's actually wanted. A driver bundling any dependency that pulls one of these in (e.g. archiver -> tar-stream -> bare-fs) ends up shipping iOS/Android/etc. prebuilds no appium driver could ever use.
Adds NativePlatformManager#trim(), run unconditionally (unlike #inflate(), which only runs when NATIVE_PLATFORMS is set): deletes every prebuilds/ subdirectory, anywhere under ROOT/node_modules, that isn't the CI runner's own platform or a configured NATIVE_PLATFORMS target. Renamed the class from NativePlatformInflator since it now owns both directions of platform selection for the bundle.