fix(core,browser,react): Add react-native export condition to resolve dual ESM/CJS bundling#21362
fix(core,browser,react): Add react-native export condition to resolve dual ESM/CJS bundling#21362antonis wants to merge 3 commits into
react-native export condition to resolve dual ESM/CJS bundling#21362Conversation
…ve dual ESM/CJS bundling Add `react-native` export condition to `@sentry/core`, `@sentry/browser`, and `@sentry/react` package.json exports. This prevents Metro (RN 0.79+) from resolving both ESM and CJS builds of these packages into native bundles. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
b1fd558 to
c706d9c
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c706d9c. Configure here.
| "react-native": { | ||
| "types": "./build/types/index.d.ts", | ||
| "default": "./build/esm/index.js" | ||
| }, |
There was a problem hiding this comment.
Missing export regression test
Low Severity
This fix PR only updates package.json exports and the changelog, with no unit, integration, or E2E test in the diff that would fail without the react-native conditions and pass with them. Per SDK testing conventions for fix PRs, a regression test for Metro ESM-only resolution is recommended.
Additional Locations (2)
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit c706d9c. Configure here.
chargome
left a comment
There was a problem hiding this comment.
Generally LGTM but do you also need these export conditions e.g. for browser-utils and other packages that are pulled into our browser SDK?
Co-authored-by: Charly Gomez <charly.gomez1310@gmail.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Lms24
left a comment
There was a problem hiding this comment.
Looks good to me in general! I wonder though how other packages handle this? Is it now a requirement for RN-compatible packages supporting CJS and ESM to explicitly define a react-native submodule export? If so that's fine (I have far too little context on Expo/RN to judge this 😅). If not, are there other solutions?
I think this is more of a workaround for Metro ESM resolution issues (facebook/metro#1582) and it's cleaner to be in the JS repo than working around this in the react native SDK. RN and most of its ecosystem simply don't use dual import/require conditions, which is why they don't hit this bug. |
alwx
left a comment
There was a problem hiding this comment.
Looks good. I guess the export regression part could be skipped for now.
Before submitting a pull request, please take a look at our
Contributing guidelines and verify:
yarn lint) & (yarn test).Closes getsentry/sentry-react-native#6262
Summary
Since React Native 0.79 (April 2025), Metro enables
unstable_enablePackageExportsby default.@sentry/core,@sentry/browser, and@sentry/reactonly exportimport/requireconditions. Due to a MetroisESMImportdetection issue, some dependencies resolve to CJS while others resolve to ESM — causing both full module trees to be bundled into native apps.This adds a
react-nativeexport condition (pointing to ESM) to all three packages. Metro includesreact-nativein its default condition names, so it matches first and always resolves to ESM — bypassing theimport/requireambiguity.Impact
Bundle analysis (sentry-react-native sample app, Android)
Safety
The
react-nativecondition is only recognized by Metro (React Native). It has no effect on:import/requireas beforeChanges
packages/core/package.json— Addreact-nativecondition to.,./browser,./serversubpathspackages/browser/package.json— Addreact-nativecondition to.(points to prod ESM build)packages/react/package.json— Addreact-nativecondition to.