Skip to content

Conversation

shirakaba
Copy link
Collaborator

@shirakaba shirakaba commented Sep 29, 2025

About

This PR adds React Native support, following the react-native-node-api prebuilds standard, a format which is understood by both Node.js and React Native.

Change in structure

Following the prebuilds standard means adopting a new file structure.

packages/ios

- dist
- └── ios-universal
-     └── NativeScript.xcframework
-         ├── Info.plist
-         ├── ios-arm64
-         │   └── NativeScript.framework
-         │       ├── Headers
-         │       │   └── NativeScript.h
-         │       ├── Info.plist
-         │       ├── NativeScript
-         │       └── _CodeSignature
-         │           └── CodeResources
-         └── ios-arm64_x86_64-simulator
-             └── NativeScript.framework
-                 ├── Headers
-                 │   └── NativeScript.h
-                 ├── Info.plist
-                 ├── NativeScript
-                 └── _CodeSignature
-                     └── CodeResources
+ build
+ └── RelWithDebInfo
+     └── NativeScript.apple.node
+         ├── Info.plist
+         ├── ios-arm64
+         │   └── NativeScript.framework
+         │       ├── Headers
+         │       │   └── NativeScript.h
+         │       ├── Info.plist
+         │       ├── NativeScript
+         │       └── _CodeSignature
+         │           └── CodeResources
+         ├── ios-arm64_x86_64-simulator
+         │   └── NativeScript.framework
+         │       ├── Headers
+         │       │   └── NativeScript.h
+         │       ├── Info.plist
+         │       ├── NativeScript
+         │       └── _CodeSignature
+         │           └── CodeResources
+         └── react-native-node-api-module

packages/macos

- dist
- └── macos
-     └── NativeScript.node
+ build
+ └── RelWithDebInfo
+     └── NativeScript.apple.node
+         ├── Info.plist
+         ├── macos-arm64_x86_64
+         │   └── NativeScript.framework
+         │       ├── Headers -> Versions/Current/Headers
+         │       ├── NativeScript -> Versions/Current/NativeScript
+         │       ├── Resources -> Versions/Current/Resources
+         │       └── Versions
+         │           ├── 0.1.0
+         │           │   ├── Headers
+         │           │   │   └── NativeScript.h
+         │           │   ├── NativeScript
+         │           │   ├── Resources
+         │           │   │   └── Info.plist
+         │           │   └── _CodeSignature
+         │           │       └── CodeResources
+         │           └── Current -> 0.1.0
+         └── react-native-node-api-module

How to review

To make this PR easy to review, I have structured it as two commits. The first commit commits all changes, while the second commit reverts all the metadata, types, and binary changes so that you can just see which 19 source files were changed.

Once the PR is approved, I'd just drop that reduce diff commit to restore the metadata, types, and binary changes.

@shirakaba shirakaba force-pushed the react-native-support branch 5 times, most recently from 8c42996 to 84df9c3 Compare September 29, 2025 16:25
Built with:

```sh
./build_all_react_native.sh
```
git restore --source=main --staged --worktree packages/macos/types packages/ios/types metadata-generator/metadata packages/macos/build packages/ios/build packages/ios-sim/types packages/ios/dist packages/macos/dist
Copy link
Collaborator Author

@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.

Self-reviewed to walk through the code changes.

Comment on lines -90 to +92
elseif(TARGET_PLATFORM_MACOS)
# If building a generic library for macOS, we'll build a dylib instead of a framework
unset(BUILD_FRAMEWORK)
else()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ Now we do want to build a .framework instead of a .dylib.

Copy link
Collaborator

Choose a reason for hiding this comment

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

With no other condition unsetting BUILD_FRAMEWORK, we would always produce a framework instead of dylib, is that right? How does it affect distribution of the macOS .node binary build? As of now, we produce a .dylib for macOS, and simply rename it to NativeScript.node.

Comment on lines -417 to +435
# if (GENERIC_NAPI)
target_link_options(
${NAME}
PRIVATE
"-Wl"
"-undefined"
"dynamic_lookup"
)
# endif()
if(GENERIC_NAPI)
target_link_options(
${NAME}
PRIVATE
"-Wl"
"-undefined"
"dynamic_lookup"
)
endif()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ As we're prebuilding this binary on its own and bringing the Node-API implementation separately later, we need to restore GENERIC_NAPI.

This may have consequences for the Node-API distribution of @nativescript/ios. If so, please adjust it as needed so that it works for both targets.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is the right change to make as not all builds would require this flag anymore. Since making it backwards compatible, we don't make separate NS FFI and runtime frameworks - its all in one binary and linking should work fine unless building for platforms that dynamically load NativeScript later (GENERIC_NAPI: Node.js and co, React Native). Would be good to test a build against V8 once to make sure it compiles.

set -e
source "$(dirname "$0")/build_utils.sh"

function to_bool() {
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 now comes from build_utils.sh.

@shirakaba shirakaba marked this pull request as ready for review September 29, 2025 16:32
@shirakaba shirakaba requested review from DjDeveloperr and NathanWalker and removed request for DjDeveloperr and NathanWalker September 29, 2025 16:32
Copy link
Collaborator

@DjDeveloperr DjDeveloperr left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Comment on lines -90 to +92
elseif(TARGET_PLATFORM_MACOS)
# If building a generic library for macOS, we'll build a dylib instead of a framework
unset(BUILD_FRAMEWORK)
else()
Copy link
Collaborator

Choose a reason for hiding this comment

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

With no other condition unsetting BUILD_FRAMEWORK, we would always produce a framework instead of dylib, is that right? How does it affect distribution of the macOS .node binary build? As of now, we produce a .dylib for macOS, and simply rename it to NativeScript.node.

Comment on lines -417 to +435
# if (GENERIC_NAPI)
target_link_options(
${NAME}
PRIVATE
"-Wl"
"-undefined"
"dynamic_lookup"
)
# endif()
if(GENERIC_NAPI)
target_link_options(
${NAME}
PRIVATE
"-Wl"
"-undefined"
"dynamic_lookup"
)
endif()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is the right change to make as not all builds would require this flag anymore. Since making it backwards compatible, we don't make separate NS FFI and runtime frameworks - its all in one binary and linking should work fine unless building for platforms that dynamically load NativeScript later (GENERIC_NAPI: Node.js and co, React Native). Would be good to test a build against V8 once to make sure it compiles.

// ===

const path =
"./build/RelWithDebInfo/NativeScript.apple.node/macos-arm64_x86_64/NativeScript.framework/Versions/0.1.0/NativeScript";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense here. We're loading it from a specific path in the JS entry point. macOS distribution should be fine then.

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.

2 participants