Merge remote-tracking branch 'upstream/main' into reduce-ios-install-time

Author: dougg0k

.changeset/rich-donuts-mix.md

@@ -1,5 +1,5 @@
 ---
-"react-native-node-api-test-app": patch
+"@react-native-node-api/test-app": patch
 "react-native-node-api": patch
 ---
 

.github/workflows/check.yml

@@ -24,8 +24,28 @@ jobs:
       - uses: actions/setup-node@v4
         with:
           node-version: lts/jod
+      # Set up JDK and Android SDK only because we need weak-node-api, to build ferric-example and to run the linting
+      # TODO: Remove this once we have a way to run linting without building the native code
+      - name: Set up JDK 17
+        uses: actions/setup-java@v3
+        with:
+          java-version: "17"
+          distribution: "temurin"
+      - name: Setup Android SDK
+        uses: android-actions/setup-android@v3
+        with:
+          packages: tools platform-tools ndk;${{ env.NDK_VERSION }}
+      - run: rustup target add x86_64-linux-android
       - run: npm ci
+      - run: npm run build
+      # Bootstrap host package to get weak-node-api and ferric-example to get types
+      # TODO: Solve this by adding an option to ferric to build only types or by committing the types into the repo as a fixture for an "init" command
+      - run: npm run bootstrap --workspace react-native-node-api
+      - run: npm run bootstrap --workspace @react-native-node-api/ferric-example
       - run: npm run lint
+        env:
+          DEBUG: eslint:eslint
+      - run: npm run prettier:check
   unit-tests:
     strategy:
       fail-fast: false

.prettierignore

