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.

Sign up for GitHub

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

Jump to bottom

feat: support ESM in react-native.config #2453

Merged
merged 8 commits into from
Nov 5, 2024
Merged

feat: support ESM in react-native.config #2453

merged 8 commits into from
Nov 5, 2024

Conversation

szymonrybczak
Copy link
Collaborator

Summary:

Closes #2167

Add support for ESM inside react-native.config file.

Test Plan:

  1. Clone the repository and do all the required steps from the Contributing guide
  2. Create a new project and add type: "module" inside package ✅
  3. Create a react-native.config.mjs with following content
export default {
  commands: [
    {
      name: 'hello',
      func: () => {
        console.log('hello esm');
      },
    },
  ],
};
  1. Run npx react-native hello and you should see hello esm log.

Checklist

  • Documentation is up to date to reflect these changes.
  • Follows commit message convention described in CONTRIBUTING.md

@szymonrybczak szymonrybczak mentioned this pull request Jul 9, 2024
Closed
@satya164
Copy link
Member

satya164 commented Jul 11, 2024 •
edited
Loading

Re: test plan, test these scenarios:

  • Add type: 'module' to package.json and use require in react-native.config.js - should fail unless using createRequire helper (import { createRequire } from 'node:module'; const require = createRequire(import.meta.url);)
  • Add type: 'module' to package.json and use require in react-native.config.cjs
  • Add type: 'module' to package.json and use import/export in react-native.config.js
  • Remove type: 'module from package.json/add type: 'commonjs' and use import/export in react-native.config.mjs

You don’t need both type: 'module' and .mjs. either is enough to signal ES modules.

@szymonrybczak szymonrybczak force-pushed the feat/support-esm-in-react-native-config.js branch from 87e5843 to 6f0db32 Compare August 30, 2024 19:54
@szymonrybczak szymonrybczak force-pushed the feat/support-esm-in-react-native-config.js branch from 566877e to 0a7e8bf Compare August 31, 2024 09:25
@szymonrybczak szymonrybczak force-pushed the feat/support-esm-in-react-native-config.js branch from c670360 to 7f9a335 Compare August 31, 2024 11:32
thymikee
thymikee reviewed Oct 31, 2024
View reviewed changes
__e2e__/config.test.ts Outdated Show resolved Hide resolved
thymikee
thymikee reviewed Oct 31, 2024
View reviewed changes
__e2e__/config.test.ts Outdated Show resolved Hide resolved
szymonrybczak and others added 2 commits October 31, 2024 09:59
@szymonrybczak @thymikee
Update __e2e__/config.test.ts
770b637 
Co-authored-by: Michał Pierzchała <[email protected]>
@szymonrybczak @thymikee
Update __e2e__/config.test.ts
d4e8139 
Co-authored-by: Michał Pierzchała <[email protected]>
@thymikee
Copy link
Member

Changing the signature of loadConfig would be breaking e.g. for rnx-kit. We could consider creating loadConfigAsync or so to avoid it. cc @tido64

@tido64
Copy link
Contributor

tido64 commented Oct 31, 2024 •
edited
Loading

Changing the signature of loadConfig would be breaking e.g. for rnx-kit. We could consider creating loadConfigAsync or so to avoid it. cc @tido64

Thanks, this would indeed break us. I'm also curious why everything needs be async just because of ESM? IMHO, async makes sense if you're doing a lot of things at once and then awaiting them all. But it looks like we're just async/awaiting in sequence, so I'm not really seeing any benefits.

@satya164
Copy link
Member

everything needs be async just because of ESM

Because it's loading ES module dynamically, which needs to be async.

@thymikee thymikee merged commit c64bb05 into main Nov 5, 2024
4 of 10 checks passed
@thymikee thymikee deleted the feat/support-esm-in-react-native-config.js branch November 5, 2024 08:58
This was referenced Nov 5, 2024
Open
Open
@karlhorky
Copy link
Contributor

Nice, thanks to all of the contributors and reviewers here, great to have ESM in react-native.config.js files!

I'm not super aware of the upstreaming / integration process here in React Native projects - will this work right away out of the box with @react-native-community/[email protected]?

Eg:

  1. I guess new React Native projects which start without a framework will receive this feature right away now?
  2. Does this affect Expo projects at all? (I guess not)

@thymikee
Copy link
Member

thymikee commented Nov 5, 2024

Yes, this should work with v15.1. You'll need to update the template (or your project files) to reference the newer version of the CLI. It doesn't affect Expo projects

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TMisiukiewicz TMisiukiewicz TMisiukiewicz approved these changes

@thymikee thymikee thymikee approved these changes

@kraenhansen kraenhansen kraenhansen approved these changes

@adamTrz adamTrz Awaiting requested review from adamTrz adamTrz is a code owner

Assignees
No one assigned
Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support ESM react-native.config files
7 participants
@szymonrybczak @satya164 @thymikee @tido64 @karlhorky @kraenhansen @TMisiukiewicz