Skip to content

fix: verify prebuild Info.plist and prove the Babel plugin's .js/.node precedence (#424) - #449

Closed
kraenhansen wants to merge 2 commits into
nextfrom
claude/sharp-fermi-a5mp24
Closed

fix: verify prebuild Info.plist and prove the Babel plugin's .js/.node precedence (#424)#449
kraenhansen wants to merge 2 commits into
nextfrom
claude/sharp-fermi-a5mp24

Conversation

@kraenhansen

Copy link
Copy Markdown
Collaborator

Closes #424.

verify-prebuilds now reads the Info.plist it finds

verifyApplePrebuild used to continue past Info.plist without reading it. It now parses it with @expo/plist (already a dependency elsewhere in the repo) and asserts, via a small verifyFrameworkInfoPlist helper:

  • CFBundleExecutable matches the framework's library name (the .framework directory's basename)
  • CFBundleIdentifier matches escapeBundleIdentifier("com.callstackincubator.node-api." + libraryName) — the default writeFrameworkInfoPlist (packages/host/src/node/prebuilds/apple.ts) writes when no --apple-bundle-identifier is passed, which is how node-addon-examples builds

escapeBundleIdentifier is now exported from react-native-node-api's public node entrypoint so the verify script (a consumer of the package, like any addon author) can import it rather than re-deriving the escaping rule. The parsed plist is validated with a zod schema rather than accessed as any, matching how the rest of the codebase validates untrusted structured data.

Note on the @expo/plist import: it's a CJS package with an export default. verify-prebuilds.mts is genuine ESM (.mts), so Node's real CJS/ESM interop binds the default import to the whole module.exports, nesting the actual { parse, build } API one .default deeper — confirmed empirically against Node's own module loader, and against tsx (the runner this script actually uses), not just against tsc.

The Babel plugin test now proves its own claim — and that found a real bug

"and does not touch required JS files" asserted the plugin leaves require('./my-addon') alone, but the fixture had no my-addon.*.node for the plugin to have found in the first place — the assertion passed for the wrong reason. Adding a sibling my-addon.apple.node/my-addon.node (as the TODO asked, following the same fixture pattern used by every other test in the file) makes the test exercise real precedence between a .js file and a same-named addon, and it failed: 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 a call that Node's own resolution would never route to the addon.

Fixed isNodeApiModule to defer to a colliding source file when the module path has no explicit .node extension, matching Node's own resolution order (.js/.json before .node). Explicit require('./my-addon.node') calls are untouched by this check, since there's no resolution ambiguity to defer to.

Verification

  • pnpm run build, pnpm run lint, pnpm run prettier:check all clean
  • packages/host's full test suite: only the same 4 permission-based tests fail, and they fail identically on next before this change (this sandbox runs as root, so permission-removal tests can't produce the expected effect) — no new failures
  • Manually exercised the new verifyFrameworkInfoPlist logic (both match and mismatch-detection paths) against a real Info.plist written by createAppleFramework, run both directly with node and through tsx (the actual runner verify-prebuilds.mts uses) — install_name_tool isn't available on Linux, so the check reads the file right after it's written, before that step
  • Added a changeset for react-native-node-api (the isNodeApiModule fix + new export); none needed for node-addon-examples, which is private

cc @kraenhansen for review.

🤖 Generated with Claude Code


Generated by Claude Code

…de 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
@kraenhansen kraenhansen added Apple 🍎 Anything related to the Apple platform (iOS, macOS, Cocoapods, Xcode, XCFrameworks, etc.) Android 🤖 Anything related to the Android platform (Gradle, NDK, Android SDK) Host 🏡 Our `react-native-node-api-modules` package labels Aug 16, 2026 — with Claude
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q1k6UQJPPaqKEKmnsRUatt

Copy link
Copy Markdown
Collaborator Author

Closing this in favor of #450, which builds on @wanxiankai's earlier #426 (the first PR to fix #424) plus the improvements this PR made (moving the fix into isNodeApiModule, zod validation, reusing escapeBundleIdentifier). Sorry for the duplicated effort — #426 wasn't spotted before this PR was opened.


Generated by Claude Code

注册 for free to join this conversation on GitHub. Already have an account? 登录 to comment

标签

Android 🤖 Anything related to the Android platform (Gradle, NDK, Android SDK) Apple 🍎 Anything related to the Apple platform (iOS, macOS, Cocoapods, Xcode, XCFrameworks, etc.) Host 🏡 Our `react-native-node-api-modules` package

项目

None yet

Development

Successfully merging this pull request may close these issues.

Two test gaps: unverified prebuild Info.plist and a missing .node fixture in the Babel plugin tests

2 participants