Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/calm-bears-resolve.md
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 2 additions & 0 deletions .github/workflows/check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion packages/host/src/node/babel-plugin/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
12 changes: 12 additions & 0 deletions packages/host/src/node/path-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions packages/host/src/node/path-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,25 @@ export type NamingStrategy = {
// Cache mapping package directory to package name across calls
const packageNameCache = new Map<string, string>();

// 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.
* @throws If the parent directory cannot be read, or if a detected module is unreadable.
* 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 {
Expand Down
7 changes: 5 additions & 2 deletions packages/node-addon-examples/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
77 changes: 60 additions & 17 deletions packages/node-addon-examples/scripts/verify-prebuilds.mts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
})) {
Expand All @@ -37,17 +52,46 @@ async function verifyAndroidPrebuild(dirent: fs.Dirent) {
}
}

async function readPlist(plistPath: string): Promise<unknown> {
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<unknown>(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, {
Expand All @@ -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"),
Expand Down
23 changes: 19 additions & 4 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading