fix: Add package.json exports for react-native #11623
Conversation
WalkthroughAdded a react-native export mapping to the root export in package.json, directing React Native consumers to ./browser/index.js via a default field. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
package.json (3)
22-24: Prefer a simple string mapping for react-native (fewer moving parts)Metro and Node both accept a string on "react-native". Since you route both import and require to the same file, a single string is clearer and avoids nesting with an inner "default".
Apply this minimal change:
- "react-native": { - "default": "./browser/index.js" - }, + "react-native": "./browser/index.js",Alternative (if you want to be fully explicit about modes):
- "react-native": { - "default": "./browser/index.js" - }, + "react-native": { + "import": "./browser/index.js", + "require": "./browser/index.js" + },
36-45: Consider whether subpath imports need an RN conditionYou already expose wildcards via "./.js" and "./". If RN consumers (or your own docs) encourage subpath imports, Metro won’t see a "react-native" condition on those entries. It should still resolve via "import"/"require", but if you’ve seen similar warnings for subpaths, mirroring the RN condition on "./*" could be warranted.
Happy to draft a scoped follow-up that adds "react-native" only where necessary to avoid noise.
79-96: Optional: note this in CHANGELOG as a developer-facing behavior tweakThough runtime behavior is unchanged, RN consumers benefit from the warning removal. A brief CHANGELOG entry under "Fixes" will help downstream maintainers understand the change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
package.json(1 hunks)
🔇 Additional comments (3)
package.json (3)
20-35: Double-check type resolution for TS consumers under RNYou keep "types": "./index.d.ts" at the same level as the conditional map, which TypeScript uses independently of the selected runtime branch. That’s generally correct. Just confirm that the browser entry’s surface matches the declarations (it should, given the existing browser map).
If you want to be extra safe, add a quick TS compile check in CI that imports "typeorm" with a React Native tsconfig (moduleResolution: bundler or node16) to ensure d.ts alignment.
48-72: Sanity check: RN runtime compatibility of the browser bundleRouting RN to "./browser/index.js" relies on the browser build’s assumptions: no Node core APIs and disabled server-only drivers (which you already alias out via the "browser" field). This is the right target for Hermes/JSC. Just ensure any new code paths continue to avoid Node-only modules.
If you have a minimal RN app repro, import "typeorm" and ensure bundling succeeds and no Metro warnings remain on:
- Metro 0.82.4 (as reported)
- At least one earlier minor (e.g., 0.81.x) to gauge backward behavior
22-24: Validate package resolution with Metro and NodeThe
"react-native": { "default": "./browser/index.js" }export condition correctly targets Metro’s resolver without affecting Node (unknown conditions are ignored).Automated verification failed in this sandbox (missing Python for native modules and no local
gulp). Please manually verify the resolution outputs:• Ensure packaging succeeds:
- Install Python (≥3.x) so native dependencies (e.g. better-sqlite3) can build.
- Run:
npm ci npm run package- Confirm that
build/package/contains the packaged files.• Test Node vs. React Native conditions using an absolute path for
createRequire:node -p "const {createRequire} = require('node:module'); const {resolve} = require('path'); const r = createRequire(resolve(__dirname,'build/package')); console.log('node-default:', r.resolve('typeorm'))"node --conditions=react-native -p "const {createRequire} = require('node:module'); const {resolve} = require('path'); const r = createRequire(resolve(__dirname,'build/package')); console.log('react-native:', r.resolve('typeorm'))"Expected results:
- node-default →
./index.js(or./index.mjsfor ESM)- react-native →
./browser/index.jsFile: package.json
Lines: 22-24"react-native": { - // missing before "default": "./browser/index.js" },
commit: |
|
@sgarner yep, patching this into my installed copy removes the warning and my typeorm imports are still functional. Happy for this to be merged. There are a fair few consumers of typeorm through expo-sqlite, who will be upgrading to expo@53. This should help to pre-empt any questions about the warning. It might not work for subpath imports like |
Description of change
Importing typeorm in a react native app (metro@0.82.4, react-native@0.79.5, expo@53.0.22) generates this warning:
This is happening because Metro by default now looks at package.json exports to find which file to use when a module is imported in react native.
Previously, Metro would consider the
browserproperty in package.json. However, in the new "Package Exports" feature, it only looks for therequireorreact-nativeconditions inside the exports property, as well as optional extra conditions on a per-platform basis. If none of those conditions can be found, it will fall back to the old behaviour, but continue to issue a warning.An easy fix is to add the
react-nativecondition as part of the exports field (Implemented here). In the end, the behaviour is the same (since the fallback behaviour imports the browser code anyway) but suppresses the warning at runtime. This is non-invasive and should have no impact on other platforms.Pull-Request Checklist
masterbranchFixes #00000Summary by CodeRabbit