Refactor existing platforms split node-api headers from weak-node-api

Author: kraenhansen

packages/cmake-rn/src/cli.ts

@@ -8,7 +8,6 @@ import { spawn, SpawnFailure } from "bufout";
 import { oraPromise } from "ora";
 import chalk from "chalk";
 
-import { getWeakNodeApiVariables } from "./weak-node-api.js";
 import {
   platforms,
   allTargets,
@@ -17,7 +16,6 @@ import {
 } from "./platforms.js";
 import { toDeclarationArguments } from "./cmake.js";
 import { BaseOpts, TargetContext, Platform } from "./platforms/types.js";
-import { isSupportedTriplet } from "react-native-node-api";
 
 // We're attaching a lot of listeners when spawning in parallel
 EventEmitter.defaultMaxListeners = 100;
@@ -261,18 +259,7 @@ async function configureProject<T extends string>(
   options: BaseOpts,
 ) {
   const { target, buildPath, outputPath } = context;
-  const { verbose, source, weakNodeApiLinkage } = options;
-
-  const nodeApiVariables =
-    weakNodeApiLinkage && isSupportedTriplet(target)
-      ? getWeakNodeApiVariables(target)
-      : // TODO: Make this a part of the platform definition
-        {};
-
-  const declarations = {
-    ...nodeApiVariables,
-    CMAKE_LIBRARY_OUTPUT_DIRECTORY: outputPath,
-  };
+  const { verbose, source } = options;
 
   await spawn(
     "cmake",
@@ -282,7 +269,9 @@ async function configureProject<T extends string>(
       "-B",
       buildPath,
       ...platform.configureArgs(context, options),
-      ...toDeclarationArguments(declarations),
+      ...toDeclarationArguments({
+        CMAKE_LIBRARY_OUTPUT_DIRECTORY: outputPath,
+      }),
     ],
     {
       outputMode: verbose ? "inherit" : "buffered",

packages/cmake-rn/src/headers.ts

@@ -45,3 +45,18 @@ export function getNodeAddonHeadersPath(): string {
     );
   }
 }
+
+/**
+ * @returns A semicolon-separated list of include directories for Node-API headers.
+ * @note Using semicolon as a path separator for CMake regardless of platform
+ */
+export function getNodeApiIncludeDirectories(): string {
+  const includePaths = [getNodeApiHeadersPath(), getNodeAddonHeadersPath()];
+  for (const includePath of includePaths) {
+    assert(
+      !includePath.includes(";"),
+      `Include path with a ';' is not supported: ${includePath}`,
+    );
+  }
+  return includePaths.join(";");
+}

packages/cmake-rn/src/platforms/android.ts

@@ -12,6 +12,8 @@ import {
 import type { Platform } from "./types.js";
 import { oraPromise } from "ora";
 import chalk from "chalk";
+import { getWeakNodeApiVariables } from "../weak-node-api.js";
+import { toDeclarationArguments } from "../cmake.js";
 
 // This should match https://github.com/react-native-community/template/blob/main/template/android/build.gradle#L7
 const DEFAULT_NDK_VERSION = "27.1.12297006";
@@ -61,7 +63,10 @@ export const platform: Platform<Target[], AndroidOpts> = {
       .addOption(ndkVersionOption)
       .addOption(androidSdkVersionOption);
   },
-  configureArgs({ target }, { ndkVersion, androidSdkVersion }) {
+  configureArgs(
+    { target },
+    { ndkVersion, androidSdkVersion, weakNodeApiLinkage },
+  ) {
     const { ANDROID_HOME } = process.env;
     assert(
       typeof ANDROID_HOME === "string",
@@ -82,38 +87,26 @@ export const platform: Platform<Target[], AndroidOpts> = {
       ndkPath,
       "build/cmake/android.toolchain.cmake",
     );
-    const architecture = ANDROID_ARCHITECTURES[target];
 
     return [
       "-G",
       "Ninja",
       "--toolchain",
       toolchainPath,
-      "-D",
-      "CMAKE_SYSTEM_NAME=Android",
-      // "-D",
-      // `CPACK_SYSTEM_NAME=Android-${architecture}`,
-      // "-D",
-      // `CMAKE_INSTALL_PREFIX=${installPath}`,
-      // "-D",
-      // `CMAKE_BUILD_TYPE=${configuration}`,
-      "-D",
-      "CMAKE_MAKE_PROGRAM=ninja",
-      // "-D",
-      // "CMAKE_C_COMPILER_LAUNCHER=ccache",
-      // "-D",
-      // "CMAKE_CXX_COMPILER_LAUNCHER=ccache",
-      "-D",
-      `ANDROID_NDK=${ndkPath}`,
-      "-D",
-      `ANDROID_ABI=${architecture}`,
-      "-D",
-      "ANDROID_TOOLCHAIN=clang",
-      "-D",
-      `ANDROID_PLATFORM=${androidSdkVersion}`,
-      "-D",
-      // TODO: Make this configurable
-      "ANDROID_STL=c++_shared",
+      ...toDeclarationArguments({
+        CMAKE_SYSTEM_NAME: "Android",
+        CMAKE_MAKE_PROGRAM: "ninja",
+        // "CMAKE_BUILD_TYPE": configuration,
+        // "CMAKE_C_COMPILER_LAUNCHER": "ccache",
+        // "CMAKE_CXX_COMPILER_LAUNCHER": "ccache",
+        ANDROID_NDK: ndkPath,
+        ANDROID_ABI: ANDROID_ARCHITECTURES[target],
+        ANDROID_TOOLCHAIN: "clang",
+        ANDROID_PLATFORM: androidSdkVersion,
+        // TODO: Make this configurable
+        ANDROID_STL: "c++_shared",
+        ...(weakNodeApiLinkage ? getWeakNodeApiVariables(target) : {}),
+      }),
     ];
   },
   buildArgs() {

packages/cmake-rn/src/platforms/apple.ts

@@ -13,6 +13,8 @@ import {
 
 import type { Platform } from "./types.js";
 import chalk from "chalk";
+import { toDeclarationArguments } from "../cmake.js";
+import { getWeakNodeApiVariables } from "../weak-node-api.js";
 
 type XcodeSDKName =
   | "iphoneos"
@@ -114,16 +116,16 @@ export const platform: Platform<Target[], AppleOpts> = {
   amendCommand(command) {
     return command.addOption(xcframeworkExtensionOption);
   },
-  configureArgs({ target }) {
+  configureArgs({ target }, { weakNodeApiLinkage }) {
     return [
       "-G",
       "Xcode",
-      "-D",
-      `CMAKE_SYSTEM_NAME=${CMAKE_SYSTEM_NAMES[target]}`,
-      "-D",
-      `CMAKE_OSX_SYSROOT=${XCODE_SDK_NAMES[target]}`,
-      "-D",
-      `CMAKE_OSX_ARCHITECTURES=${APPLE_ARCHITECTURES[target]}`,
+      ...toDeclarationArguments({
+        CMAKE_SYSTEM_NAME: CMAKE_SYSTEM_NAMES[target],
+        CMAKE_OSX_SYSROOT: XCODE_SDK_NAMES[target],
+        CMAKE_OSX_ARCHITECTURES: APPLE_ARCHITECTURES[target],
+        ...(weakNodeApiLinkage ? getWeakNodeApiVariables(target) : {}),
+      }),
     ];
   },
   buildArgs() {

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

@@ -10,7 +10,7 @@ import {
 } from "react-native-node-api";
 
 import { ANDROID_ARCHITECTURES } from "./platforms/android.js";
-import { getNodeAddonHeadersPath, getNodeApiHeadersPath } from "./headers.js";
+import { getNodeApiIncludeDirectories } from "./headers.js";
 
 export function toCmakePath(input: string) {
   return input.split(path.win32.sep).join(path.posix.sep);
@@ -42,15 +42,9 @@ export function getWeakNodeApiPath(triplet: SupportedTriplet): string {
 }
 
 export function getWeakNodeApiVariables(triplet: SupportedTriplet) {
-  const includePaths = [getNodeApiHeadersPath(), getNodeAddonHeadersPath()];
-  for (const includePath of includePaths) {
-    assert(
-      !includePath.includes(";"),
-      `Include path with a ';' is not supported: ${includePath}`,
-    );
-  }
   return {
-    CMAKE_JS_INC: includePaths.join(";"),
+    // TODO: Make these names less "cmake-js" specific with an option to use the CMAKE_JS prefix
+    CMAKE_JS_INC: getNodeApiIncludeDirectories(),
     CMAKE_JS_LIB: getWeakNodeApiPath(triplet),
   };
 }