Repository navigation
Land next's non-Hermes changes on main - #468
Merged
Merged
Conversation
* ci: verify the ferric Apple binaries depend on weak-node-api Extends the "Test ferric Apple triplets" job so it doesn't only assert which architectures were produced, but also that each produced binary actually links the weak-node-api framework, catching regressions where a triplet builds but drops the dependency. The expected number of `@rpath/weak-node-api.framework/weak-node-api` lines is derived from the otool output itself — `otool -L` prints one header per file, or one per architecture for fat files — rather than hard-coded, so it doesn't rot when a triplet is added or dropped. Also renames lipo-info.txt to lipo-output.txt for symmetry with the new otool-output.txt, and uploads both as artifacts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SrPdjhQ6aG949mVDDaiT2U * ci: accept the versioned weak-node-api install name on macOS The first CI run of this check reported 8 dependencies across 10 binary slices. The two misses were the macOS slices: macOS frameworks use the versioned bundle layout, so weak-node-api's install name there is @rpath/weak-node-api.framework/Versions/0.1.1/weak-node-api where iOS, tvOS and visionOS get the flat @rpath/weak-node-api.framework/weak-node-api Both are a genuine dependency on the framework, so match the optional "Versions/<version>/" component rather than only the flat form. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SrPdjhQ6aG949mVDDaiT2U --------- Co-authored-by: Claude <noreply@anthropic.com> (cherry picked from commit 230d3cd)
* ci: run linting without building native code (#414) The lint job set up a full native toolchain (JDK 17, Android SDK + NDK, x86_64-linux-android Rust target) and ran two native bootstraps purely to get generated TypeScript types for type-checking. Resolve both TODOs: - Add `ferric build --dts-only`, which generates a crate's `.d.ts` and JS entrypoint via a plain host `cargo build` (napi-rs typedef codegen), without cross-compiling any Android/Apple binaries. The library basename is derived from `cargo metadata`'s cdylib target instead of from built artifact paths, so no platform build is needed to compute it. Wire this up as `ferric-example`'s new `build:types` script. - Use `weak-node-api`'s existing `prebuild:prepare` script (header copy + C++/TS declaration codegen) instead of `bootstrap` (which also runs the native CMake build). It already required nothing beyond clang-format. With both native builds no longer needed for typing, the lint job drops the JDK 17, Android SDK, and `rustup target add` steps entirely. Verified locally (Node 24, cargo present, no Android/Apple SDK): fresh `pnpm install && pnpm run build`, then `pnpm --filter weak-node-api run prebuild:prepare`, `pnpm --filter @react-native-node-api/ferric-example run build:types`, `pnpm run lint`, `pnpm run prettier:check`, `pnpm run depcheck` and `pnpm run publint` all pass end-to-end with no native toolchain present, reproducing the new lint job's steps. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DaK9eAAF5G8wj6UT8VekAm * ci: commit ferric-example's declarations as a fixture instead of building them kraenhansen suspected generateTypeScriptDeclarations doesn't actually skip a native build. Confirmed: napi-rs's `napi build` has no typegen-only mode — it always runs a real `cargo build`, and --dts-only leaves a fully populated ~123MB target/ directory (including a compiled libferric_example.so) behind. "Skipping the native build entirely" was wrong; only Android/Apple cross-compilation was actually skipped, and the lint job stayed coupled to the host Rust toolchain's health exactly as #414 wanted to avoid. Switch to the issue's other suggested option: commit ferric_example.d.ts and ferric_example.js as a checked-in fixture (no longer gitignored), and drop the ferric-example build:types step from the lint job entirely — it no longer needs to regenerate anything. --dts-only stays, now documented accurately, as the way to regenerate the fixture by hand after changing packages/ferric-example/src/lib.rs. To catch drift, the two CI jobs that already do a real `ferric build` (Android and Apple triplets) now `git diff --exit-code` the two committed files right after building. Both are label-gated rather than running on every PR, so this doesn't fully close the gap — flagged in the PR thread. Also excludes the two fixture files from Prettier: they're left in napi-rs's own output formatting so regenerating them reproduces the committed bytes exactly, and the new drift check doesn't false-positive on formatting alone. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DaK9eAAF5G8wj6UT8VekAm * ci: drop the explanatory comment from ferric-example/.gitignore Per review feedback on #435. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DaK9eAAF5G8wj6UT8VekAm --------- Co-authored-by: Claude <noreply@anthropic.com> (cherry picked from commit a2e0f75)
`ANDROID_STL` was hardcoded to `c++_shared` when configuring Android builds, with no escape hatch for an addon that needs `c++_static` or must match a prebuilt third-party dependency's STL (#418). The generic `-D`/`--define` cache-variable pass-through (added for #332, which #227 also asks for) already lets a consumer set arbitrary CMake cache variables, including `ANDROID_STL` - but it didn't actually work: our hardcoded Android defaults were appended to the CMake command line *after* the user-provided `-D` arguments, and CMake resolves a variable set multiple times via `-D` to its last occurrence, so the hardcoded value always won. Fix the ordering so the user's `--define` is applied last. `ANDROID_STL` still defaults to `c++_shared`, matching what React Native itself uses. Extract the CMake definitions building into an exported `buildCommonDefinitions` and add unit tests covering the default and the override precedence. Claude-Session: https://claude.ai/code/session_01DaK9eAAF5G8wj6UT8VekAm Co-authored-by: Claude <noreply@anthropic.com> (cherry picked from commit 6e004c7)
Add a --code-signing-allowed flag to the Apple platform of cmake-rn. CODE_SIGNING_ALLOWED=NO remains the default (needed for the free-standing dynamic libraries we produce), but a consumer who needs signed binaries in the XCFramework - enterprise distribution, or a target whose downstream tooling verifies signatures - can now opt in. Addresses the Apple half of #418. Claude-Session: https://claude.ai/code/session_01DaK9eAAF5G8wj6UT8VekAm Co-authored-by: Claude <noreply@anthropic.com> (cherry picked from commit 2a5bd28)
* fix(host): compile out log_debug in release (NDEBUG) builds `log_debug`'s per-addon diagnostic chatter (library found/loaded, symbol resolution, ...) was firing unconditionally on every addon load, including in shipped release builds - unwanted logcat/os_log output plus string-formatting cost on a startup path. log_debug is now an inline no-op declared in Logger.hpp when NDEBUG is defined (set by CMake's Release/MinSizeRel/RelWithDebInfo configurations, and by Xcode's Release configuration by default), mirroring React Native's own dev/release logging split. Logger.cpp's real definition is compiled only outside of NDEBUG. log_warning/log_error are untouched and keep firing in every build type, including RelWithDebInfo. This is compile-time only, not the "compile-time default with runtime override" the issue floats as the ideal: there's no existing runtime config plumbing (env var, JS-settable flag, ...) in this codebase to hook an override into, so adding one would mean inventing new plumbing rather than reusing something established. Shipping the safer compile-time-only guard now per the issue's own fallback. Closes #420 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DaK9eAAF5G8wj6UT8VekAm * docs: keep inline comments brief, trim the Logger ones Adds a `.claude/CLAUDE.md` section: default to no inline comment, and keep the ones that survive to a line or two. Rationale, rejected alternatives and change narration go in the PR description, which is where a `git blame` leads anyway and which does not go stale as the surrounding code moves. Applies it to the log_debug/NDEBUG comments this PR added. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NmbKZsRagnasxGVXtnCLoF --------- Co-authored-by: Claude <noreply@anthropic.com> (cherry picked from commit 641a071)
* Add --namespaced-targets to gyp-to-cmake
* Use namespaced targets in node-addon-examples
* Support building multiple addons
* Limit spawn concurrency
* Emit prebuilds per target, next to their sources
A project declaring multiple addons wrote every prebuild into a single
output directory, named after the CMake target. Both the location and the
name are now derived per target from the CMake File API:
- The output directory defaults to {targetSourceDir}/build/{configuration},
where the new {targetSourceDir} placeholder expands to the target's own
source directory. A single-addon project reports "." and so resolves to
the same path as before.
- The prebuild is named after the artifact on disk (the target's
OUTPUT_NAME) rather than the target name, so a target renamed to avoid a
clash within the project still produces the name the JS require expects.
Together this keeps a prebuild where the Babel plugin and auto-linking
resolve it from, and reduces --namespaced-targets to an internal concern.
Also fixes, in the same area:
- gyp-to-cmake emitted OUTPUT_NAME regardless of --namespaced-targets, due
to an always-truthy condition, and never emitted it for Apple framework
targets, which CMake names after it.
- The Apple build ran a full "cmake --build" once per shared library,
concurrently against one build tree, and called "xcodebuild -list" (a
synchronous spawn) once per library per triplet.
- xcodebuild invocations now run in sequence per build directory, as
concurrent invocations against a single Xcode project are not reliable.
- postBuild looked for "<target name>.framework" while createAppleFramework
names it after the artifact, so the two diverged under namespacing.
- --concurrency accepted any value, and did not implement the documented
fallback to 1 under --verbose. Max listeners is now derived from it.
- verify-prebuilds globbed a directory the prebuilds had moved out of, so
it passed by finding nothing. It now covers tests/ too and requires a
non-zero count.
- The root example project globbed recursively, which both missed examples
copied in after configure and would add a nested project twice. It is
now generated from the same script pipeline.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KfKQDvEkxNtkSE4aaF9yG8
---------
Co-authored-by: Claude <noreply@anthropic.com>
(cherry picked from commit 136fd5f)
The spinners were passed `isEnabled: !silent`. A disabled ora spinner still writes `- <text>` on start and the success/fail symbol on completion (to stderr) — it only skips the animation. `isSilent` is the option that suppresses output entirely. Callers capture stdout only (`$(... --silent)` in CI and the Gradle error message, backticks in patch-hermes.rb), so the stray output was noise rather than a broken path, but `--silent` now does what it says. Porting of a19b9e9 from next: main has a third spinner (copying JSI headers) using the same option. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
kraenhansen
added
Apple 🍎
kraenhansen
marked this pull request as ready for review
October 11, 2026 19:57
kraenhansen
added a commit
that referenced
this pull request
Oct 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lands the commits from
nextthat don't depend on the Hermesstatic_hadoption onmainfirst, so thenextpre-release (#394) is mostly the Hermes change. Please merge with "Rebase and merge" to keep the individual commits.ferric build --dts-only)ANDROID_STLbe overridden via--define--code-signing-allowedlog_debugin release buildsthreadsafe-functionfixture'sCMakeLists.txt, which only exists onnext(Implement hermes_napi_host for async work and thread-safe functions #398)vendor-hermes --silentactually silent #410vendor-hermes --silent— redone by hand, main has a third spinnerStaying on
next: #372, #398, #408, #411, #429, #432, #434, #440–#446, #465.Rebasing
nextonto this afterwards (simulated): the five clean picks drop out, the original #413/#410 replay and restore the parts left out here, and the only conflict ispackages/host/src/node/cli/hermes.tsin #372 — take #372's side. The result matches today'snextapart from main's own release commits..changeset/pre.json'sinitialVersionsshould be bumped to the versions released from main.Test plan
pnpm install,pnpm run build,pnpm run lint🤖 Generated with Claude Code