Skip to content

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

Merged
kraenhansen merged 2 commits into
claude/migrate-pnpm-workspaces-qp0m9tfrom
claude/pnpm-ios-test-failure-5l4lz4
Jul 23, 2026
Merged

fix(node-tests): declare assert dependency, fail bundling on unresolved imports#385
kraenhansen merged 2 commits into
claude/migrate-pnpm-workspaces-qp0m9tfrom
claude/pnpm-ios-test-failure-5l4lz4

Conversation

@kraenhansen

@kraenhansen kraenhansen commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Fixes the single failing iOS test on the pnpm-migration branch:

Node Tests › js-native-api › 2_function_arguments › test › default
ERROR test.titlePath(...).forEach is not a function

The real error (behind the mask)

The 2_function_arguments test from nodejs/node calls require('assert'). Under npm workspaces (main), the assert@2.1.0 ponyfill declared by node-addon-examples was hoisted to the root node_modules, where node-tests' rolldown bundle step resolved it as a phantom dependency and inlined it into test.bundle.js. Under pnpm's strict layout the package is no longer reachable from packages/node-tests, so rolldown emitted only a warning (UNRESOLVED_IMPORT … treating it as an external dependency) and kept a runtime __require("assert") in the bundle. Metro bundles that fine (it's not a statically analyzable require), but it throws at runtime on device when the test body runs — the real error behind the mask.

Verified locally: regenerating the bundle on the base branch reproduces the unresolved-import warning and the runtime __require("assert"); with this fix the bundle inlines assert (2 kB → 153 kB) and the bundled test executes end-to-end against a stubbed addon.

The rest of the pnpm chain checks out: with the built addon in place, both the babel plugin (bundle-time) and the link phase (findNodeApiModulePathsByDependency + getLibraryName from apps/test-app) compute the identical library name node-tests--2_function_arguments under pnpm's symlinked node_modules, so discovery/copying is not affected.

The mask itself (upstream, not fixed here)

test.titlePath(...).forEach is not a function is a secondary crash in mocha-remote-server's failure formatter, reproducible in isolation: flatted.parse with mocha-remote's $$-reviver captures an unresolved reference for $$titlePath whenever the fail-event payload contains shared string references (which real payloads always do — parent.$$fullTitle shares strings with the title path), so titlePath() returns a non-array and mocha's Base reporter throws while printing the failure. Any failing on-device test currently gets masked this way — worth an upstream mocha-remote issue, but out of scope here.

Changes

  • packages/node-tests/package.json: declare assert: ^2.1.0 (dev dependency — only needed at bundle time; rolldown inlines it).
  • packages/node-tests/rolldown.config.mts: fail hard on UNRESOLVED_IMPORT instead of warning, so any future phantom dependency breaks bootstrap loudly instead of failing masked at runtime on device. The intended addon externals are declared via external and are unaffected (verified both directions locally).
  • pnpm-lock.yaml: lockfile update.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MBr6N8caYidCuijvV5ak2A

Generated by Claude Code

@claude
fix(node-tests): declare assert dependency and fail bundling on unres…
99a2a59 
…olved 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
@kraenhansen kraenhansen added the Apple 🍎 Anything related to the Apple platform (iOS, macOS, Cocoapods, Xcode, XCFrameworks, etc.) label Jul 23, 2026 — with Claude
@claude
ci: trigger label-gated jobs
98ffc44 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MBr6N8caYidCuijvV5ak2A
@kraenhansen
kraenhansen merged commit b91761a into claude/migrate-pnpm-workspaces-qp0m9t Jul 23, 2026
9 checks passed
@kraenhansen
kraenhansen deleted the claude/pnpm-ios-test-failure-5l4lz4 branch July 23, 2026 17:39
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

No one assigned

Labels

Apple 🍎 Anything related to the Apple platform (iOS, macOS, Cocoapods, Xcode, XCFrameworks, etc.)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kraenhansen @claude