Skip to content

fix(react-native): prevent set -e from aborting xcode script when npm is not in PATH #3270

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

Merged
marandaneto merged 2 commits into from
Mar 23, 2026
Merged

fix(react-native): prevent set -e from aborting xcode script when npm is not in PATH #3270

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
89a6a73
fix(react-native): prevent set -e from aborting xcode script when npm…
cat-ph Mar 22, 2026
78057b7
chore: changeset
cat-ph Mar 22, 2026
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
6 changes: 3 additions & 3 deletions packages/react-native/tooling/posthog-xcode.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,18 @@ if [ -f "$HOME/.posthog/posthog-cli" ]; then
PH_CLI_PATH="$HOME/.posthog/posthog-cli"
else
# Check if installed via npm -g @posthog/cli
NPM_GLOBAL_PREFIX=$(npm prefix -g 2>/dev/null)
NPM_GLOBAL_PREFIX=$(npm prefix -g 2>/dev/null || true)
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 22, 2026

Choose a reason for hiding this comment

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

P2 Missing inline comments on behavioral changes

Per the project convention, significant changes that alter script behavior should include inline comments explaining the purpose. These three || true additions are a non-obvious workaround for a set -e + missing-npm interaction. Without a comment, a future reader (or automated linter) may treat them as noise and remove them, reintroducing the bug.

Consider adding a brief inline comment, e.g.:

Suggested change
NPM_GLOBAL_PREFIX=$(npm prefix -g 2>/dev/null || true)
NPM_GLOBAL_PREFIX=$(npm prefix -g 2>/dev/null || true) # || true prevents set -e from aborting when npm is not in Xcode's stripped PATH

Rule Used: Add inline comments to clarify the purpose of sign... (source)

Learnt From
PostHog/posthog#32083

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/react-native/tooling/posthog-xcode.sh
Line: 39

Comment:
**Missing inline comments on behavioral changes**

Per the project convention, significant changes that alter script behavior should include inline comments explaining the purpose. These three `|| true` additions are a non-obvious workaround for a `set -e` + missing-`npm` interaction. Without a comment, a future reader (or automated linter) may treat them as noise and remove them, reintroducing the bug.

Consider adding a brief inline comment, e.g.:

```suggestion
  NPM_GLOBAL_PREFIX=$(npm prefix -g 2>/dev/null || true) # || true prevents set -e from aborting when npm is not in Xcode's stripped PATH
```

**Rule Used:** Add inline comments to clarify the purpose of sign... ([source](https://app.greptile.com/review/custom-context?memory=4d5b48c5-045d-4693-9cd9-4081bb19508b))

**Learnt From**
[PostHog/posthog#32083](https://github.com/PostHog/posthog/pull/32083)

How can I resolve this? If you propose a fix, please make it concise.

if [ -n "$NPM_GLOBAL_PREFIX" ] && [ -f "$NPM_GLOBAL_PREFIX/bin/posthog-cli" ]; then
PH_CLI_PATH="$NPM_GLOBAL_PREFIX/bin/posthog-cli"
else
# Check if installed as local dependency
NPM_LOCAL_ROOT=$(npm root 2>/dev/null)
NPM_LOCAL_ROOT=$(npm root 2>/dev/null || true)
if [ -n "$NPM_LOCAL_ROOT" ] && [ -f "$NPM_LOCAL_ROOT/.bin/posthog-cli" ]; then
PH_CLI_PATH="$NPM_LOCAL_ROOT/.bin/posthog-cli"
else
# Fallback to searching common locations
export PATH="/usr/local/bin:/opt/homebrew/bin:$HOME/.cargo/bin:$HOME/.local/bin:$HOME/.posthog:$PATH"
Copy link
Copy Markdown
Contributor Author

@cat-ph cat-ph Mar 22, 2026

Choose a reason for hiding this comment

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

thought: might still have cases where we don't catch this (i.e. other specific node version managers), but at least this version prevents a direct exit when npm isn't found and gives this fallback path a chance

PH_CLI_PATH=$(command -v posthog-cli 2>/dev/null)
PH_CLI_PATH=$(command -v posthog-cli 2>/dev/null || true)
fi
fi
fi
Expand Down
Loading