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

Support ESM react-native.config files #2167

Open
kraenhansen opened this issue Nov 12, 2023 · 7 comments
Open

Support ESM react-native.config files #2167

kraenhansen opened this issue Nov 12, 2023 · 7 comments

Comments

@kraenhansen
Copy link
Contributor

kraenhansen commented Nov 12, 2023

Describe the Feature

I'd love to use modern ESM code in my React Native apps and libraries. The CLI's config package currently blocks having "type": "module" in a React Native app or library package, because the config command loads config files synchronous, with a ~ 5 year old version of cosmiconfig.

Setting "type": "module" in a package.json of an app or dependency yields the following error:

error Failed to load configuration of your project.
Error [ERR_REQUIRE_ESM]: require() of ES Module {...}/rnc-cli-bug/react-native.config.js from {...}/node_modules/cosmiconfig/node_modules/import-fresh/index.js not supported.
react-native.config.js is treated as an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which declares all .js files in that package scope as ES modules.
Instead rename react-native.config.js to end in .cjs, change the requiring code to use dynamic import() which is available in all CommonJS modules, or change "type": "module" to "type": "commonjs" in {...}/rnc-cli-bug/package.json to treat all .js files as CommonJS (using .mjs for all ES modules instead).

Users cannot rename react-native.config.js to react-native.config.cjs because it's not in the list of files being searched for, even when patching that list, yields this error:

Error: No loader specified for extension ".cjs", so searchPlaces item "react-native.config.cjs" is invalid

So, there doesn't seem to exist any workaround for this.

Possible Implementations

I believe the best solution is to upgrade cosmicconfig to a more recent version and use their async API to load the config file.
This entails that the following methods would become async:

Which would in practice break the API of both the @react-native-community/cli and @react-native-community/cli-config packages and probably require a major version bump.

A less intrusive alternative, would be to upgrade but only support CommonJS configs and simply add react-native.config.cjs and fix the issue loading it.

Note: Upgrading cosmicconfig would have the added benefit of packages being able to declare their React Native configs in their package.json files too. But that should either be disabled or named differently to avoid conflicts with the react-native main field already relied upon by Metro.

I'd be willing to contribute a PR to fix this, if there's a good chance it'll be considered for review.

Related Issues

I'm surprised, but I can't seem to find existing issues mentioning this limitation.

@robhogan
Copy link
Collaborator

robhogan commented Nov 12, 2023

FWIW, we intend to update cosmiconfig in Metro to support metro.config.mjs (and type: module) as soon as possible - one thing that's holding us back is that we ran into nodejs/node#35889 - use of await import is liable to segfault Node VMs - and we were seeing this intermittently in Jest (integration tests that involved using cosmiconfig) as soon as we tried to update. I'll be trying again as soon as we've moved our CI to Node 20.

Related: facebook/metro#916

@thymikee
Copy link
Member

@robhogan so it looks like we're blocked on Metro? Please keep us updated, we'd happily implement this

@robhogan
Copy link
Collaborator

I wouldn't say blocked - I can't think of any technical reason this couldn't go into the CLI before Metro, although it might be less surprising if we were consistent.

I just wanted to share the experience we had with this upgrade - it might be that you're similarly blocked by Jest under Node<20, if you have integration tests that don't mock out cosmiconfig.

@szymonrybczak
Copy link
Collaborator

I just wanted to share the experience we had with this upgrade - it might be that you're similarly blocked by Jest under Node<20, if you have integration tests that don't mock out cosmiconfig.

Yes, we have - and I tried updating cosmiconfig and tests are failing because of this reason.

@karlhorky
Copy link
Contributor

one thing that's holding us back is that we ran into nodejs/node#35889 - use of await import is liable to segfault Node VMs

@robhogan seems like that issue is now resolved - in case this should be noted anywhere as this blocker being removed.

@robhogan
Copy link
Collaborator

one thing that's holding us back is that we ran into nodejs/node#35889 - use of await import is liable to segfault Node VMs

@robhogan seems like that issue is now resolved - in case this should be noted anywhere as this blocker being removed.

Unfortunately it's only resolved for Node 20, and won't be backported, so it'll need to wait until we're on Node 20 (for CI, at least - there's probably no risk in prod as long as we don't use this in a Node VM).

@karlhorky
Copy link
Contributor

karlhorky commented Mar 27, 2024

Ok, thanks for the answer! So I guess it's waiting until facebook/react-native and facebook/metro are on Node.js version 20?

Taking a quick look in those repos, maybe that is tracked here?

  1. facebook/react-native CI config: https://github.com/facebook/react-native/blob/575507dd61298be2f3071e0fc01ed9715cbd7a66/.circleci/config.yml#L52
  2. facebook/react-native package.json -> engines: https://github.com/facebook/react-native/blob/575507dd61298be2f3071e0fc01ed9715cbd7a66/packages/react-native/package.json#L23-L25
  3. facebook/metro CI config: https://github.com/facebook/metro/blob/84956bd11ab0ded6f06a867120980f8524e9cab6/.circleci/config.yml#L12-L15
  4. facebook/metro package.json -> engines: https://github.com/facebook/metro/blob/84956bd11ab0ded6f06a867120980f8524e9cab6/package.json#L93-L95

Or does this also have to do with the react-native-community/cli repo? I see a couple of references to Node.js version numbers here, which still include v18:

node-version: [18, 20]

node-version: [18, 20]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants