-
Notifications
You must be signed in to change notification settings - Fork 4
Add x86_64 and universal simulator slices #288
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for x86_64 and universal (arm64;x86_64) simulator slices across all Apple platforms (iOS, tvOS, visionOS, macOS) and refactors the cmake-rn build system to intelligently select triplets based on build purpose (development vs. release) and host architecture.
Key changes:
- Added x86_64 and universal simulator triplets for iOS, tvOS, and visionOS
- Refactored
defaultTriplets()to accept apurposeparameter for context-aware triplet selection - Updated triplet ordering to prioritize universal slices over single-architecture variants
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/host/src/node/prebuilds/triplets.ts | Adds x86_64 and universal simulator triplets to APPLE_TRIPLETS array |
| packages/host/src/node/prebuilds/apple.ts | Removes APPLE_ARCHITECTURES mapping (likely moved to cmake-rn) |
| packages/cmake-rn/src/platforms/types.ts | Updates Platform type to accept purpose parameter in defaultTriplets() |
| packages/cmake-rn/src/platforms/apple.ts | Adds new triplet mappings and implements purpose-aware default triplet selection |
| packages/cmake-rn/src/platforms/android.ts | Updates Android platform to use purpose parameter for triplet selection |
| packages/cmake-rn/src/cli.ts | Updates CLI to pass "release" or "development" purpose when calling defaultTriplets() |
| .changeset/bright-parts-roll.md | Documents the changes for version control |
| defaultTriplets(): Triplets[number][] | Promise<Triplets[number][]>; | ||
| defaultTriplets( | ||
| purpose: "development" | "release", | ||
| ): Readonly<Triplets> | Promise<Readonly<Triplets>>; |
Copilot
AI
Oct 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return type should be Triplet[] | Promise<Triplet[]> instead of Readonly<Triplets> | Promise<Readonly<Triplets>>. The Readonly wrapper is unnecessary here since arrays are already passed by reference, and the caller doesn't need the full Triplets type constraint - they just need an array of triplet strings.
| ): Readonly<Triplets> | Promise<Readonly<Triplets>>; | |
| ): Triplet[] | Promise<Triplet[]>; |
| } else if (process.arch === "arm64") { | ||
| return ["arm64-apple-ios-sim"]; | ||
| } else if (process.arch === "x64") { | ||
| return ["x86_64-apple-ios-sim"]; | ||
| } else { | ||
| return []; |
Copilot
AI
Oct 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] For development mode, consider returning universal simulator slices (arm64;x86_64) instead of architecture-specific ones when possible. This would provide better compatibility across different simulators without requiring architecture-specific builds, aligning with the PR's goal of prioritizing universal slices.
| } else if (process.arch === "arm64") { | |
| return ["arm64-apple-ios-sim"]; | |
| } else if (process.arch === "x64") { | |
| return ["x86_64-apple-ios-sim"]; | |
| } else { | |
| return []; | |
| } else { | |
| // For development, prefer universal simulator slice for best compatibility | |
| return ["arm64;x86_64-apple-ios-sim"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a great improvement, thank you!
| */ | ||
| defaultTriplets(): Triplets[number][] | Promise<Triplets[number][]>; | ||
| defaultTriplets( | ||
| purpose: "development" | "release", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Xcode uses the terminology build configuration (which you can name "orange" | "banana" if you wish, but defaults to Debug and Release when creating new projects).
They confusingly use the term Development for codesigning for local deployment.
From an Apple point of view, I'd go with:
- purpose: "development" | "release",
+ configuration: "Debug" | "Release",On the other hand, Android uses build variants:
- purpose: "development" | "release",
+ variant: "debug" | "release",So uhhh I guess we can't win. 🤔
But either way, "debug" is probably more consistent than "development"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value isn't encoding the build configuration "Release" vs "Debug" that you're referring to. What I'm trying to capture here (which might be total overkill BTW) is that a library want to build only for some triplets when they're in the process of iterating their library, to match simulators of a specific architecture for example. To speed things up while iterating locally, the don't want to produce pre-builds for all possible triplets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right. Then surely this should be changed to onlyBuildActiveArchitecture: boolean or something? As "active architecture" I believe is terminology commonly used across various IDEs and build systems.
Or buildAllArchitectures: boolean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree - it's the same concept as the activeArchOnly option passable to the React Native CLI: src/commands/buildAndroid/index.ts#L58-L77 where it's actually probing ADB. We might be able to do the same or something similar to figure out what simulators and emulators are actively booted 🤔
That being said, I don't know if it's worth it though investing too much into this feature: We're running all of the builds of triplets in parallel so the increase in build-time doesn't scale linearly with the amount of triplets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll leave it to you! Just driving by 😄
Fixes #192 by adding x86_64 and universal simulator slices to the host and refactor
cmake-rnto pick the right defaults: Universal (arm64;x86_64) over arm64.