Explicit weak-node-api linkage

.changeset/cold-symbols-refuse.md

@@ -0,0 +1,5 @@
+---
+"gyp-to-cmake": minor
+---
+
+Add --weak-node-api option to emit CMake configuration for use with cmake-rn's default way of Node-API linkage.

.changeset/open-ducks-shop.md

@@ -0,0 +1,5 @@
+---
+"cmake-rn": minor
+---
+
+Breaking: `CMAKE_JS_*` defines are no longer injected by default (use --cmake-js to opt-in)

.changeset/three-colts-tie.md

@@ -0,0 +1,5 @@
+---
+"cmake-rn": minor
+---
+
+Expose includable WEAK_NODE_API_CONFIG to CMake projects

docs/WEAK-NODE-API.md

@@ -0,0 +1,7 @@
+# The `weak-node-api` library
+
+Android's dynamic linker imposes restrictions on the access to global symbols (such as the Node-API free functions): A dynamic library must explicitly declare any dependency bringing symbols it needs as `DT_NEEDED`.
+
+The implementation of Node-API is split between Hermes and our host package and to avoid addons having to explicitly link against either, we've introduced a `weak-node-api` library (published in `react-native-node-api` package). This library exposes only Node-API and will have its implementation injected by the host.
+
+While technically not a requirement on non-Android platforms, we choose to make this the general approach across React Native platforms. This keeps things aligned across platforms, while exposing just the Node-API without forcing libraries to build with suppression of errors for undefined symbols.

packages/cmake-rn/README.md

