Skip to content
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.

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

Help React Native import @apollo/client/main.cjs and other CommonJS bundles #9716

Merged
merged 4 commits into from
May 13, 2022

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented May 13, 2022

We use the .cjs file extension for our CommonJS bundle entry points (such as @apollo/client/core/core.cjs) to indicate to Node.js that "type": "module" in @apollo/client/package.json does not apply to those files.

Without additional configuration of its Metro bundler, React Native refuses to resolve .cjs files by default, but it will attempt to append various file extensions, including the (relatively) React Native-specific ending .native.js. By providing a dist/**/*.cjs.native.js file for every existing dist/**/*.cjs file we ship, React Native should always have a safe fallback to find the right code.

If this works, it will completely supersede/obviate the manual advice involving metro.config.js we added to CHANGELOG.md in #9084.

There is some risk of the dual package hazard (importing the same module multiple times unexpectedly), but as long as Metro consistently resolves the .native.js version of the file, all should be well. Apollo Client should completely ignore the .native.js version of the file, so React Native is the only reason those alternate modules should ever be used.

This PR incorporates the following PRs for dependency packages that also used .cjs bundle files:

Issues potentially fixed:

@benjamn
Copy link
Member Author

benjamn commented May 13, 2022

The .cjs.native.js files are just copies of the corresponding .cjs files, to be clear.

I thought about making each .cjs.native.js file a thin CommonJS wrapper that re-exports the .cjs module's exports (like module.exports = require("./main.cjs")) but Metro doesn't like require("./main.cjs") for the same reasons as before, and even if it did accept that style of code, it would have to be careful not to resolve ./main.cjs back to the current ./main.cjs.native.js module after appending .native.js again.

I also thought about making the .cjs.native.js files be symbolic links to the .cjs files, but my experience suggests symlinks aren’t portable (Windows). Happy to explore other ideas here if there's interest.

@brainkim
Copy link
Contributor

Thank you for investigating this, Ben! I don’t have the bandwidth right now to spin up a React Native project so I will rubber-stamp approve this, having only looked at the files.

@benjamn benjamn merged commit 74d2072 into main May 13, 2022
@benjamn benjamn deleted the help-react-native-import-cjs-bundles branch May 13, 2022 21:54
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The suggested .cjs resolver fix on 3.5.4 adds 10 seconds of boot on iOS.
2 participants