Skip to content
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.

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

fix: spawn CLI directly without using Node #689

Merged
merged 4 commits into from
Nov 17, 2024

Conversation

gronxb
Copy link
Contributor

@gronxb gronxb commented Nov 16, 2024

Summary

image
The default is set to yarn, but an error occurs when using pnpm to run bob build.

The error arises because the CLI is spawned via Node. Since the CLI in .bin is designed to run with Node by default, it should spawn correctly even without explicitly prepending node.

Test plan

The changes work correctly even when tested with Yarn.

  1. Create a repository using the command:
> npx create-react-native-library@latest rn-lib
  1. Remove the packageManager field from the package.json.
  2. Run pnpm install
  • as-is
  1. An error occurs.
  • to-be
  1. Success.

@atlj
Copy link
Collaborator

atlj commented Nov 17, 2024

Hey, thanks for the issue, for looks like pnpm treats the binaries differently than yarn and this is why we are having this problems. Here's the bob binary generated with pnpm:

#!/bin/sh
basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")

case `uname` in
    *CYGWIN*) basedir=`cygpath -w "$basedir"`;;
esac

if [ -z "$NODE_PATH" ]; then
  export NODE_PATH="<MYPATHS>"
else
  export NODE_PATH="<MYPATHS>"
fi
if [ -x "$basedir/node" ]; then
  exec "$basedir/node"  "$basedir/../@react-native-community/cli/build/bin.js" "$@"
else
  exec node  "$basedir/../@react-native-community/cli/build/bin.js" "$@"
fi

and the same binary generated with yarn:

#!/usr/bin/env node

// eslint-disable-next-line import/no-commonjs
require('../lib/index');

@atlj atlj enabled auto-merge November 17, 2024 15:14
@atlj atlj added this pull request to the merge queue Nov 17, 2024
Merged via the queue into callstack:main with commit a299f14 Nov 17, 2024
29 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Nov 17, 2024
<!-- Please provide enough information so that others can review your
pull request. -->
<!-- Keep pull requests small and focused on a single change. -->

### Summary

![image](https://github.com/user-attachments/assets/2880ad03-3d96-4056-bbdf-299c34aba724)
The default is set to `yarn`, but an error occurs when using `pnpm` to
run `bob build`.

The error arises because the CLI is spawned via Node. Since the CLI in
`.bin` is designed to run with Node by default, it should spawn
correctly even without explicitly prepending node.
<!-- What existing problem does the pull request solve? Can you solve
the issue with a different approach? -->

### Test plan

> The changes work correctly even when tested with Yarn.

1. Create a repository using the command:
```sh
> npx create-react-native-library@latest rn-lib
```
3. Remove the `packageManager` field from the `package.json`.
4. Run `pnpm install`
* as-is
5. An error occurs.
* to-be
5. Success.

<!-- List the steps with which we can test this change. Provide
screenshots if this changes anything visual. -->

---------

Co-authored-by: Burak Güner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants