diff --git a/.changeset/tidy-addon-precedence.md b/.changeset/tidy-addon-precedence.md new file mode 100644 index 00000000..cce06bdf --- /dev/null +++ b/.changeset/tidy-addon-precedence.md @@ -0,0 +1,5 @@ +--- +"react-native-node-api": patch +--- + +Preserve JavaScript file resolution when an extensionless require has a sibling Node-API addon. diff --git a/package.json b/package.json index 55bec8a5..0a1f44a4 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "publint": "node scripts/run-in-published.ts pnpm exec publint --strict", "prettier:check": "prettier --experimental-cli --check .", "prettier:write": "prettier --experimental-cli --write .", - "test": "pnpm --filter react-native-node-api --filter cmake-rn --filter gyp-to-cmake run test", + "test": "pnpm --filter react-native-node-api --filter cmake-rn --filter gyp-to-cmake run test && pnpm --filter @react-native-node-api/node-addon-examples run test:verify", "bootstrap": "node --run build && pnpm --recursive run bootstrap", "changeset": "changeset", "release": "node --run prerelease && changeset publish", diff --git a/packages/host/src/node/babel-plugin/plugin.test.ts b/packages/host/src/node/babel-plugin/plugin.test.ts index 7dae2979..dd75272b 100644 --- a/packages/host/src/node/babel-plugin/plugin.test.ts +++ b/packages/host/src/node/babel-plugin/plugin.test.ts @@ -129,8 +129,8 @@ describe("plugin", () => { itTransforms("and does not touch required JS files", { files: { "package.json": `{ "name": "my-package" }`, - // TODO: Add a ./my-addon.node to make this test complete "my-addon.js": "// Some JS file", + "my-addon.node": "// This is supposed to be a binary file", "index.js": ` const addon = require('./my-addon'); console.log(addon); diff --git a/packages/host/src/node/babel-plugin/plugin.ts b/packages/host/src/node/babel-plugin/plugin.ts index 45e269e9..284ab7dc 100644 --- a/packages/host/src/node/babel-plugin/plugin.ts +++ b/packages/host/src/node/babel-plugin/plugin.ts @@ -1,4 +1,5 @@ import assert from "node:assert/strict"; +import { createRequire } from "node:module"; import path from "node:path"; import type { PluginObj, NodePath } from "@babel/core"; @@ -66,6 +67,14 @@ export function replaceWithRequireNodeAddon( ); } +function resolvesToNonAddon(id: string, filename: string): boolean { + try { + return !createRequire(path.resolve(filename)).resolve(id).endsWith(".node"); + } catch { + return false; + } +} + export function plugin(): PluginObj { return { visitor: { @@ -101,7 +110,8 @@ export function plugin(): PluginObj { } } else if ( !path.isAbsolute(id) && - isNodeApiModule(path.join(from, id)) + isNodeApiModule(path.join(from, id)) && + !resolvesToNonAddon(id, this.filename) ) { const relativePath = path.join(from, id); replaceWithRequireNodeAddon(p, relativePath, { diff --git a/packages/node-addon-examples/package.json b/packages/node-addon-examples/package.json index 0d2d129b..3459607c 100644 --- a/packages/node-addon-examples/package.json +++ b/packages/node-addon-examples/package.json @@ -24,11 +24,13 @@ "gyp-to-cmake": "gyp-to-cmake --weak-node-api .", "build": "tsx scripts/build-examples.mts", "copy-and-build": "node --run copy-examples && node --run gyp-to-cmake && node --run build", + "test:verify": "tsx --test scripts/verify-prebuilds.test.mts", "verify": "tsx scripts/verify-prebuilds.mts", "test": "node --run copy-and-build && node --run verify", "bootstrap": "node --run copy-and-build" }, "devDependencies": { + "@expo/plist": "0.4.7", "cmake-rn": "workspace:*", "node-addon-examples": "github:nodejs/node-addon-examples#4b7dd86a85644610e6de80154df9acac9329b509", "gyp-to-cmake": "workspace:*", diff --git a/packages/node-addon-examples/scripts/verify-prebuilds.mts b/packages/node-addon-examples/scripts/verify-prebuilds.mts index cdbd312b..36fe6d1e 100644 --- a/packages/node-addon-examples/scripts/verify-prebuilds.mts +++ b/packages/node-addon-examples/scripts/verify-prebuilds.mts @@ -1,6 +1,9 @@ import fs from "node:fs"; import assert from "node:assert/strict"; import path from "node:path"; +import { pathToFileURL } from "node:url"; + +import { parse } from "@expo/plist/build/parse.js"; import { EXAMPLES_DIR } from "./cmake-projects.mjs"; @@ -16,6 +19,32 @@ const EXPECTED_XCFRAMEWORK_PLATFORMS = [ "xros-arm64-simulator", ]; +export async function verifyFrameworkInfoPlist( + infoPlistPath: string, + libraryName: string, +) { + const parsed: unknown = parse( + await fs.promises.readFile(infoPlistPath, "utf8"), + ); + assert( + typeof parsed === "object" && parsed !== null, + `Expected an object in ${infoPlistPath}`, + ); + assert.equal( + Reflect.get(parsed, "CFBundleExecutable"), + libraryName, + `Unexpected CFBundleExecutable in ${infoPlistPath}`, + ); + assert.equal( + Reflect.get(parsed, "CFBundleIdentifier"), + `com.callstackincubator.node-api.${libraryName}`.replace( + /[^A-Za-z0-9-.]/g, + "-", + ), + `Unexpected CFBundleIdentifier in ${infoPlistPath}`, + ); +} + async function verifyAndroidPrebuild(dirent: fs.Dirent) { console.log( "Verifying Android prebuild", @@ -65,7 +94,11 @@ async function verifyApplePrebuild(dirent: fs.Dirent) { "Expected only directory and files in framework", ); if (file.name === "Info.plist") { - // TODO: Verify the contents of the Info.plist file + const libraryName = path.basename(frameworkDir, ".framework"); + await verifyFrameworkInfoPlist( + path.join(frameworkDir, file.name), + libraryName, + ); continue; } else { assert( @@ -82,17 +115,26 @@ async function verifyApplePrebuild(dirent: fs.Dirent) { } } -for await (const dirent of fs.promises.glob("**/*.*.node", { - cwd: EXAMPLES_DIR, - withFileTypes: true, -})) { - if (dirent.name.endsWith(".android.node")) { - await verifyAndroidPrebuild(dirent); - } else if (dirent.name.endsWith(".apple.node")) { - await verifyApplePrebuild(dirent); - } else { - throw new Error( - `Unexpected prebuild file: ${dirent.name} in ${dirent.parentPath}`, - ); +async function main() { + for await (const dirent of fs.promises.glob("**/*.*.node", { + cwd: EXAMPLES_DIR, + withFileTypes: true, + })) { + if (dirent.name.endsWith(".android.node")) { + await verifyAndroidPrebuild(dirent); + } else if (dirent.name.endsWith(".apple.node")) { + await verifyApplePrebuild(dirent); + } else { + throw new Error( + `Unexpected prebuild file: ${dirent.name} in ${dirent.parentPath}`, + ); + } } } + +if ( + process.argv[1] && + import.meta.url === pathToFileURL(path.resolve(process.argv[1])).href +) { + await main(); +} diff --git a/packages/node-addon-examples/scripts/verify-prebuilds.test.mts b/packages/node-addon-examples/scripts/verify-prebuilds.test.mts new file mode 100644 index 00000000..26e26f9d --- /dev/null +++ b/packages/node-addon-examples/scripts/verify-prebuilds.test.mts @@ -0,0 +1,65 @@ +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { describe, it } from "node:test"; + +import { build } from "@expo/plist/build/build.js"; + +import { verifyFrameworkInfoPlist } from "./verify-prebuilds.mjs"; + +async function writeInfoPlist( + directory: string, + contents: Record, +) { + const infoPlistPath = path.join(directory, "Info.plist"); + await fs.promises.writeFile(infoPlistPath, build(contents), "utf8"); + return infoPlistPath; +} + +describe("verifyFrameworkInfoPlist", () => { + it("accepts the expected executable and escaped bundle identifier", async (context) => { + const directory = await fs.promises.mkdtemp( + path.join(os.tmpdir(), "verify-framework-plist-"), + ); + context.after(() => fs.promises.rm(directory, { recursive: true })); + const infoPlistPath = await writeInfoPlist(directory, { + CFBundleExecutable: "my_addon", + CFBundleIdentifier: "com.callstackincubator.node-api.my-addon", + }); + + await verifyFrameworkInfoPlist(infoPlistPath, "my_addon"); + }); + + it("rejects an unexpected executable", async (context) => { + const directory = await fs.promises.mkdtemp( + path.join(os.tmpdir(), "verify-framework-plist-"), + ); + context.after(() => fs.promises.rm(directory, { recursive: true })); + const infoPlistPath = await writeInfoPlist(directory, { + CFBundleExecutable: "wrong-addon", + CFBundleIdentifier: "com.callstackincubator.node-api.my-addon", + }); + + await assert.rejects( + () => verifyFrameworkInfoPlist(infoPlistPath, "my-addon"), + /Unexpected CFBundleExecutable/, + ); + }); + + it("rejects an unexpected bundle identifier", async (context) => { + const directory = await fs.promises.mkdtemp( + path.join(os.tmpdir(), "verify-framework-plist-"), + ); + context.after(() => fs.promises.rm(directory, { recursive: true })); + const infoPlistPath = await writeInfoPlist(directory, { + CFBundleExecutable: "my-addon", + CFBundleIdentifier: "com.example.wrong", + }); + + await assert.rejects( + () => verifyFrameworkInfoPlist(infoPlistPath, "my-addon"), + /Unexpected CFBundleIdentifier/, + ); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a011bada..d2fb4d83 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -281,6 +281,9 @@ importers: specifier: workspace:* version: link:../host devDependencies: + '@expo/plist': + specifier: 0.4.7 + version: 0.4.7 cmake-rn: specifier: workspace:* version: link:../cmake-rn