Skip to content

Commit 93187f3

Browse files
chrfalchclaude
andcommitted
fix(cocoapods): address review on dependency-only facades
Review feedback on the prebuilt-deps facades: - rndeps_facades.rb: soften the "graph-equivalent" comment — facades match the source pod's spec/subspec shape but route every derived subspec's dep to ReactNativeDependencies, so intra-pod subspec deps (RCT-Folly/Fabric -> RCT-Folly/Default) are not reproduced (harmless; the deps are declared explicitly in react_native_pods.rb). - rndeps_facades.rb: add `require_relative './helpers'` so Helpers::Constants is self-contained against require ordering (the defined? guard stays as backstop). - reactNativeDependencies.js: document that the no-version-marker branch returns before the version.txt write, so a locally-staged artifact never gains a marker and later runs re-hit the branch (intentional; downstream must not assume it). - rndependencies.rb: uniq the HEADER_SEARCH_PATHS so a second call can't duplicate. - prebuilt-deps.md: drop the stray space before SocketRocket. Note: the Ruby-tests request is not actioned — the repo's cocoapods Ruby test harness is not wired into CI, so the tests would not run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 34e18af commit 93187f3

4 files changed

Lines changed: 19 additions & 6 deletions

File tree

packages/react-native/scripts/cocoapods/__docs__/prebuilt-deps.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ How the third-party C/C++ deps (`RCT-Folly`, `glog`, `boost`,
1010
In prebuilt-deps mode the `ReactNativeDependencies` POD (CocoaPods) is the
1111
single authority for the third-party deps: compiled code lives in its
1212
xcframework binary, and the artifact's own
13-
`Headers/{folly,glog,boost,fmt,double-conversion,fast_float, SocketRocket}` are
13+
`Headers/{folly,glog,boost,fmt,double-conversion,fast_float,SocketRocket}` are
1414
flattened into the pod's `Headers/` by the podspec's `prepare_command`.
1515
Consumers resolve bare `<folly/...>` / `<SocketRocket/...>` via CocoaPods
1616
public-header linkage from `s.dependency "ReactNativeDependencies"`, plus an

packages/react-native/scripts/cocoapods/rndependencies.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@ def add_rn_third_party_dependencies(s)
4747
header_search_paths << "$(PODS_ROOT)/SocketRocket"
4848
header_search_paths << "$(PODS_ROOT)/RCT-Folly"
4949

50-
current_pod_target_xcconfig["HEADER_SEARCH_PATHS"] = header_search_paths
50+
# uniq so a second call on the same spec can't duplicate entries.
51+
current_pod_target_xcconfig["HEADER_SEARCH_PATHS"] = header_search_paths.uniq
5152
else
5253
# Prebuilt-deps mode: this pod SELF-SERVES the third-party headers from its
5354
# own xcframework (incl. SocketRocket - sole supplier in this mode). See

packages/react-native/scripts/cocoapods/rndeps_facades.rb

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@
55

66
require 'json'
77
require 'fileutils'
8+
# Self-contained against require ordering: this module reads
9+
# Helpers::Constants.socket_rocket_config. react_native_pods.rb normally loads
10+
# helpers.rb first, but requiring it here (idempotent) removes that implicit
11+
# dependency. The defined? guard at the use site stays as a backstop.
12+
require_relative './helpers'
813

914
# Dependency-only facade podspecs for the third-party deps in prebuilt-deps
1015
# mode (deps-side analogue of RNCoreFacades). Design + rationale:
@@ -45,8 +50,12 @@ module RNDepsFacades
4550
# `react_native_path` locates the real third-party podspecs we mirror.
4651
# version + subspecs + default_subspecs are DERIVED from the real spec (or,
4752
# for SocketRocket, synthesized from the socket_rocket_config version) so the
48-
# facade stays graph-equivalent to the source pod. NO source_files and NO
49-
# headers are emitted — the ReactNativeDependencies pod supplies both. A
53+
# facade matches the source pod's spec/subspec SHAPE. It is not fully
54+
# graph-equivalent: every derived subspec depends only on
55+
# ReactNativeDependencies, so intra-pod subspec deps (e.g. RCT-Folly/Fabric
56+
# -> RCT-Folly/Default) are not reproduced — harmless here because the deps
57+
# are all declared explicitly in react_native_pods.rb. NO source_files and
58+
# NO headers are emitted — the ReactNativeDependencies pod supplies both. A
5059
# facaded pod whose real podspec can't be read is a hard error (see
5160
# load_real_spec) — silently shipping an empty facade would hide drift.
5261
def self.generate(react_native_path, install_root, ios_version)

packages/react-native/scripts/ios-prebuild/reactNativeDependencies.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,11 @@ function checkExistingVersion(
162162
`React Native Dependencies found on disk at: ${artifactsPath}.\nNo version file has been found. We are going to use it anyway, but there might be some unexpected behaviors.`,
163163
);
164164
// Honor the message above: an artifact without a version marker is a
165-
// locally-staged one (e.g. a freshly composed deps build) — falling
166-
// through here rmSync'd it and re-downloaded. Use it as-is.
165+
// locally-staged one (e.g. a freshly composed deps build). Use it as-is.
166+
// NOTE: this returns BEFORE the version.txt write below, so a
167+
// locally-staged artifact never gains a marker — every later run re-hits
168+
// this branch. That is intentional (don't clobber a hand-staged build),
169+
// but downstream code must not assume version.txt exists here.
167170
return true;
168171
}
169172
} else {

0 commit comments

Comments
 (0)