fix: Replace simple-binary-install with custom implementation for pnpm compatibility by ocombe · Pull Request #170 · coralogix/protofetch

ocombe · 2025-10-22T12:57:38Z

Summary

Replaces simple-binary-install with custom implementation using node-fetch and tar
Fixes pnpm compatibility issues caused by pnpm's symlink-based directory structure
Adds comprehensive CI tests for npm and pnpm installation on all platforms

Problem

The cx-protofetch npm package was incompatible with pnpm due to issues with the simple-binary-install library. pnpm
uses a symlink-based directory structure that causes the library to calculate incorrect extraction paths, resulting in
Z_DATA_ERROR: incorrect header check errors during binary extraction.

Solution

Implemented a custom binary download and extraction solution that:

Downloads binaries directly using node-fetch
Extracts tarballs using the tar library
Works correctly with both npm and pnpm directory structures
Provides better error handling and logging

Changes

Package Code

package.json: Replaced simple-binary-install with node-fetch and tar dependencies
getBinary.js: Complete rewrite with direct download and extraction logic
scripts.js: Updated to handle async downloadBinary function
run.js: Updated to spawn binary from bin/ directory
.gitignore: Added bin/ directory to ignore list

CI/CD

Added test-npm-package job to CI workflow
Tests npm installation on Ubuntu, macOS (Intel & ARM), Windows
Tests pnpm installation on all platforms
Release job now depends on npm package tests passing

Testing Performed

Tested npm installation locally on macOS ARM64
Tested pnpm installation locally on macOS ARM64
Verified binary execution with --version and --help
CI tests will verify on all platforms

Breaking Changes

None - the API remains unchanged. Consumers use the package the same way.

Copilot

Pull Request Overview

This PR replaces the simple-binary-install library with a custom implementation to fix pnpm compatibility issues. The pnpm-specific problem stemmed from its symlink-based directory structure causing incorrect extraction paths in the original library.

Key Changes:

Custom binary download/extraction logic using node-fetch and tar libraries
Updated binary execution to spawn from the bin/ directory
Comprehensive CI testing for both npm and pnpm across all supported platforms

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
.github/workflows/ci.yml	Adds test-npm-package job to verify npm and pnpm installation across all platforms
.github/npm/scripts.js	Updates to handle async downloadBinary function with promise-based error handling
.github/npm/run.js	Replaces simple-binary-install with direct binary spawning from bin/ directory
.github/npm/package.json	Swaps simple-binary-install dependency for node-fetch and tar
.github/npm/getBinary.js	Complete rewrite implementing custom download and extraction logic
.github/npm/.gitignore	Adds bin/ directory to ignore list

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

.github/npm/run.js

.github/workflows/ci.yml

.github/npm/getBinary.js

.github/npm/run.js

rtimush

Nice, thanks! 👍

Replaces simple-binary-install with a custom implementation using node-fetch and tar to ensure compatibility with pnpm's symlink-based directory structure. This resolves issues with incorrect extraction paths and Z_DATA_ERROR errors. Includes improvements such as Windows binary support, retry logic, security filters, and better error handling. Adds CI tests to verify npm and pnpm installations on all platforms and ensures releases only proceed if the tests pass. The implementation downloads binaries directly, extracts tarballs, and correctly handles both npm and pnpm directory structures.

cx-shahar-kazaz · 2025-10-22T14:29:26Z

.github/workflows/ci.yml

+      - name: Install pnpm
+        uses: pnpm/action-setup@v4
+        with:
+          version: 8


latest is 10, why 8?

cx-shahar-kazaz · 2025-10-22T14:29:49Z

.github/workflows/ci.yml

+      - name: Setup Node.js
+        uses: actions/setup-node@v4
+        with:
+          node-version: '20'


node 20 is maintenance, use 22

cx-shahar-kazaz · 2025-10-22T14:30:39Z

.github/workflows/ci.yml

+        run: |
+          cd artifacts
+          # Start HTTP server in background
+          python3 -m http.server 8000 &


Any reason not to continue with Node here?

cx-shahar-kazaz · 2025-10-23T04:47:44Z

.github/npm/getBinary.js

-import { Binary } from 'simple-binary-install';
-import * as os from 'os';
-import * as fs from 'fs';
+import fetch from 'node-fetch';


please use Node's built in.

cx-shahar-kazaz · 2025-10-23T04:47:56Z

.github/npm/getBinary.js

-import * as os from 'os';
-import * as fs from 'fs';
+import fetch from 'node-fetch';
+import { mkdirSync, chmodSync, existsSync, readFileSync } from 'fs';


Prefix all node native modules with node:

cx-shahar-kazaz · 2025-10-23T04:49:00Z

.github/npm/getBinary.js

 	}

-	if (type === 'Darwin' && arch === 'arm64') {
+	if (type === 'darwin' && arch === 'arm64') {


I'd consider wrapping in utility functions; these things are repetitive across the code.

cx-shahar-kazaz · 2025-10-23T04:50:08Z

.github/npm/run.js

@@ -1,4 +1,26 @@
 #!/usr/bin/env node
-import { getBinary } from './getBinary.js';
+import { spawn } from 'child_process';


prefix with node:

cx-shahar-kazaz · 2025-10-23T04:51:02Z

.github/npm/scripts.js

 if (process.argv.includes('install')) {
-	getBinary().install();
+	// Check for --url argument for testing (only localhost allowed for security)
+	const urlArg = process.argv.find(arg => arg.startsWith('--url='));


cx-shahar-kazaz · 2025-10-23T04:51:15Z

.github/npm/scripts.js

+		try {
+			const parsedUrl = new URL(providedUrl);
+			// Only allow localhost URLs for testing
+			if (parsedUrl.hostname === 'localhost' || parsedUrl.hostname === '127.0.0.1') {


isLocalhost()

rtimush requested review from Copilot and rtimush and removed request for rtimush October 22, 2025 12:58

Copilot AI reviewed Oct 22, 2025

View reviewed changes

.github/npm/run.js Show resolved Hide resolved

.github/workflows/ci.yml Show resolved Hide resolved

.github/workflows/ci.yml Outdated Show resolved Hide resolved

.github/npm/getBinary.js Outdated Show resolved Hide resolved

.github/npm/run.js Outdated Show resolved Hide resolved

ocombe force-pushed the fix/pnpm-compatibility branch from 521de48 to aadff87 Compare October 22, 2025 13:54

ocombe requested a review from rtimush October 22, 2025 14:27

rtimush approved these changes Oct 22, 2025

View reviewed changes

ocombe force-pushed the fix/pnpm-compatibility branch from 4b2eec5 to 6036b53 Compare October 22, 2025 15:49

rtimush merged commit 3ae841f into master Oct 22, 2025
20 checks passed

rtimush deleted the fix/pnpm-compatibility branch October 22, 2025 16:17

cx-shahar-kazaz reviewed Oct 23, 2025

View reviewed changes

Conversation

ocombe commented Oct 22, 2025

Summary

Problem

Solution

Changes

Package Code

CI/CD

Testing Performed

Breaking Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rtimush left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants