Skip to content

Conversation

@szymonrybczak
Copy link
Collaborator

@szymonrybczak szymonrybczak commented May 6, 2024

Summary:

This Pull Request implements fetching only platform's specific properties. It means when passing --platform option to config command, config logic will only get platform's properties, because it doesn't make sense to get informations about Android platform when running native_modules.rb, and same deal with native_modules.gradle.

I did some benchmarks in project with 50 linked dependencies, and results are:

Command Result
Standalone config 7.62s
config --platform ios 0.66s
config --platform android 7.43s

So it means that linking dependencies during pod install got ~7 seconds faster in this specific scenario. If there are more dependencies to be linked, the results are higher.

Test Plan:

  1. Clone the repository and do all the required steps from the Contributing guide
  2. Run this command:
node /path/to/react-native-cli/packages/cli/build/bin.js config

Same output as before, if there are tools that depends on config's command response they won't be affected by this change.
3. Run this command:

node /path/to/react-native-cli/packages/cli/build/bin.js config --platform ios

Returns only ios as in platforms field, and also for each entry in dependencies only ios platform should be presented. And this should be valid for any other platform that is available with linked dependency.
4. Installing Cocoapods inside ios/ folder should work in the same way as before.
5. Android app should build correctly and link all native dependencies.

Checklist

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

@szymonrybczak szymonrybczak marked this pull request as ready for review May 7, 2024 08:20
@szymonrybczak
Copy link
Collaborator Author

CI Blocked by #2376

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants