fix(core): Resolve expo CLI directly instead of using npx in sourcemap upload#6155
Merged
Conversation
…p upload
Replace `npx expo config --json` with direct `require.resolve('expo/bin/cli')`
to avoid npm's `devEngines.packageManager` enforcement breaking the script
for projects that restrict their package manager to pnpm or yarn.
Fixes #6152
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
Plus 1 more 🤖 This preview updates automatically when you update the PR. |
Contributor
|
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
Exercises the `require.resolve('expo/bin/cli', { paths: [process.cwd()] })`
fallback path used in pnpm/yarn PnP environments where expo is not
resolvable from the script's own location.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
q: the auto-generated PR description has the claim that yarn PnP works but how could it be since the whole PR depends on the existance of |
alwx
requested changes
May 18, 2026
| const mockNpxScript = path.join(mockBinDir, 'npx'); | ||
| // The mock npx script outputs the config JSON when called with 'expo config --json' | ||
| // Create a mock expo/bin/cli that outputs the given expo config as JSON | ||
| const mockExpoCliDir = path.join(tempDir, 'node_modules', 'expo', 'bin'); |
Contributor
There was a problem hiding this comment.
expo/bin/cli is not a public API — ok, it works today but if Expo renames/relocates it, sourcemap upload will silently fall back to sentry.properties/env vars
Contributor
There was a problem hiding this comment.
what I'm saying is that it's probably better to resolve expo/package.json and read its bin.expo field
Contributor
Author
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instead of hardcoding `expo/bin/cli`, resolve `expo/package.json` and read its `bin.expo` field to construct the CLI path. This is resilient to Expo renaming or relocating the bin entry point. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
Good point @alwx 👍 I've updated the PR description. Ready for another pass |
📲 Install BuildsAndroid
|
alwx
approved these changes
May 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📢 Type of change
📜 Description
Replaces
spawnSync('npx', ['expo', 'config', '--json'])in@sentry/expo-upload-sourcemapswith direct module resolution viarequire.resolve('expo/package.json'), reading thebin.expofield from it, and spawning the resolved CLI path withprocess.execPath.This avoids going through npm/npx entirely, which prevents npm's
devEngines.packageManagerenforcement from rejecting the call when the project restricts its package manager to pnpm.The resolution tries two paths:
require.resolvefrom the script's location (works with hoisted layouts — npm, yarn classic)require.resolvewithpaths: [process.cwd()](works with strict isolation — pnpm)If neither resolves, the existing fallback chain (
sentry.properties→ env vars) kicks in unchanged.💡 Motivation and Context
Projects using npm v11+'s
devEngines.packageManagerfield to enforce pnpm getEBADDEVENGINESwhen the sourcemap upload script internally callsnpx expo config --json, because npm detects the package manager mismatch and refuses to run.Fixes #6152
💚 How did you test it?
devEngines.packageManager: pnpm, confirmednpx expo config --jsonfails withEBADDEVENGINESsamples/expoproject — identical behavior to the old approach📝 Checklist
sendDefaultPIIis enabled🔮 Next steps