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

feat: Enable New Architecture with react-native.config.js #2091

Conversation

TMisiukiewicz
Copy link
Collaborator

Summary:

Currently, to enable New Architecture, there is a need of running RCT_NEW_ARCH_ENABLED=1 pod install. This change introduces new param in config file: project.ios.newArchEnabledwhich forces to run app with a given architecture. This change is caused by enabling pods installation in run-ios and build-ios commands (#2077) - currently, if someone ran RCT_NEW_ARCH_ENABLED=1 pod install and then added a package with native code, the script would disable New Arch. This PR allows to:

  1. Install pods in run/build commands with an Architecture the project is using based on a value from Pods.xcodeproj/project.pbxproj
  2. Force using the given architecture on iOS based on the project.ios.newArchEnabled field. In case anyone change the architecture by using a RCT_NEW_ARCH_ENABLED flag and the param in config is set, it will install the pods accordingly to the Architecture set in the config

Test Plan:

  1. Init new project with nightly
  2. Use run-ios and check if project ran correctly
  3. Use RCT_NEW_ARCH_ENABLED=1 pod install and run-ios and verify if fabric: true in Metro console
  4. Create react-native.config.js with the following content:
module.exports = {
  project: {
    ios: {
      newArchEnabled: false,
    },
  },
};

  1. Run run-ios and verify fabric: true is not visible in Metro logs
  2. Change value to true and run run-ios again - verify fabric: true is visible in Metro logs
  3. Add a dependency containing native code e.g. react-native-gesture-handler to the project
  4. Run run-ios again, let the pods install and verify fabric: true is set in the Metro logs
  5. Run pod install in ios folder and run run-ios again, verify that pods are installed again and fabric: true is visible in Metro logs

Checklist

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

}

async function runPodInstall(loader: Ora, options?: RunPodInstallOptions) {
const shouldHandleRepoUpdate = options?.shouldHandleRepoUpdate || true;
try {
loader.start(
`Installing CocoaPods dependencies ${chalk.dim(
'(this may take a few minutes)',
)}`,
);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡We can add here a log that New Architecture is enabled.

@cortinico
Copy link
Member

@thymikee
Copy link
Member

thymikee commented Oct 3, 2023

@cortinico we could read this data from the manifest if present. Small feat to add, once we settle on the proposal. And since this is still a draft, we can align the API to mimic the one presented in the reactNativeManifest

@kelset
Copy link
Member

kelset commented Oct 4, 2023

btw I'm not even sure that the RFC is in shape right now, wasn't it supposed to get re-written once again?

@cipolleschi
Copy link
Contributor

btw I'm not even sure that the RFC is in shape right now, wasn't it supposed to get re-written once again?

Yes, I have to do it, but I have higher priority tasks to run first. However, I'm not a fan of having the same settings in multiple places. As soon as the RFC land, that should be the only place where user can enable/disable the new architecture. All other places and ways to set it should then be ignored.

So, consider that when working on this feature. Part of the work you are doing could need to be removed later on.

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

Successfully merging this pull request may close these issues.

None yet

6 participants