fix(react-native): prevent set -e from aborting xcode script when npm is not in PATH#3270
fix(react-native): prevent set -e from aborting xcode script when npm is not in PATH#3270marandaneto merged 2 commits intomainfrom
Conversation
… is not in PATH npm prefix -g and npm root exit non-zero (127) when npm isn't in Xcode's stripped PATH (e.g. installed via nvm or Homebrew on Apple Silicon). With set -e active, this caused the script to abort before reaching the PATH-extended fallback that would find posthog-cli. Added || true to prevent set -e from firing on these optional lookups.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Prompt To Fix All With AIThis 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.Last reviewed commit: "fix(react-native): p..." |
| 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) |
There was a problem hiding this 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.:
| 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.|
Size Change: 0 B Total Size: 6.57 MB ℹ️ View Unchanged
|
| 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" |
There was a problem hiding this comment.
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
|
thanks @cat-ph |
|
We should probably update this script as well ? https://github.com/PostHog/posthog-ios/blob/main/build-tools/upload-symbols.sh |
upload-symbols.sh should be okay cause it's not calling |
Problem
Xcode Archive builds fail when
posthog-xcode.shcannot findposthog-cli. The script triesnpm prefix -gandnpm rootto locate the CLI, but Xcode runs build phases with a strippedPATHthat excludes nvm, fnm, and Homebrew (Apple Silicon) directories. Whennpmis not in that PATH, both commands exit with code 127. Becauseset -eis active throughout the script, this causes an immediate abort before the fallback block (which extendsPATHwith common locations like/opt/homebrew/bin) is ever reached.Reported by users on monorepo setups using Expo CNG with
@posthog/cliinstalled globally via nvm or Homebrew on Apple Silicon.Changes
Added
|| trueto the three command substitutions used to locate the CLI:npm prefix -g- global npm prefix lookupnpm root- local npm root lookupcommand -v posthog-cli- final fallback searchThis prevents
set -efrom aborting the script on a missingnpmand allows the PATH-extended fallback to run as intended.Release info Sub-libraries affected
Libraries affected
Checklist