@@ -3,3 +3,24 @@
 A wrapper around Cmake making it easier to produce prebuilt binaries targeting iOS and Android matching [the prebuilt binary specification](https://github.com/callstackincubator/react-native-node-api/blob/main/docs/PREBUILDS.md).
 
 Serves the same purpose as `cmake-js` does for the Node.js community and could potentially be upstreamed into `cmake-js` eventually.
+
+## Linking against Node-API
+
+Android's dynamic linker imposes restrictions on the access to global symbols (such as the Node-API free functions): A dynamic library must explicitly declare any dependency bringing symbols it needs as `DT_NEEDED`.
+
+The implementation of Node-API is split between Hermes and our host package and to avoid addons having to explicitly link against either, we've introduced a `weak-node-api` library (published in `react-native-node-api` package). This library exposes only Node-API and will have its implementation injected by the host.
+
+To link against `weak-node-api` just include the CMake config exposed through `WEAK_NODE_API_CONFIG` and add `weak-node-api` to the `target_link_libraries` of the addon's library target.
+
+```cmake
+cmake_minimum_required(VERSION 3.15...3.31)
+project(tests-buffers)
+
+include(${WEAK_NODE_API_CONFIG})
+
+add_library(addon SHARED addon.c)
+target_link_libraries(addon PRIVATE weak-node-api)
+target_compile_features(addon PRIVATE cxx_std_20)
+```
+
+This is different from how `cmake-js` "injects" the Node-API for linking (via `${CMAKE_JS_INC}`, `${CMAKE_JS_SRC}` and `${CMAKE_JS_LIB}`). To allow for interoperability between these tools, we inject these when you pass `--cmake-js` to `cmake-rn`.

packages/cmake-rn/src/cli.ts

@@ -14,7 +14,10 @@ import {
 } from "@react-native-node-api/cli-utils";
 import { isSupportedTriplet } from "react-native-node-api";
 
-import { getWeakNodeApiVariables } from "./weak-node-api.js";
+import {
+  getCmakeJSVariables,
+  getWeakNodeApiVariables,
+} from "./weak-node-api.js";
 import {
   platforms,
   allTriplets as allTriplets,
@@ -85,8 +88,8 @@ const outPathOption = new Option(
 const defineOption = new Option(
   "-D,--define <entry...>",
   "Define cache variables passed when configuring projects",
-).argParser<Record<string, string | CmakeTypedDefinition>>(
-  (input, previous = {}) => {
+)
+  .argParser<Record<string, string>[]>((input, previous = []) => {
     // TODO: Implement splitting of value using a regular expression (using named groups) for the format <var>[:<type>]=<value>
     // and return an object keyed by variable name with the string value as value or alternatively an array of [value, type]
     const match = input.match(
@@ -98,9 +101,10 @@ const defineOption = new Option(
       );
     }
     const { name, type, value } = match.groups;
-    return { ...previous, [name]: type ? { value, type } : value };
-  },
-);
+    previous.push({ [type ? `${name}:${type}` : name]: value });
+    return previous;
+  })
+  .default([]);
 
 const targetOption = new Option(
   "--target <target...>",
@@ -117,6 +121,11 @@ const noWeakNodeApiLinkageOption = new Option(
   "Don't pass the path of the weak-node-api library from react-native-node-api",
 );
 
+const cmakeJsOption = new Option(
+  "--cmake-js",
+  "Define CMAKE_JS_* variables used for compatibility with cmake-js",
+).default(false);
+
 let program = new Command("cmake-rn")
   .description("Build React Native Node API modules with CMake")
   .addOption(tripletOption)
@@ -129,7 +138,8 @@ let program = new Command("cmake-rn")
   .addOption(cleanOption)
   .addOption(targetOption)
   .addOption(noAutoLinkOption)
-  .addOption(noWeakNodeApiLinkageOption);
+  .addOption(noWeakNodeApiLinkageOption)
+  .addOption(cmakeJsOption);
 
 for (const platform of platforms) {
   const allOption = new Option(
@@ -296,19 +306,26 @@ async function configureProject<T extends string>(
   options: BaseOpts,
 ) {
   const { triplet, buildPath, outputPath } = context;
-  const { verbose, source, weakNodeApiLinkage } = options;
+  const { verbose, source, weakNodeApiLinkage, cmakeJs } = options;
+
+  // TODO: Make the two following definitions a part of the platform definition
 
   const nodeApiDefinitions =
     weakNodeApiLinkage && isSupportedTriplet(triplet)
-      ? getWeakNodeApiVariables(triplet)
-      : // TODO: Make this a part of the platform definition
-        {};
+      ? [getWeakNodeApiVariables(triplet)]
+      : [];
 
-  const definitions = {
+  const cmakeJsDefinitions =
+    cmakeJs && isSupportedTriplet(triplet)
+      ? [getCmakeJSVariables(triplet)]
+      : [];
+
+  const definitions = [
     ...nodeApiDefinitions,
+    ...cmakeJsDefinitions,
     ...options.define,
-    CMAKE_LIBRARY_OUTPUT_DIRECTORY: outputPath,
-  };
+    { CMAKE_LIBRARY_OUTPUT_DIRECTORY: outputPath },
+  ];
 
   await spawn(
     "cmake",
@@ -352,18 +369,13 @@ async function buildProject<T extends string>(
   );
 }
 
-type CmakeTypedDefinition = { value: string; type: string };
-
-function toDefineArguments(
-  declarations: Record<string, string | CmakeTypedDefinition>,
-) {
-  return Object.entries(declarations).flatMap(([key, definition]) => {
-    if (typeof definition === "string") {
-      return ["-D", `${key}=${definition}`];
-    } else {
-      return ["-D", `${key}:${definition.type}=${definition.value}`];
-    }
-  });
+function toDefineArguments(declarations: Array<Record<string, string>>) {
+  return declarations.flatMap((values) =>
+    Object.entries(values).flatMap(([key, definition]) => [
+      "-D",
+      `${key}=${definition}`,
+    ]),
+  );
 }
 
 export { program };

packages/cmake-rn/src/weak-node-api.ts

@@ -41,16 +41,36 @@ export function getWeakNodeApiPath(triplet: SupportedTriplet): string {
   }
 }
 
-export function getWeakNodeApiVariables(triplet: SupportedTriplet) {
+function getNodeApiIncludePaths() {
   const includePaths = [getNodeApiHeadersPath(), getNodeAddonHeadersPath()];
   for (const includePath of includePaths) {
     assert(
       !includePath.includes(";"),
       `Include path with a ';' is not supported: ${includePath}`,
     );
   }
+  return includePaths;
+}
+
+export function getWeakNodeApiVariables(
+  triplet: SupportedTriplet,
+): Record<string, string> {
+  return {
+    // Expose an includable CMake config file declaring the weak-node-api target
+    WEAK_NODE_API_CONFIG: path.join(weakNodeApiPath, "weak-node-api.cmake"),
+    WEAK_NODE_API_INC: getNodeApiIncludePaths().join(";"),
+    WEAK_NODE_API_LIB: getWeakNodeApiPath(triplet),
+  };
+}
+
+/**
+ * For compatibility with cmake-js
+ */
+export function getCmakeJSVariables(
+  triplet: SupportedTriplet,
+): Record<string, string> {
   return {
-    CMAKE_JS_INC: includePaths.join(";"),
+    CMAKE_JS_INC: getNodeApiIncludePaths().join(";"),
     CMAKE_JS_LIB: getWeakNodeApiPath(triplet),
   };
 }

packages/gyp-to-cmake/src/cli.ts

@@ -62,25 +62,38 @@ export const program = new Command("gyp-to-cmake")
     "--no-path-transforms",
     "Don't transform output from command expansions (replacing '\\' with '/')",
   )
+  .option("--weak-node-api", "Link against the weak-node-api library", false)
+  .option("--define-napi-version", "Define NAPI_VERSION for all targets", false)
+  .option("--cpp <version>", "C++ standard version", "17")
   .argument(
     "[path]",
     "Path to the binding.gyp file or directory to traverse recursively",
     process.cwd(),
   )
   .action(
-    wrapAction((targetPath: string, { pathTransforms }) => {
-      const options: TransformOptions = {
-        unsupportedBehaviour: "throw",
-        disallowUnknownProperties: false,
-        transformWinPathsToPosix: pathTransforms,
-      };
-      const stat = fs.statSync(targetPath);
-      if (stat.isFile()) {
-        transformBindingGypFile(targetPath, options);
-      } else if (stat.isDirectory()) {
-        transformBindingGypsRecursively(targetPath, options);
-      } else {
-        throw new Error(`Expected either a file or a directory: ${targetPath}`);
-      }
-    }),
+    wrapAction(
+      (
+        targetPath: string,
+        { pathTransforms, cpp, defineNapiVersion, weakNodeApi },
+      ) => {
+        const options: TransformOptions = {
+          unsupportedBehaviour: "throw",
+          disallowUnknownProperties: false,
+          transformWinPathsToPosix: pathTransforms,
+          compileFeatures: cpp ? [`cxx_std_${cpp}`] : [],
+          defineNapiVersion,
+          weakNodeApi,
+        };
+        const stat = fs.statSync(targetPath);
+        if (stat.isFile()) {
+          transformBindingGypFile(targetPath, options);
+        } else if (stat.isDirectory()) {
+          transformBindingGypsRecursively(targetPath, options);
+        } else {
+          throw new Error(
+            `Expected either a file or a directory: ${targetPath}`,
+          );
+        }
+      },
+    ),
   );