Skip to content

feat: split into optional dependencies #134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gmegidish wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: split into optional dependencies #134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 42 additions & 9 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,18 +396,51 @@ jobs:
env:
NPM_AUTH_TOKEN: ${{ secrets.NPM_AUTH_TOKEN }}
run: |
# prepare binaries for distribution
mv ./mobilecli-darwin/mobilecli-darwin-arm64 publish/npm/bin
mv ./mobilecli-darwin/mobilecli-darwin-amd64 publish/npm/bin
mv ./mobilecli-linux/mobilecli-linux-amd64 publish/npm/bin
mv ./mobilecli-linux/mobilecli-linux-arm64 publish/npm/bin
mv ./mobilecli-windows/mobilecli-windows-amd64.exe publish/npm/bin
chmod +x publish/npm/bin/*
# setup npmrc
echo "//registry.npmjs.org/:_authToken=$NPM_AUTH_TOKEN" >> ~/.npmrc
# copy README.md

# publish platform-specific packages
# darwin-arm64
cp ./mobilecli-darwin/mobilecli-darwin-arm64 publish/npm-darwin-arm64/
chmod +x publish/npm-darwin-arm64/mobilecli-darwin-arm64
cd publish/npm-darwin-arm64
npm version "${{ github.ref_name }}" --no-git-tag-version
npm publish --access public
cd ../..

# darwin-amd64
cp ./mobilecli-darwin/mobilecli-darwin-amd64 publish/npm-darwin-amd64/
chmod +x publish/npm-darwin-amd64/mobilecli-darwin-amd64
cd publish/npm-darwin-amd64
npm version "${{ github.ref_name }}" --no-git-tag-version
npm publish --access public
cd ../..

# linux-arm64
cp ./mobilecli-linux/mobilecli-linux-arm64 publish/npm-linux-arm64/
chmod +x publish/npm-linux-arm64/mobilecli-linux-arm64
cd publish/npm-linux-arm64
npm version "${{ github.ref_name }}" --no-git-tag-version
npm publish --access public
cd ../..

# linux-amd64
cp ./mobilecli-linux/mobilecli-linux-amd64 publish/npm-linux-amd64/
chmod +x publish/npm-linux-amd64/mobilecli-linux-amd64
cd publish/npm-linux-amd64
npm version "${{ github.ref_name }}" --no-git-tag-version
npm publish --access public
cd ../..

# windows-amd64
cp ./mobilecli-windows/mobilecli-windows-amd64.exe publish/npm-windows-amd64/
cd publish/npm-windows-amd64
npm version "${{ github.ref_name }}" --no-git-tag-version
npm publish --access public
cd ../..

# publish main package (wrapper)
cp README.md publish/npm
# publish to npm
cd publish/npm
npm version "${{ github.ref_name }}" --no-git-tag-version
npm install --ignore-scripts
Expand Down
13 changes: 13 additions & 0 deletions publish/npm-darwin-amd64/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"name": "@mobilenext/mobilecli-darwin-amd64",
"version": "0.0.4",
"author": "Mobile Next",
"description": "mobilecli binary for macOS x64",
"repository": {
"type": "git",
"url": "git+https://github.com/mobile-next/mobilecli.git"
},
"license": "MIT",
"os": ["darwin"],
"cpu": ["x64"]
}
13 changes: 13 additions & 0 deletions publish/npm-darwin-arm64/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"name": "@mobilenext/mobilecli-darwin-arm64",
"version": "0.0.4",
"author": "Mobile Next",
"description": "mobilecli binary for macOS ARM64",
"repository": {
"type": "git",
"url": "git+https://github.com/mobile-next/mobilecli.git"
},
"license": "MIT",
"os": ["darwin"],
"cpu": ["arm64"]
}
13 changes: 13 additions & 0 deletions publish/npm-linux-amd64/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"name": "@mobilenext/mobilecli-linux-amd64",
"version": "0.0.4",
"author": "Mobile Next",
"description": "mobilecli binary for Linux x64",
"repository": {
"type": "git",
"url": "git+https://github.com/mobile-next/mobilecli.git"
},
"license": "MIT",
"os": ["linux"],
"cpu": ["x64"]
}
13 changes: 13 additions & 0 deletions publish/npm-linux-arm64/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"name": "@mobilenext/mobilecli-linux-arm64",
"version": "0.0.4",
"author": "Mobile Next",
"description": "mobilecli binary for Linux ARM64",
"repository": {
"type": "git",
"url": "git+https://github.com/mobile-next/mobilecli.git"
},
"license": "MIT",
"os": ["linux"],
"cpu": ["arm64"]
}
13 changes: 13 additions & 0 deletions publish/npm-windows-amd64/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"name": "@mobilenext/mobilecli-windows-amd64",
"version": "0.0.4",
"author": "Mobile Next",
"description": "mobilecli binary for Windows x64",
"repository": {
"type": "git",
"url": "git+https://github.com/mobile-next/mobilecli.git"
},
"license": "MIT",
"os": ["win32"],
"cpu": ["x64"]
}
38 changes: 32 additions & 6 deletions publish/npm/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,41 @@
const { join } = require("node:path");
const { spawn } = require("node:child_process");

let binary;
let packageName;
let binaryName;

switch (process.platform) {
case "darwin":
switch (process.arch) {
case "arm64":
binary = "mobilecli-darwin-arm64";
packageName = "@mobilenext/mobilecli-darwin-arm64";
binaryName = "mobilecli-darwin-arm64";
break;
case "x64":
binary = "mobilecli-darwin-amd64";
packageName = "@mobilenext/mobilecli-darwin-amd64";
binaryName = "mobilecli-darwin-amd64";
break;
}
break;

case "linux":
switch (process.arch) {
case "arm64":
binary = "mobilecli-linux-arm64";
packageName = "@mobilenext/mobilecli-linux-arm64";
binaryName = "mobilecli-linux-arm64";
break;
case "x64":
binary = "mobilecli-linux-amd64";
packageName = "@mobilenext/mobilecli-linux-amd64";
binaryName = "mobilecli-linux-amd64";
break;
}
break;

case "win32":
switch (process.arch) {
case "x64":
packageName = "@mobilenext/mobilecli-windows-amd64";
binaryName = "mobilecli-windows-amd64.exe";
break;
}
break;
Expand All @@ -33,7 +47,19 @@ switch (process.platform) {
process.exit(1);
}

const binaryPath = join(__dirname, "bin", binary);
if (!packageName) {
console.error(`Unsupported platform: ${process.platform}-${process.arch}`);
process.exit(1);
}

let binaryPath;
try {
const packagePath = require.resolve(packageName);
binaryPath = join(packagePath, "..", binaryName);
} catch (error) {
console.error(`Failed to find ${packageName}. Please reinstall @mobilenext/mobilecli.`);
process.exit(1);
}
Comment on lines +55 to +62
Copy link

@coderabbitai coderabbitai bot Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's check if the file exists and read the relevant section
cat -n publish/npm/index.js | head -70 | tail -20

Repository: mobile-next/mobilecli

Length of output: 733

🏁 Script executed:

# Let's also look for package.json files to understand the package structure
find . -name "package.json" -type f | head -20

Repository: mobile-next/mobilecli

Length of output: 310

🏁 Script executed:

# Check the project root package.json to understand the package structure
cat package.json | head -50

Repository: mobile-next/mobilecli

Length of output: 109

🏁 Script executed:

# Let's examine the platform-specific package.json files to understand their structure
echo "=== npm-darwin-amd64 package.json ===" && cat publish/npm-darwin-amd64/package.json
echo -e "\n=== npm-linux-amd64 package.json ===" && cat publish/npm-linux-amd64/package.json

Repository: mobile-next/mobilecli

Length of output: 738

🏁 Script executed:

# Let's check the structure of these platform-specific package directories
ls -la publish/npm-darwin-amd64/ | head -20

Repository: mobile-next/mobilecli

Length of output: 234

🏁 Script executed:

# Find where packageName is assigned/determined in the file
cat -n publish/npm/index.js | head -60

Repository: mobile-next/mobilecli

Length of output: 1913

🏁 Script executed:

# Check if binary files exist in the platform directories
find publish/npm-darwin-amd64/ publish/npm-linux-amd64/ -type f

Repository: mobile-next/mobilecli

Length of output: 139

🏁 Script executed:

# Let's check the main npm package.json that depends on these
cat publish/npm/package.json

Repository: mobile-next/mobilecli

Length of output: 698

🏁 Script executed:

# Create a test to understand how require.resolve behaves with these packages
python3 << 'EOF'
# We need to understand Node.js require.resolve behavior
# Let's create a simple test scenario

# Key question: Does require.resolve(packageName) work without "main" in package.json?
# And does require.resolve(`${packageName}/package.json`) work?

# Based on Node.js documentation:
# - require.resolve() without a "main" field will fail
# - require.resolve('pkg/package.json') explicitly resolves the package.json

print("Testing Node.js require.resolve behavior:\n")
print("Scenario: Package has no 'main' field in package.json")
print("- require.resolve('packageName') -> Would FAIL (no main entry)")
print("- require.resolve('packageName/package.json') -> Would SUCCEED (explicit path)")
print("\nThe review comment's claim is CORRECT:")
print("These binary-only packages don't define 'main' in package.json")
print("So require.resolve(packageName) WILL throw an error")
print("Using require.resolve(`${packageName}/package.json`) is the correct fix")
EOF

Repository: mobile-next/mobilecli

Length of output: 530

🏁 Script executed:

# Let's verify the actual binary file is in the right location
ls -la publish/npm-darwin-amd64/ | grep -E "mobilecli|bin"

Repository: mobile-next/mobilecli

Length of output: 47

🏁 Script executed:

# Check if binary files actually exist in node_modules or if they're supposed to be there
# Let's look at a complete view of what should be in these packages
ls -la publish/npm-darwin-arm64/

Repository: mobile-next/mobilecli

Length of output: 234

require.resolve(packageName) will fail for these binary-only packages because they lack a main entry point

The platform-specific packages (@mobilenext/mobilecli-darwin-amd64, @mobilenext/mobilecli-linux-amd64, etc.) only define a package.json with no main field and contain no JavaScript files. Node's require.resolve() requires a resolvable entry point (like main, index.js, or exports), so it will throw an error for these packages. This causes binaryPath to never be set and users will always encounter the "Failed to find" error.

Use require.resolve() with an explicit path to package.json instead:

 let binaryPath;
 try {
-	const packagePath = require.resolve(packageName);
-	binaryPath = join(packagePath, "..", binaryName);
+	const packageJsonPath = require.resolve(`${packageName}/package.json`);
+	binaryPath = join(packageJsonPath, "..", binaryName);
 } catch (error) {
 	console.error(`Failed to find ${packageName}. Please reinstall @mobilenext/mobilecli.`);
 	process.exit(1);
 }

This resolves package.json directly (which is guaranteed to exist) and derives the correct binary directory path from there.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let binaryPath;
try {
const packagePath = require.resolve(packageName);
binaryPath = join(packagePath, "..", binaryName);
} catch (error) {
console.error(`Failed to find ${packageName}. Please reinstall @mobilenext/mobilecli.`);
process.exit(1);
}
let binaryPath;
try {
const packageJsonPath = require.resolve(`${packageName}/package.json`);
binaryPath = join(packageJsonPath, "..", binaryName);
} catch (error) {
console.error(`Failed to find ${packageName}. Please reinstall @mobilenext/mobilecli.`);
process.exit(1);
}
🤖 Prompt for AI Agents 
In publish/npm/index.js around lines 55 to 62, require.resolve(packageName) will
throw for binary-only packages without a main entry; change the resolve target
to package.json (e.g. require.resolve(packageName + '/package.json')) and derive
binaryPath by taking the directory of that resolved package.json and joining it
with the binary name, so binaryPath is correctly set for platform-specific
packages.


const args = process.argv.slice(2);
const child = spawn(binaryPath, args, {
Expand Down
7 changes: 7 additions & 0 deletions publish/npm/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@
"license": "MIT",
"bin": {
"mobilecli": "index.js"
},
"optionalDependencies": {
"@mobilenext/mobilecli-darwin-arm64": "0.0.4",
"@mobilenext/mobilecli-darwin-amd64": "0.0.4",
"@mobilenext/mobilecli-linux-arm64": "0.0.4",
"@mobilenext/mobilecli-linux-amd64": "0.0.4",
"@mobilenext/mobilecli-windows-amd64": "0.0.4"
}

}
Loading