Skip to content

Conversation

@kraenhansen
Copy link
Collaborator

Fixes #192 by adding x86_64 and universal simulator slices to the host and refactor cmake-rn to pick the right defaults: Universal (arm64;x86_64) over arm64.

@kraenhansen kraenhansen self-assigned this Oct 24, 2025
@kraenhansen kraenhansen added Apple 🍎 Anything related to the Apple platform (iOS, macOS, Cocoapods, Xcode, XCFrameworks, etc.) Android 🤖 Anything related to the Android platform (Gradle, NDK, Android SDK) CMake RN Our `cmake` wrapping CLI Host 🏡 Our `react-native-node-api-modules` package labels Oct 24, 2025
@kraenhansen kraenhansen requested a review from Copilot October 24, 2025 10:32
Copy link
Contributor

Copilot AI left a 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 a purpose parameter 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>>;
Copy link

Copilot AI Oct 24, 2025

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.

Suggested change
): Readonly<Triplets> | Promise<Readonly<Triplets>>;
): Triplet[] | Promise<Triplet[]>;

Copilot uses AI. Check for mistakes.
Comment on lines 227 to 232
} else if (process.arch === "arm64") {
return ["arm64-apple-ios-sim"];
} else if (process.arch === "x64") {
return ["x86_64-apple-ios-sim"];
} else {
return [];
Copy link

Copilot AI Oct 24, 2025

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.

Suggested change
} 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"];

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@shirakaba shirakaba left a 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",
Copy link
Collaborator

@shirakaba shirakaba Oct 24, 2025

Choose a reason for hiding this comment

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

⚠️ bikeshedding - feel free to solve more important problems instead!

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"?

Copy link
Collaborator Author

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.

Copy link
Collaborator

@shirakaba shirakaba Oct 25, 2025

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator

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 😄

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

Labels

Android 🤖 Anything related to the Android platform (Gradle, NDK, Android SDK) Apple 🍎 Anything related to the Apple platform (iOS, macOS, Cocoapods, Xcode, XCFrameworks, etc.) CMake RN Our `cmake` wrapping CLI Host 🏡 Our `react-native-node-api-modules` package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review Apple architectures (add x64_64 slices for all simulators?)

2 participants