Skip to content

Split public header packaging between framework and file set#382

Merged
kraenhansen merged 2 commits into
mainfrom
claude/issue-376-fix-d7f8dm
Jul 19, 2026
Merged

Split public header packaging between framework and file set#382
kraenhansen merged 2 commits into
mainfrom
claude/issue-376-fix-d7f8dm

Conversation

@kraenhansen

@kraenhansen kraenhansen commented Jul 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Context

CMake 4.2 (the current default on GitHub's macos-latest runners) rejects the weak-node-api build with:

CMake Error in CMakeLists.txt:
  The file set "HEADERS", of type "HEADERS", is incompatible with the
  "FRAMEWORK" target "weak-node-api".

packages/weak-node-api/CMakeLists.txt attached the public headers to the target two ways at once — a modern FILE_SET HEADERS and the framework's legacy PUBLIC_HEADER (inside the if(APPLE) block that also sets FRAMEWORK TRUE). CMake 4.2 makes a HEADERS file set incompatible with a FRAMEWORK target, so the Apple build now errors.

As a temporary unblock, #374 / #375 / #377 pinned CMake to 4.1.2 on five macOS jobs. This fixes the root cause and removes those pins.

Changes

packages/weak-node-api/CMakeLists.txt

  • Hoisted the header list into a PUBLIC_HEADER_FILES variable (previously read back from the target's HEADER_SET property).
  • On Apple, ship the public headers via the framework's PUBLIC_HEADER property only — no FILE_SET HEADERS on the framework target. The header list is identical to before, so XCFramework packaging is unchanged.
  • On other platforms, keep the HEADERS file set for install packaging.
  • Added an explicit target_include_directories(... $<BUILD_INTERFACE:...>) so in-tree consumers (the C++ tests, which #include <weak_node_api.hpp>) still get the include paths the file set used to supply implicitly.

.github/workflows/check.yml

  • Reverted the macOS CMake 4.1.2 pin from all five jobs: unit-tests, weak-node-api-tests, test-ios, test-macos, test-ferric-apple-triplets.
  • Kept the CMAKE_VERSION env var — it still selects the Android SDK cmake package in test-android, unrelated to this issue.

Verification

Validated the non-Apple path end-to-end locally: generated the sources, then cmake -S . -B build-tests -DBUILD_TESTS=ON → build → ctest, all passing — including the test's #include <weak_node_api.hpp>, confirming the new target_include_directories propagates includes to consumers.

The Apple .framework / XCFramework build is exercised by this PR's macOS CI (hence the weak-node-api / Apple 🍎 / Ferric 🦀 labels). All four Apple framework-building jobs — Unit tests (macos-latest), Weak Node-API tests (macos-latest), Test app (iOS), and Test ferric Apple triplets — pass on the runner's default CMake, confirming the pin is no longer needed.

Note: test-macos (the MacOS 💻 label) is intentionally not applied. It currently fails to build the React Native macOS test app due to an unrelated fmt consteval error in RN's own C++ (Yoga / logger), which has nothing to do with this change. That's tracked separately.

Acceptance criteria

  • weak-node-api configures and builds on macOS with the runner's default CMake (no version pin) — validated by CI.
  • Reverted the macOS CMake pin from all affected jobs.
  • macOS .framework / XCFramework still ships the correct public headers (identical PUBLIC_HEADER list).

Closes #376

🤖 Generated with Claude Code

@claude
Split public header packaging between framework and file set
a6ab29f 
CMake 4.2 rejects a `HEADERS` file set on a `FRAMEWORK` target, which
broke every macOS job that builds the weak-node-api framework:

    CMake Error in CMakeLists.txt:
      The file set "HEADERS", of type "HEADERS", is incompatible with the
      "FRAMEWORK" target "weak-node-api".

The workaround pinned CMake to 4.1.2 on five macOS jobs (#374, #375,
#377, and the pre-existing test-macos pin). This fixes the root cause
instead.

On Apple, the public headers are shipped via the framework's
PUBLIC_HEADER property; other platforms use a HEADERS file set for
install packaging. The build-interface include directories that the
file set previously supplied to in-tree consumers (e.g. the C++ tests)
are now provided explicitly via target_include_directories, so both
branches build unchanged.

With the root cause fixed, revert the macOS CMake version pin from all
five jobs. CMAKE_VERSION is retained since it still selects the Android
SDK cmake package for test-android.

Closes #376

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Aq4PqMYq8cQTP9Lf43JiwH
@kraenhansen kraenhansen added CI Continuous integration Apple 🍎 Anything related to the Apple platform (iOS, macOS, Cocoapods, Xcode, XCFrameworks, etc.) Ferric 🦀 MacOS 💻 Anything related to the Apple MacOS platform or React Native MacOS support weak-node-api labels Jul 19, 2026 — with Claude
@claude
ci: trigger CI with Apple/macOS/Ferric/weak-node-api labels
6c66d77 
Empty commit to re-run the label-gated macOS jobs now that the labels
are applied, so the reverted CMake pin is validated on the runner's
default CMake.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Aq4PqMYq8cQTP9Lf43JiwH
@kraenhansen kraenhansen removed the MacOS 💻 Anything related to the Apple MacOS platform or React Native MacOS support label Jul 19, 2026 — with Claude
@kraenhansen kraenhansen self-assigned this Jul 19, 2026
@kraenhansen
kraenhansen merged commit 1dee1a0 into main Jul 19, 2026
16 of 17 checks passed
@kraenhansen
kraenhansen deleted the claude/issue-376-fix-d7f8dm branch July 19, 2026 19:16
kraenhansen pushed a commit that referenced this pull request Jul 19, 2026
@claude
ci: adopt main's test-android gating and CMake-pin removal after rebase
61a5437 
Reconcile the pnpm-migrated check.yml with two changes that landed on main
while this branch was in review:

- #382 fixed the CMake 4.2 framework-HEADERS root cause in
  weak-node-api/CMakeLists.txt (included via the rebase) and removed the
  now-redundant "Install compatible CMake version" pin from all five macOS
  jobs. Drop those steps here too; CMAKE_VERSION is retained since
  test-android still uses it to select the Android SDK cmake package.
- #380 gated test-android to labeled PRs only (the ubuntu-self-hosted runner
  is offline and otherwise leaves the job queued forever on main).

With this, check.yml differs from main purely by the pnpm conversion.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnwodAoNbqPec77191HVXn
kraenhansen added a commit that referenced this pull request Jul 23, 2026
@kraenhansen @claude
chore: migrate from npm workspaces to pnpm (#381)
4e5b831 
* chore: migrate from npm workspaces to pnpm

Motivation: pnpm's recursive runner (`pnpm -r run`) is fail-fast by default
and runs in topological (dependency-graph) order, so the root `bootstrap`
and `prerelease` no longer need the non-fail-fast `npm run <s> --workspaces`
pattern that buried the real root cause under cascading failures.

Workspace + package manager:
- Replace the root `workspaces` array with pnpm-workspace.yaml
- Add `packageManager: pnpm@10.33.0` and switch devEngines to pnpm ^10
- Allow only esbuild's build script via onlyBuiltDependencies (pnpm 10 blocks
  dependency lifecycle scripts by default); no shamefully-hoist needed
- Replace package-lock.json with pnpm-lock.yaml; keep node_modules isolated

Internal deps -> workspace:* protocol (cmake-rn, ferric, gyp-to-cmake, host,
node-addon-examples, node-tests, ferric-example, test-app). Add explicit
`weak-node-api` edges to node-addon-examples and node-tests so the topological
bootstrap sequences weak-node-api (which builds the xcframework/.so they link)
before its consumers.

Phantom dependencies surfaced by pnpm's isolated node_modules (npm hoisting
had masked these):
- host: add `@types/babel__core` (used by src/node/babel-plugin/plugin.ts)
- host: add `weak-node-api` as a devDependency (used by
  scripts/generate-injector.mts; previously only a peerDependency)

Scripts:
- bootstrap: `tsc --build && pnpm -r run bootstrap` (fail-fast, topological)
- prerelease/release: make the build explicit instead of relying on npm's
  implicit prerelease hook (pnpm disables pre/post scripts by default)
- test: `pnpm --filter ... run test`
- depcheck/run-in-published: replace `npm query .workspace` with `pnpm ls -r`
- Pin prettier to 3.6.2: 3.7+ is incompatible with @prettier/plugin-oxc@0.0.4
  (regenerating any lockfile floated it to 3.9.5 and crashed the plugin)

CI: port check.yml and release.yml to pnpm (pnpm/action-setup, cache: pnpm,
`pnpm install --frozen-lockfile`, `--filter`, `pnpm exec`). The ephemeral,
non-workspace macOS test app keeps its own `npm install` in
scripts/init-macos-test-app.ts.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnwodAoNbqPec77191HVXn

* ci: fix pnpm github-dep clone on CI and drop redundant --frozen-lockfile

pnpm records GitHub git dependencies (node-addon-examples) with an SSH repo
URL (git@github.com:...). Stock CI runners have no SSH key, so the clone
fails. Add an ad-hoc git config via workflow-level env
(GIT_CONFIG_COUNT/KEY_0/VALUE_0) that rewrites git@github.com: to
https://github.com/, so the public repo is fetched anonymously over HTTPS in
every job without a per-job step.

Also drop the explicit `--frozen-lockfile` from `pnpm install`: pnpm enables
it by default when the CI environment variable is set, so it was redundant.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnwodAoNbqPec77191HVXn

* fix: declare cmake-rn dependency in weak-node-api

weak-node-api's `prebuild:build` script invokes the `cmake-rn` CLI, but the
package never declared cmake-rn. Under npm's hoisting every workspace bin was
linked into the root node_modules/.bin, so `cmake-rn` was always on PATH.
pnpm only links a package's *declared* dependencies' bins, so on CI the
bootstrap failed with `cmake-rn: not found` (a phantom bin dependency that
only surfaces when the native prebuild runs).

Declare `cmake-rn` as a devDependency (workspace:*) so pnpm links its bin into
weak-node-api/node_modules/.bin. This introduces a benign dev-time cycle
(cmake-rn imports weak-node-api's JS for prebuild paths; weak-node-api's build
uses the cmake-rn CLI) which pnpm reports as a warning and handles fine; the
topological bootstrap still sequences weak-node-api before its consumers.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnwodAoNbqPec77191HVXn

* ci: adopt main's test-android gating and CMake-pin removal after rebase

Reconcile the pnpm-migrated check.yml with two changes that landed on main
while this branch was in review:

- #382 fixed the CMake 4.2 framework-HEADERS root cause in
  weak-node-api/CMakeLists.txt (included via the rebase) and removed the
  now-redundant "Install compatible CMake version" pin from all five macOS
  jobs. Drop those steps here too; CMAKE_VERSION is retained since
  test-android still uses it to select the Android SDK cmake package.
- #380 gated test-android to labeled PRs only (the ubuntu-self-hosted runner
  is offline and otherwise leaves the job queued forever on main).

With this, check.yml differs from main purely by the pnpm conversion.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnwodAoNbqPec77191HVXn

* fix: resolve unit-test regressions from lockfile regen and workspace filter

Two unit-test failures surfaced on CI that are artifacts of the migration, not
real behavioural changes:

1. host apple.test.ts (macOS) — `@expo/plist` floated from 0.4.7 (held by
   main's package-lock) to 0.4.9 when the lockfile was regenerated. 0.4.9's
   `parse()` returns a null-prototype object, so the test's strict
   `deepEqual` against a plain object literal fails on the prototype. Pin
   `@expo/plist` to 0.4.7 to match main's resolved version (same class of fix
   as the prettier 3.6.2 pin). The null prototype only affects the test's
   strict equality, not runtime property access.

2. node-addon-examples test (ubuntu/windows) — its `verify-prebuilds` step
   requires all four Android ABIs, but the unit-tests job only builds
   x86_64 (no CMAKE_RN_TRIPLETS). This test never actually ran on main:
   `npm test --workspace node-addon-examples` does not match the package's
   scoped name (@react-native-node-api/node-addon-examples), so npm silently
   skipped it. The faithful pnpm `--filter <scoped-name>` translation ran it
   for the first time and it failed. Drop it from the root `test` filter to
   preserve main's effective behaviour; properly enabling it would require
   building every ABI in the job (out of scope for the package-manager
   migration).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnwodAoNbqPec77191HVXn

* ci: trigger label-gated jobs

Empty commit to start a fresh Check run now that the Apple 🍎 / MacOS 💻 /
Ferric 🦀 / weak-node-api labels are applied, so the label-gated iOS, macOS,
ferric-apple-triplet and weak-node-api jobs actually run against the pnpm
migration. (The workflow only triggers on opened/synchronize/reopened, not
on labeling.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnwodAoNbqPec77191HVXn

* fix: run macOS test-app scaffolding via pnpm dlx (avoid npm devEngines error)

The test-macos job failed at `init-macos-test-app`: it scaffolds the app with
`npx @react-native-community/cli init` run from the workspace root, whose
package.json now declares `devEngines.packageManager: pnpm`. npm 11 refuses to
run (EBADDEVENGINES) because it isn't pnpm.

Switch that single root-level invocation to `pnpm dlx` — pnpm doesn't enforce
devEngines.packageManager (and satisfies it anyway). The remaining steps
(`npm install`, `npx react-native-macos-init`) run inside the scaffolded
standalone app directory, which isn't linked to the root as a workspace, so
they keep using npm/npx unaffected. Keeps the devEngines guardrail intact.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnwodAoNbqPec77191HVXn

* fix(macos-test-app): get the label-gated `test-macos` job green (#384)

* fix(macos-test-app): unblock the macOS test app bundle and native build

Two independent failures kept the label-gated `test-macos` job red.

Metro bundle: the babel plugin rewrites `require("*.node")` in the workspace
packages into `require("react-native-node-api").requireNodeAddon(...)`. Those
files live outside the (intentionally non-workspace) macOS app, so Metro
resolves the bare `react-native-node-api` specifier by walking up from the
package directory. npm's hoisted workspaces happened to place it in the
repo-root node_modules; pnpm's isolated node_modules does not, so the rewritten
require failed with "Unable to resolve module react-native-node-api". Add the
app's own node_modules (where its `file:` deps are installed) to Metro's
`nodeModulesPaths` so resolution no longer depends on the root package
manager's hoisting layout.

Native build: GitHub's macos-latest runner now ships Xcode 26.4 / Apple clang
21, which enforces C++20 `consteval` strictly and rejects fmt 11.0.2's
FMT_STRING() usages ("call to consteval function ... is not a constant
expression") in fmt, Yoga and React-logger. React Native 0.81 bundles fmt
11.0.2 and the upstream fix (fmt 12.1.0) only reached RN >= 0.83.9, so patch the
generated Podfile to define FMT_USE_CONSTEVAL=0 across all pods, falling back to
runtime format-string validation.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011PM3HdJVbivXpzc2MQJV9T

* fix(macos-test-app): patch fmt header directly, add mocha dep, pipefail build

First CI run showed the Metro-bundle fix works (the job reached xcodebuild), but
surfaced two more issues:

- fmt consteval still failed: the GCC_PREPROCESSOR_DEFINITIONS FMT_USE_CONSTEVAL=0
  define did not reach every fmt-consuming translation unit. Patch the vendored
  fmt headers directly instead (flip `#define FMT_USE_CONSTEVAL 1` to 0), the
  approach known to work for RN 0.81 on Xcode 26.4.

- "Run test app" failed with "Cannot find module 'mocha'": mocha-remote-server
  needs mocha at runtime. It resolves via hoisting in the workspace apps, but the
  standalone macOS app must depend on it explicitly, so add mocha to the deps
  transferred from apps/test-app.

Also add `set -o pipefail` to the xcodebuild step so a build failure is not
masked by xcbeautify's exit code (which is what let the previous run limp past a
failed archive into the run step).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011PM3HdJVbivXpzc2MQJV9T

* chore(macos-test-app): trim inline comments to essentials

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011PM3HdJVbivXpzc2MQJV9T

* fix(macos-test-app): bump react-native-macos to 0.81.8, drop fmt patch

react-native-macos 0.81.8 bumps its vendored fmt from 11.0.2 to 12.1.0
(verified: third-party-podspecs/fmt.podspec pins 11.0.2 at v0.81.1 and 12.1.0
at v0.81.8), which resolves the Xcode 26.4 / Apple clang 21 consteval build
failure at its source. Bump REACT_NATIVE_MACOS_VERSION from 0.81.1 to 0.81.8 and
remove the manual Podfile header patch that forced FMT_USE_CONSTEVAL off.

Core react-native stays at 0.81.5 (facebook's 0.81 line has no 0.81.8; the two
packages track independent patch cadences within the same minor).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011PM3HdJVbivXpzc2MQJV9T

* fix(macos-test-app): bump react-native core to 0.81.6 for the macos 0.81.8 peer

react-native-macos-init failed to install react-native-macos@0.81.8 because it
peer-pins react-native 0.81.6 exactly, while REACT_NATIVE_VERSION was still
0.81.5 (the peer for the previous 0.81.1). Bump core to 0.81.6 so the two align.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011PM3HdJVbivXpzc2MQJV9T

* test(macos-test-app): drop repo-root watchFolders to check if still needed

Experiment: with nodeModulesPaths in place, is the watchFolders push still
required for Metro to serve the out-of-tree workspace package sources? Revert
if the bundle step fails.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011PM3HdJVbivXpzc2MQJV9T

* Revert "test(macos-test-app): drop repo-root watchFolders to check if still needed"

This reverts commit 30f8e58.

---------

Co-authored-by: Claude <noreply@anthropic.com>

* fix: declare react-native-node-api in packages shipping .node addons

The react-native-node-api babel plugin rewrites `require("./x.node")` into
`require("react-native-node-api").requireNodeAddon(...)`, so every package that
ships a Node-API addon has an implicit *runtime* dependency on
react-native-node-api once its JS is bundled by Metro. npm hoisted
react-native-node-api to the root node_modules, so Metro resolved it from those
packages; pnpm's isolated node_modules does not, so the Metro bundle failed with
`Unable to resolve module react-native-node-api from
packages/ferric-example/ferric_example.js`.

This is what made the iOS test app hang for ~6h: Metro errored on the first
bundle, but `test:ios:allTests` runs Metro under `mocha-remote -- concurrently`,
which never exits on a bundle error and waits for a client that never connects
until the job hits GitHub's 6h timeout. It affects the Android app the same way.

Declare `react-native-node-api` (workspace:*) in the two addon packages that were
missing it — ferric-example and node-addon-examples (node-tests already declares
it) — so pnpm links it into their node_modules and Metro can resolve the injected
require. Verified locally that it now resolves from each package.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnwodAoNbqPec77191HVXn

* fix(node-tests): declare `assert` dependency, fail bundling on unresolved imports (#385)

* fix(node-tests): declare assert dependency and fail bundling on unresolved imports

The bundled Node.js test for 2_function_arguments requires 'assert', which
was previously satisfied as a phantom dependency: npm workspaces hoisted
node-addon-examples' assert@2.1.0 ponyfill to the root node_modules, where
rolldown resolved and inlined it. Under pnpm's strict node_modules layout the
package is no longer reachable from node-tests, so rolldown silently kept a
runtime __require("assert") call in the bundle (UNRESOLVED_IMPORT is only a
warning), which then fails at runtime on device where Metro cannot resolve it.

The failure surfaced as the masked 'test.titlePath(...).forEach is not a
function' error, a secondary crash in mocha-remote-server's failure formatter.

Declaring assert as a dependency of node-tests lets rolldown inline it again.
Also make the bundle step fail hard on unresolved imports, so any future
phantom dependency breaks bootstrap loudly instead of failing masked on-device.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MBr6N8caYidCuijvV5ak2A

* ci: trigger label-gated jobs

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MBr6N8caYidCuijvV5ak2A

---------

Co-authored-by: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@kraenhansen kraenhansen

Labels

Apple 🍎 Anything related to the Apple platform (iOS, macOS, Cocoapods, Xcode, XCFrameworks, etc.) CI Continuous integration Ferric 🦀 weak-node-api

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Revert CMake pin (#375) and properly fix HEADERS file set on FRAMEWORK target

2 participants

@kraenhansen @claude