diff --git a/.changeset/calm-bears-resolve.md b/.changeset/calm-bears-resolve.md new file mode 100644 index 00000000..b274b102 --- /dev/null +++ b/.changeset/calm-bears-resolve.md @@ -0,0 +1,5 @@ +--- +"react-native-node-api": patch +--- + +Preserve Node.js module resolution precedence when a JavaScript file and native addon share a basename: `require('./foo')` no longer gets rewritten to load a Node-API addon when a same-named `foo.js`/`.cjs`/`.mjs`/`.json` file exists alongside it, since that source file is what `require()` actually resolves to. An explicit `require('./foo.node')` is unaffected. diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 478d8634..59193362 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -199,6 +199,8 @@ jobs: env: CMAKE_RN_TRIPLETS: arm64;x86_64-apple-ios-sim FERRIC_TARGETS: aarch64-apple-ios-sim,x86_64-apple-ios + - name: Verify prebuilds + run: pnpm --filter @react-native-node-api/node-addon-examples run verify # Mirrors React Native's scripts/xcode/ccache.conf, which only applied # while ccache ran through their wrapper script (it sets CCACHE_CONFIGPATH # to it) and so no longer does now that it runs as a compiler launcher. diff --git a/packages/host/src/node/babel-plugin/plugin.test.ts b/packages/host/src/node/babel-plugin/plugin.test.ts index 7dae2979..d3fe5fac 100644 --- a/packages/host/src/node/babel-plugin/plugin.test.ts +++ b/packages/host/src/node/babel-plugin/plugin.test.ts @@ -129,7 +129,7 @@ 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.node": "// This is supposed to be a binary file", "my-addon.js": "// Some JS file", "index.js": ` const addon = require('./my-addon'); diff --git a/packages/host/src/node/path-utils.test.ts b/packages/host/src/node/path-utils.test.ts index 7d8e62fd..66419024 100644 --- a/packages/host/src/node/path-utils.test.ts +++ b/packages/host/src/node/path-utils.test.ts @@ -66,6 +66,18 @@ describe("isNodeApiModule", () => { assert(isNodeApiModule(path.join(tempDirectoryPath, "addon.node"))); }); + it("does not treat .cjs or .mjs files as shadowing .node", (context) => { + const tempDirectoryPath = setupTempDirectory(context, { + "cjs-addon.cjs": "// Some CommonJS file", + "cjs-addon.node": "// This is supposed to be a binary file", + "mjs-addon.mjs": "// Some ES module file", + "mjs-addon.node": "// This is supposed to be a binary file", + }); + + assert(isNodeApiModule(path.join(tempDirectoryPath, "cjs-addon"))); + assert(isNodeApiModule(path.join(tempDirectoryPath, "mjs-addon"))); + }); + it( "returns false when directory cannot be read due to permissions", // Skipping on Windows because there is no way to set ACLs on directories in Node.js on Windows without brittle powershell commands diff --git a/packages/host/src/node/path-utils.ts b/packages/host/src/node/path-utils.ts index 859de2de..9ab3ac2f 100644 --- a/packages/host/src/node/path-utils.ts +++ b/packages/host/src/node/path-utils.ts @@ -59,6 +59,9 @@ export type NamingStrategy = { // Cache mapping package directory to package name across calls const packageNameCache = new Map(); +// Extensions Node's own require() resolves before ever trying `.node`. +const COLLIDING_SOURCE_EXTENSIONS = [".js", ".json"]; + /** * @param modulePath Batch-scans the path to the module to check (must be extensionless or end in .node) * @returns True if a platform specific prebuild exists for the module path, warns on unreadable modules. @@ -66,6 +69,15 @@ const packageNameCache = new Map(); * TODO: Consider checking for a specific platform extension. */ export function isNodeApiModule(modulePath: string): boolean { + if ( + !modulePath.endsWith(".node") && + COLLIDING_SOURCE_EXTENSIONS.some((extension) => + fs.existsSync(modulePath + extension), + ) + ) { + // An explicit require('./foo.node') has no such ambiguity to defer to. + return false; + } { // HACK: Take a shortcut (if applicable): existing `.node` files are addons try { diff --git a/packages/node-addon-examples/package.json b/packages/node-addon-examples/package.json index acfd70ec..a30cddeb 100644 --- a/packages/node-addon-examples/package.json +++ b/packages/node-addon-examples/package.json @@ -30,14 +30,17 @@ "bootstrap": "node --run copy-and-build" }, "devDependencies": { + "@expo/plist": "0.4.7", + "bplist-parser": "^0.5.0", "cmake-rn": "workspace:*", - "node-addon-examples": "github:nodejs/node-addon-examples#4b7dd86a85644610e6de80154df9acac9329b509", "gyp-to-cmake": "workspace:*", + "node-addon-examples": "github:nodejs/node-addon-examples#4b7dd86a85644610e6de80154df9acac9329b509", "read-pkg": "^9.0.1", "weak-node-api": "workspace:*" }, "dependencies": { "assert": "^2.1.0", - "react-native-node-api": "workspace:*" + "react-native-node-api": "workspace:*", + "zod": "^4.1.11" } } diff --git a/packages/node-addon-examples/scripts/verify-prebuilds.mts b/packages/node-addon-examples/scripts/verify-prebuilds.mts index 94b4e1bb..0a27b9d8 100644 --- a/packages/node-addon-examples/scripts/verify-prebuilds.mts +++ b/packages/node-addon-examples/scripts/verify-prebuilds.mts @@ -2,19 +2,29 @@ import fs from "node:fs"; import assert from "node:assert/strict"; import path from "node:path"; +import plistModule from "@expo/plist"; +import { parseBuffer } from "bplist-parser"; +import { z } from "zod"; + import { DIRS } from "./cmake-projects.mjs"; -const EXPECTED_ANDROID_ARCHS = ["armeabi-v7a", "arm64-v8a", "x86_64", "x86"]; +// @expo/plist is CJS with an `export default`; under this genuine ESM +// (.mts) module's interop, the default import binds to the whole +// `module.exports`, nesting the real API one `.default` deeper. +const plist = plistModule.default; + +const FrameworkInfoPlistSchema = z.object({ + CFBundleExecutable: z.string(), +}); -const EXPECTED_XCFRAMEWORK_PLATFORMS = [ - "ios-arm64", - "ios-arm64-simulator", - "macos-arm64_x86_64", - "tvos-arm64", - "tvos-arm64-simulator", - "xros-arm64", - "xros-arm64-simulator", -]; +const KNOWN_ANDROID_ARCHS = ["armeabi-v7a", "arm64-v8a", "x86_64", "x86"]; + +async function readSubdirectories(dir: string) { + const entries = await fs.promises.readdir(dir, { withFileTypes: true }); + const subdirectories = entries.filter((entry) => entry.isDirectory()); + assert(subdirectories.length > 0, `Expected at least one slice in ${dir}`); + return subdirectories; +} async function verifyAndroidPrebuild(dirent: fs.Dirent) { console.log( @@ -23,8 +33,13 @@ async function verifyAndroidPrebuild(dirent: fs.Dirent) { "in", dirent.parentPath, ); - for (const arch of EXPECTED_ANDROID_ARCHS) { - const archDir = path.join(dirent.parentPath, dirent.name, arch); + const prebuildDir = path.join(dirent.parentPath, dirent.name); + for (const arch of await readSubdirectories(prebuildDir)) { + assert( + KNOWN_ANDROID_ARCHS.includes(arch.name), + `Unexpected Android arch: ${arch.name}`, + ); + const archDir = path.join(prebuildDir, arch.name); for (const file of await fs.promises.readdir(archDir, { withFileTypes: true, })) { @@ -37,17 +52,46 @@ async function verifyAndroidPrebuild(dirent: fs.Dirent) { } } +async function readPlist(plistPath: string): Promise { + const contents = await fs.promises.readFile(plistPath); + // xcodebuild emits binary plists, while the host writes XML ones + if (contents.subarray(0, 6).toString("ascii") === "bplist") { + const [parsed] = parseBuffer(contents); + return parsed; + } + return plist.parse(contents.toString("utf8")); +} + +async function verifyFrameworkInfoPlist(frameworkDir: string) { + const libraryName = path.basename(frameworkDir, ".framework"); + const infoPlistPath = path.join(frameworkDir, "Info.plist"); + const parsed = FrameworkInfoPlistSchema.parse(await readPlist(infoPlistPath)); + assert.equal( + parsed.CFBundleExecutable, + libraryName, + `Unexpected CFBundleExecutable in ${infoPlistPath}`, + ); + assert( + fs.existsSync(path.join(frameworkDir, libraryName)), + `Missing executable ${libraryName} in ${frameworkDir}`, + ); +} + async function verifyApplePrebuild(dirent: fs.Dirent) { console.log("Verifying Apple prebuild", dirent.name, "in", dirent.parentPath); - for (const arch of EXPECTED_XCFRAMEWORK_PLATFORMS) { - const archDir = path.join(dirent.parentPath, dirent.name, arch); - for (const file of await fs.promises.readdir(archDir, { + const prebuildDir = path.join(dirent.parentPath, dirent.name); + for (const slice of await readSubdirectories(prebuildDir)) { + const sliceDir = path.join(prebuildDir, slice.name); + for (const file of await fs.promises.readdir(sliceDir, { withFileTypes: true, })) { assert( file.isDirectory(), "Expected only directories in xcframework arch directory", ); + if (file.name === "dSYMs") { + continue; + } assert(file.name.endsWith(".framework"), "Expected framework directory"); const frameworkDir = path.join(file.parentPath, file.name); for (const file of await fs.promises.readdir(frameworkDir, { @@ -65,8 +109,7 @@ 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 - continue; + await verifyFrameworkInfoPlist(frameworkDir); } else { assert( !file.name.endsWith(".node"), diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9632a67c..135d5ff1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -280,7 +280,16 @@ importers: react-native-node-api: specifier: workspace:* version: link:../host + zod: + specifier: ^4.1.11 + version: 4.4.3 devDependencies: + '@expo/plist': + specifier: 0.4.7 + version: 0.4.7 + bplist-parser: + specifier: ^0.5.0 + version: 0.5.0 cmake-rn: specifier: workspace:* version: link:../cmake-rn @@ -289,7 +298,7 @@ importers: version: link:../gyp-to-cmake node-addon-examples: specifier: github:nodejs/node-addon-examples#4b7dd86a85644610e6de80154df9acac9329b509 - version: git+https://git@github.com:nodejs/node-addon-examples.git#4b7dd86a85644610e6de80154df9acac9329b509 + version: https://codeload.github.com/nodejs/node-addon-examples/tar.gz/4b7dd86a85644610e6de80154df9acac9329b509 read-pkg: specifier: ^9.0.1 version: 9.0.1 @@ -2711,6 +2720,10 @@ packages: resolution: {integrity: sha512-2cGmJupaNgg+QUwVLAucDuWuoMZ6EX9iHDRswZ5lsNYEmwPaRknMPCLZz07yTzVq/83p4o/wzbDZbBrTvGGTIw==} engines: {node: '>=18'} + bplist-parser@0.5.0: + resolution: {integrity: sha512-owgQ0RGbndIGNrQkpHe99HVSslIK59t31nVyAPDRqhq9mNDc2kRl5HwYO292GuKjHqiJRdJNiaI63a02hjLFIQ==} + engines: {node: '>=20.19.0'} + brace-expansion@1.1.16: resolution: {integrity: sha512-IDw48K2/2kRkg9LdJxurvq3lV3aBgq0REY89duEqFRthjlPdXHKMj7EnQOXVckxzgisinf3nHfrcE2FufFLXMw==} @@ -4070,8 +4083,8 @@ packages: resolution: {integrity: sha512-ekZMeaaIzSQTSpr7X2X3iJM7lTzgnx8ahAG9pJfT/7+14mlEM8ZYQ9cgCDvSSRbReFK0oHli3WrZdCiRsgAT9Q==} engines: {node: ^18 || ^20 || >= 21} - node-addon-examples@git+https://git@github.com:nodejs/node-addon-examples.git#4b7dd86a85644610e6de80154df9acac9329b509: - resolution: {commit: 4b7dd86a85644610e6de80154df9acac9329b509, repo: git@github.com:nodejs/node-addon-examples.git, type: git} + node-addon-examples@https://codeload.github.com/nodejs/node-addon-examples/tar.gz/4b7dd86a85644610e6de80154df9acac9329b509: + resolution: {tarball: https://codeload.github.com/nodejs/node-addon-examples/tar.gz/4b7dd86a85644610e6de80154df9acac9329b509} version: 1.0.0 node-api-headers@1.9.0: @@ -7722,6 +7735,8 @@ snapshots: transitivePeerDependencies: - supports-color + bplist-parser@0.5.0: {} + brace-expansion@1.1.16: dependencies: balanced-match: 1.0.2 @@ -9270,7 +9285,7 @@ snapshots: node-addon-api@8.9.0: {} - node-addon-examples@git+https://git@github.com:nodejs/node-addon-examples.git#4b7dd86a85644610e6de80154df9acac9329b509: + node-addon-examples@https://codeload.github.com/nodejs/node-addon-examples/tar.gz/4b7dd86a85644610e6de80154df9acac9329b509: dependencies: chalk: 5.6.2 cmake-js: 7.4.0