Skip to content

Commit bcb987e

Browse files
committed
Close two test gaps: unverified prebuild Info.plist and a missing .node fixture
Closes #424. - verify-prebuilds.mts now reads the Info.plist it finds inside each .framework, asserting CFBundleExecutable matches the framework's library name and CFBundleIdentifier matches the default writeFrameworkInfoPlist derives (com.callstackincubator.node-api.<libraryName>, escaped), since none of the examples pass --apple-bundle-identifier. escapeBundleIdentifier is now exported from react-native-node-api's node entrypoint so the verifier (a consumer of the package, like any addon author) can reuse it. - The babel plugin's "does not touch required JS files" test now includes a sibling my-addon.apple.node/my-addon.node fixture alongside my-addon.js, per the TODO. That exposed a real bug: isNodeApiModule didn't check whether a same-named .js/.cjs/.mjs/.json file would already satisfy require() before considering .node prebuilds, so the plugin rewrote calls Node's own resolution would never route to the addon. Fixed by deferring to a colliding source file when the module path has no explicit .node extension. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1k6UQJPPaqKEKmnsRUatt
1 parent 0a29fbd commit bcb987e

7 files changed

Lines changed: 77 additions & 22 deletions

File tree

.changeset/loud-pandas-jump.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"react-native-node-api": patch
3+
---
4+
5+
Fixed the Babel plugin rewriting `require('./foo')` to load a Node-API addon
6+
even when a same-named `foo.js`/`.cjs`/`.mjs`/`.json` file exists alongside it
7+
— that source file is what Node's own `require()` resolves to, so the addon
8+
was never reachable at runtime through that specific call, only through an
9+
explicit `require('./foo.node')`.
10+
11+
Also exported `escapeBundleIdentifier` from the package's `node` entrypoint.

packages/host/src/node/babel-plugin/plugin.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,9 @@ describe("plugin", () => {
129129
itTransforms("and does not touch required JS files", {
130130
files: {
131131
"package.json": `{ "name": "my-package" }`,
132-
// TODO: Add a ./my-addon.node to make this test complete
133132
"my-addon.js": "// Some JS file",
133+
"my-addon.apple.node/my-addon.node":
134+
"// This is supposed to be a binary file",
134135
"index.js": `
135136
const addon = require('./my-addon');
136137
console.log(addon);

packages/host/src/node/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ export {
2020
createXCframework,
2121
createUniversalAppleLibrary,
2222
determineXCFrameworkFilename,
23+
escapeBundleIdentifier,
2324
} from "./prebuilds/apple.js";
2425

2526
export {

packages/host/src/node/path-utils.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,25 @@ export type NamingStrategy = {
5959
// Cache mapping package directory to package name across calls
6060
const packageNameCache = new Map<string, string>();
6161

62+
// Extensions Node's own require() resolves before ever trying `.node`.
63+
const COLLIDING_SOURCE_EXTENSIONS = [".js", ".cjs", ".mjs", ".json"];
64+
6265
/**
6366
* @param modulePath Batch-scans the path to the module to check (must be extensionless or end in .node)
6467
* @returns True if a platform specific prebuild exists for the module path, warns on unreadable modules.
6568
* @throws If the parent directory cannot be read, or if a detected module is unreadable.
6669
* TODO: Consider checking for a specific platform extension.
6770
*/
6871
export function isNodeApiModule(modulePath: string): boolean {
72+
if (
73+
!modulePath.endsWith(".node") &&
74+
COLLIDING_SOURCE_EXTENSIONS.some((extension) =>
75+
fs.existsSync(modulePath + extension),
76+
)
77+
) {
78+
// An explicit require('./foo.node') has no such ambiguity to defer to.
79+
return false;
80+
}
6981
{
7082
// HACK: Take a shortcut (if applicable): existing `.node` files are addons
7183
try {

packages/node-addon-examples/package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@
3737
"weak-node-api": "workspace:*"
3838
},
3939
"dependencies": {
40+
"@expo/plist": "0.4.7",
4041
"assert": "^2.1.0",
41-
"react-native-node-api": "workspace:*"
42+
"react-native-node-api": "workspace:*",
43+
"zod": "^4.1.11"
4244
}
4345
}

packages/node-addon-examples/scripts/verify-prebuilds.mts

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,22 @@ import fs from "node:fs";
22
import assert from "node:assert/strict";
33
import path from "node:path";
44

5+
import plistModule from "@expo/plist";
6+
import { escapeBundleIdentifier } from "react-native-node-api";
7+
import { z } from "zod";
8+
59
import { DIRS } from "./cmake-projects.mjs";
610

11+
// @expo/plist is CJS with an `export default`; under Node's ESM/CJS interop
12+
// the default import binds to the whole `module.exports`, which nests the
13+
// real API one `.default` deeper.
14+
const plist = plistModule.default;
15+
16+
const FrameworkInfoPlistSchema = z.object({
17+
CFBundleExecutable: z.string(),
18+
CFBundleIdentifier: z.string(),
19+
});
20+
721
const EXPECTED_ANDROID_ARCHS = ["armeabi-v7a", "arm64-v8a", "x86_64", "x86"];
822

923
const EXPECTED_XCFRAMEWORK_PLATFORMS = [
@@ -37,6 +51,27 @@ async function verifyAndroidPrebuild(dirent: fs.Dirent) {
3751
}
3852
}
3953

54+
async function verifyFrameworkInfoPlist(
55+
infoPlistPath: string,
56+
libraryName: string,
57+
) {
58+
const contents = await fs.promises.readFile(infoPlistPath, "utf8");
59+
const parsed = FrameworkInfoPlistSchema.parse(plist.parse(contents));
60+
assert.equal(
61+
parsed.CFBundleExecutable,
62+
libraryName,
63+
`Unexpected CFBundleExecutable in ${infoPlistPath}`,
64+
);
65+
assert.equal(
66+
parsed.CFBundleIdentifier,
67+
// Mirrors the default writeFrameworkInfoPlist derives in
68+
// packages/host/src/node/prebuilds/apple.ts, since none of the
69+
// examples pass --apple-bundle-identifier.
70+
escapeBundleIdentifier(`com.callstackincubator.node-api.${libraryName}`),
71+
`Unexpected CFBundleIdentifier in ${infoPlistPath}`,
72+
);
73+
}
74+
4075
async function verifyApplePrebuild(dirent: fs.Dirent) {
4176
console.log("Verifying Apple prebuild", dirent.name, "in", dirent.parentPath);
4277
for (const arch of EXPECTED_XCFRAMEWORK_PLATFORMS) {
@@ -65,8 +100,11 @@ async function verifyApplePrebuild(dirent: fs.Dirent) {
65100
"Expected only directory and files in framework",
66101
);
67102
if (file.name === "Info.plist") {
68-
// TODO: Verify the contents of the Info.plist file
69-
continue;
103+
const libraryName = path.basename(frameworkDir, ".framework");
104+
await verifyFrameworkInfoPlist(
105+
path.join(frameworkDir, file.name),
106+
libraryName,
107+
);
70108
} else {
71109
assert(
72110
!file.name.endsWith(".node"),

pnpm-lock.yaml

Lines changed: 8 additions & 18 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)