@@ -0,0 +1,13 @@
+# Ignore artifacts
+dist
+build
+Pods
+target
+.cxx
+
+# Ignore hermes
+packages/host/hermes
+packages/node-addon-examples/examples
+packages/node-tests/node
+packages/node-tests/tests
+packages/node-tests/*.generated.js

apps/test-app/App.tsx

@@ -9,47 +9,70 @@ import {
 } from "mocha-remote-react-native";
 
 import { suites as nodeAddonExamplesSuites } from "@react-native-node-api/node-addon-examples";
+import { suites as nodeTestsSuites } from "@react-native-node-api/node-tests";
 
 function describeIf(
   condition: boolean,
   title: string,
-  fn: (this: Mocha.Suite) => void
+  fn: (this: Mocha.Suite) => void,
 ) {
   return condition ? describe(title, fn) : describe.skip(title, fn);
 }
 
 type Context = {
   allTests?: boolean;
   nodeAddonExamples?: boolean;
+  nodeTests?: boolean;
   ferricExample?: boolean;
 };
 
 function loadTests({
   allTests = false,
   nodeAddonExamples = allTests,
+  nodeTests = allTests,
   ferricExample = allTests,
 }: Context) {
   describeIf(nodeAddonExamples, "Node Addon Examples", () => {
     for (const [suiteName, examples] of Object.entries(
-      nodeAddonExamplesSuites
+      nodeAddonExamplesSuites,
     )) {
       describe(suiteName, () => {
         for (const [exampleName, requireExample] of Object.entries(examples)) {
           it(exampleName, async () => {
             const test = requireExample();
             if (test instanceof Function) {
-              await test();
+              const result = test();
+              if (result instanceof Promise) {
+                await result;
+              }
             }
           });
         }
       });
     }
   });
 
+  describeIf(nodeTests, "Node Tests", () => {
+    function registerTestSuite(suite: typeof nodeTestsSuites) {
+      for (const [name, suiteOrTest] of Object.entries(suite)) {
+        if (typeof suiteOrTest === "function") {
+          it(name, suiteOrTest);
+        } else {
+          describe(name, () => {
+            registerTestSuite(suiteOrTest);
+          });
+        }
+      }
+    }
+
+    registerTestSuite(nodeTestsSuites);
+  });
+
   describeIf(ferricExample, "ferric-example", () => {
     it("exports a callable sum function", () => {
-      /* eslint-disable-next-line @typescript-eslint/no-require-imports -- TODO: Determine why a dynamic import doesn't work on Android */
-      const exampleAddon = require("ferric-example");
+      const exampleAddon =
+        /* eslint-disable-next-line @typescript-eslint/no-require-imports -- TODO: Determine why a dynamic import doesn't work on Android */
+        require("ferric-example") as typeof import("ferric-example");
       const result = exampleAddon.sum(1, 3);
       if (result !== 4) {
         throw new Error(`Expected 1 + 3 to equal 4, but got ${result}`);

apps/test-app/babel.config.js

@@ -1,5 +1,5 @@
 module.exports = {
-  presets: ['module:@react-native/babel-preset'],
+  presets: ["module:@react-native/babel-preset"],
   // plugins: [['module:react-native-node-api/babel-plugin', { stripPathSuffix: true }]],
-  plugins: ['module:react-native-node-api/babel-plugin'],
+  plugins: ["module:react-native-node-api/babel-plugin"],
 };

apps/test-app/package.json

@@ -10,10 +10,12 @@
     "test:android": "mocha-remote --exit-on-error -- concurrently --kill-others-on-fail --passthrough-arguments npm:metro 'npm:android -- {@}' --",
     "test:android:allTests": "MOCHA_REMOTE_CONTEXT=allTests npm run test:android -- ",
     "test:android:nodeAddonExamples": "MOCHA_REMOTE_CONTEXT=nodeAddonExamples npm run test:android -- ",
+    "test:android:nodeTests": "MOCHA_REMOTE_CONTEXT=nodeTests npm run test:android -- ",
     "test:android:ferricExample": "MOCHA_REMOTE_CONTEXT=ferricExample npm run test:android -- ",
     "test:ios": "mocha-remote --exit-on-error -- concurrently --passthrough-arguments --kill-others-on-fail npm:metro 'npm:ios -- {@}' --",
     "test:ios:allTests": "MOCHA_REMOTE_CONTEXT=allTests npm run test:ios -- ",
     "test:ios:nodeAddonExamples": "MOCHA_REMOTE_CONTEXT=nodeAddonExamples npm run test:ios -- ",
+    "test:ios:nodeTests": "MOCHA_REMOTE_CONTEXT=nodeTests npm run test:ios -- ",
     "test:ios:ferricExample": "MOCHA_REMOTE_CONTEXT=ferricExample npm run test:ios -- "
   },
   "dependencies": {
@@ -23,6 +25,8 @@
     "@react-native-community/cli": "^18.0.0",
     "@react-native-community/cli-platform-android": "^18.0.0",
     "@react-native-community/cli-platform-ios": "^18.0.0",
+    "@react-native-node-api/node-addon-examples": "*",
+    "@react-native-node-api/node-tests": "*",
     "@react-native/babel-preset": "0.79.0",
     "@react-native/metro-config": "0.79.0",
     "@react-native/typescript-config": "0.79.0",

apps/test-app/react-native.config.js

@@ -1,4 +1,3 @@
-
 const project = (() => {
   try {
     const { configureProjects } = require("react-native-test-app");
@@ -25,4 +24,4 @@ const project = (() => {
 
 module.exports = {
   ...(project ? { project } : undefined),
-};
+};

docs/ANDROID.md

@@ -4,6 +4,35 @@
 
 Because we're using a version of Hermes patched with Node-API support, we need to build React Native from source.
 
+Follow [the React Native documentation on how to build from source](https://reactnative.dev/contributing/how-to-build-from-source#update-your-project-to-build-from-source).
+
+In particular, you will have to edit the `android/settings.gradle` file as follows:
+
+> ```diff
+> // ...
+> include ':app'
+> includeBuild('../node_modules/@react-native/gradle-plugin')
+>
+> + includeBuild('../node_modules/react-native') {
+> +     dependencySubstitution {
+> +         substitute(module("com.facebook.react:react-android")).using(project(":packages:react-native:ReactAndroid"))
+> +         substitute(module("com.facebook.react:react-native")).using(project(":packages:react-native:ReactAndroid"))
+> +         substitute(module("com.facebook.react:hermes-android")).using(project(":packages:react-native:ReactAndroid:hermes-engine"))
+> +         substitute(module("com.facebook.react:hermes-engine")).using(project(":packages:react-native:ReactAndroid:hermes-engine"))
+> +     }
+> + }
+> ```
+
+To download our custom version of Hermes, you need to run from your app package:
+
+```
+npx react-native-node-api vendor-hermes
+```
+
+This will print a path which needs to be stored in `REACT_NATIVE_OVERRIDE_HERMES_DIR` to instruct the React Native Gradle scripts to use it.
+
+This can be combined into a single line:
+
 ```
 export REACT_NATIVE_OVERRIDE_HERMES_DIR=`npx react-native-node-api vendor-hermes --silent`
 ```

docs/HOW-IT-WORKS.md

@@ -25,22 +25,14 @@ The generated code looks something like this:
 
 ```javascript
 module.exports = require("react-native-node-api").requireNodeAddon(
-  "calculator-lib--prebuild"
+  "calculator-lib--prebuild",
 );
 ```
 
 > [!NOTE]
 > In the time of writing, this code only supports iOS as passes the path to the library with its .framework.
 > We plan on generalizing this soon 🤞
 
-### A note on the need for path-hashing
-
-Notice that the `requireNodeAddon` call doesn't reference the library by it's original name (`prebuild.node`) but instead a name containing a hash.
-
-In Node.js dynamic libraries sharing names can be disambiguated based off their path on disk. Dynamic libraries added to an iOS application are essentially hoisted and occupy a shared global namespace. This leads to collisions and makes it impossible to disambiguate multiple libraries sharing the same name. We need a way to map a require call, referencing the library by its path relative to the JS file, into a unique name of the library once it's added into the application.
-
-To work around this issue, we scan for and copy any library (including its entire xcframework structure with nested framework directories) from the dependency package into our host package when the app builds and reference these from its podspec (as vendored_frameworks). We use a special file in the xcframeworks containing Node-API modules. To avoid collisions we rename xcframework, framework and library files to a unique name, containing a hash. The hash is computed based off the package-name of the containing package and the relative path from the package root to the library file (with any platform specific file extensions replaced with the neutral ".node" extension).
-
 ## Transformed code calls into `react-native-node-api`, loading the platform specific dynamic library
 
 The native implementation of `requireNodeAddon` is responsible for loading the dynamic library and allow the Node-API module to register its initialization function, either by exporting a `napi_register_module_v1` function or by calling the (deprecated) `napi_module_register` function.

docs/USAGE.md

@@ -17,16 +17,15 @@ The app developer has to install both `calculator-lib` as well as `react-native-
 The reason for the latter is a current limitation of the React Native Community CLI which doesn't consider transitive dependencies when enumerating packages for auto-linking.
 
 > [!WARNING]
-> It's important to match the exact version of the `react-native-node-api` declared as peer dependency by `calculator-lib`.
+> It's important to match the version range of the `react-native-node-api` declared as a peer dependency by `calculator-lib`.
 
-For the app to resolve the Node-API dynamic library files, the app developer must update their Metro config to use a `resolveRequest` function exported from `react-native-node-api`:
+For the app to resolve the Node-API dynamic library files, the app developer must update their Babel config to use a `requireNodeAddon` function exported from `react-native-node-api`:
 
 ```javascript
-const { getDefaultConfig, mergeConfig } = require("@react-native/metro-config");
-const nodeApi = require("react-native-node-api/metro-config");
-module.exports = mergeConfig(getDefaultConfig(__dirname), {
-  resolver: { resolveRequest: nodeApi.resolveRequest },
-});
+module.exports = {
+  presets: ["module:@react-native/babel-preset"],
+  plugins: ["module:react-native-node-api/babel-plugin"], // 👈 This needs to be added to the babel.config.js of the app
+};
 ```
 
 At some point the app code will import (or require) the entrypoint of `calculator-lib`:
@@ -121,18 +120,36 @@ NAPI_MODULE_INIT(/* napi_env env, napi_value exports */) {
 }
 ```
 
-### Build the prebuilt binaries
+```cmake
+# CMakeLists.txt
 
-```
-npx react-native-node-api build ./addon.c
+cmake_minimum_required(VERSION 3.15...3.31)
+project(addon)
+
+add_compile_definitions(-DNAPI_VERSION=4)
+
+file(GLOB SOURCE_FILES "addon.c")
+
+add_library(${PROJECT_NAME} SHARED ${SOURCE_FILES} ${CMAKE_JS_SRC})
+set_target_properties(${PROJECT_NAME} PROPERTIES PREFIX "" SUFFIX ".node")
+target_include_directories(${PROJECT_NAME} PRIVATE ${CMAKE_JS_INC})
+target_link_libraries(${PROJECT_NAME} PRIVATE ${CMAKE_JS_LIB})
+target_compile_features(${PROJECT_NAME} PRIVATE cxx_std_17)
+
+if(MSVC AND CMAKE_JS_NODELIB_DEF AND CMAKE_JS_NODELIB_TARGET)
+  # Generate node.lib
+  execute_process(COMMAND ${CMAKE_AR} /def:${CMAKE_JS_NODELIB_DEF} /out:${CMAKE_JS_NODELIB_TARGET} ${CMAKE_STATIC_LINKER_FLAGS})
+endif()
 ```
 
-This is a shorthand command which generates a CMake project from the single source-file and prebuilds for both the Apple and Android platforms. See the [CLI documentation](./CLI.md) for more information on the options available and [documentation on prebuilds](./PREBUILDS.md) for the specifics on their format and structure.
+### Build the prebuilt binaries
 
-<!-- TODO: Add a listing of the files produced when running command: Some temp (cached) CMakeList.txt, the CMake project dir, 2x platform specific prebuild directories -->
+```
+npx cmake-rn
+```
 
 ### Load and export the native module
 
 ```javascript
-module.exports = require("./prebuild.node");
+module.exports = require("./build/Release/addon.node");
 ```