Skip to content

Commit 0ca460f

Browse files
j-piaseckifacebook-github-bot
authored andcommitted
Fix a circular import in the prebuilt iOS headers (#58635)
Summary: Each module ships a small header that acts as its entry point. Those were being packaged into the React framework, but the code that includes them lives in the shared headers bundle the framework is built on top of. The two ended up pointing at each other and the iOS build failed. Ship the entry-point headers with the shared headers instead. They still resolve the same way for anyone including them, and no other header moves. Changelog: [Internal] Differential Revision: D121187558
1 parent b71d466 commit 0ca460f

3 files changed

Lines changed: 121 additions & 5 deletions

File tree

‎packages/react-native/scripts/ios-prebuild/__docs__/headers-rules.md‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ scripts under `scripts/ios-prebuild/`:
88
| Script | Role |
99
| ---------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------- |
1010
| `headers-inventory.js` | **Discover + classify** every shipped header (the facts) |
11-
| `headers-spec.js` | **The rules** (R1–R11) — turns the inventory into a layout plan + module maps |
11+
| `headers-spec.js` | **The rules** (R1–R12) — turns the inventory into a layout plan + module maps |
1212
| `headers-compose.js` | **Emit** — projects the plan into `React.xcframework` and `ReactNativeHeaders.xcframework` |
1313
| `headers-verify.js` | **Gate** — generator-time verification: include-health ratchet, structural byte-compare, consumer-shaped compile smokes (runs in the prebuild CI compose job) |
1414

@@ -35,7 +35,7 @@ everything else. No overlay, no include rewriting, no consumer flags.
3535
podspecs ──► headers-inventory.js ──► inventory (facts per header)
3636
│
3737
▼
38-
headers-spec.js (rules R1–R10)
38+
headers-spec.js (rules R1–R12)
3939
│ plan: what goes where + module maps
4040
▼
4141
headers-compose.js (emission)
@@ -233,6 +233,28 @@ emitted as a one-line redirect shim (`#import <owner>`). Shims that are
233233
namespace-module members are fine: they import the owning module, so
234234
declarations stay single-owned.
235235

236+
**R12 — a namespace module's own umbrella stays in ReactNativeHeaders.** The C++
237+
stable API ships one umbrella per module, physically nested inside it
238+
(`ReactCommon/react/debug/React/Debug.h`), so its natural path is
239+
`React/Debug.h` and R1 would hoist it into the framework. It must not be:
240+
**ReactNativeHeaders is the lower layer** — `React.framework` imports it
241+
(`RCTCallInvoker.h` → `<ReactCommon/CallInvoker.h>`). A framework-owned umbrella
242+
makes every `#include <React/X.h>` inside a lowercase-namespace header an import
243+
of module `React`, closing a cycle:
244+
245+
```text
246+
React -> ReactNativeHeaders_react -> React
247+
```
248+
249+
Found empirically: `react/timing/primitives.h` -> `<React/Debug.h>`. This is
250+
the same two-module-ownership failure as `UMBRELLA_CXX_GUARDED_EXCLUSIONS`
251+
(`RCTFrameTimingsObserver.h`, which reaches the same `primitives.h`), in the
252+
opposite direction. They are `objc-blocked` by construction (they re-export
253+
their module's C++ surface), so they were never R4 umbrella or R5 module
254+
members; `planFromInventory` fails closed if one ever becomes a modular
255+
candidate, since its R5 module would be named `React` and alias the framework
256+
module.
257+
236258
## Stage 3 — Emission (headers-compose.js)
237259

238260
`computeSpecPlan(rnRoot)` = inventory → plan, throwing on R8 collisions. Then:

‎packages/react-native/scripts/ios-prebuild/__tests__/headers-spec-test.js‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,55 @@ describe('R11 redirect shims for dual-identity headers', () => {
266266
});
267267
});
268268

269+
describe('R12 namespace module umbrellas stay in ReactNativeHeaders', () => {
270+
test('a module-nested React/ umbrella is not hoisted into the framework', () => {
271+
const m = validManifest();
272+
m.headers.push(
273+
entry(
274+
'React/Debug.h',
275+
'objc-blocked',
276+
'ReactCommon/react/debug/React/Debug.h',
277+
),
278+
);
279+
const plan = planFromInventoryForTest(m);
280+
// Lower layer: including it from a react/-namespace header cannot create a
281+
// React module edge (React -> ReactNativeHeaders_react -> React).
282+
expect(
283+
plan.reactNativeHeaders.find(e => e.naturalPath === 'React/Debug.h')
284+
?.relPath,
285+
).toBe('React/Debug.h');
286+
expect(
287+
plan.react.find(e => e.naturalPath === 'React/Debug.h'),
288+
).toBeUndefined();
289+
expect(plan.umbrella).not.toContain('React/Debug.h');
290+
expect(Object.keys(plan.namespaceModules)).not.toContain('React');
291+
});
292+
293+
test('genuine React.framework headers are still hoisted (R1)', () => {
294+
const m = validManifest();
295+
m.headers.push(
296+
entry('React/RCTMessageThread.h', 'cxx', 'React/Base/RCTMessageThread.h'),
297+
);
298+
const plan = planFromInventoryForTest(m);
299+
expect(
300+
plan.react.find(e => e.naturalPath === 'React/RCTMessageThread.h')
301+
?.relPath,
302+
).toBe('RCTMessageThread.h');
303+
});
304+
305+
test('fails closed if an R12 umbrella becomes a modular candidate', () => {
306+
const m = validManifest();
307+
m.headers.push(
308+
entry(
309+
'React/Debug.h',
310+
'objc-modular-candidate',
311+
'ReactCommon/react/debug/React/Debug.h',
312+
),
313+
);
314+
expect(() => planFromInventoryForTest(m)).toThrow(/R12/);
315+
});
316+
});
317+
269318
describe('DEPS_NAMESPACES (R2 — the deps sidecar namespace set)', () => {
270319
test('includes SocketRocket: one physical home, in the sidecar', () => {
271320
// Pre-sidecar, SocketRocket was excluded from relocation because a REAL

‎packages/react-native/scripts/ios-prebuild/headers-spec.js‎

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,20 @@
7979
* one-line redirect shim (`#import <owner>`). Shims that are namespace-
8080
* module members are fine: they import the owning module, so declarations
8181
* stay single-owned.
82+
* R12. A namespace module's OWN umbrella (ReactCommon/<ns...>/React/<Name>.h,
83+
* natural path React/<Name>.h) ships in ReactNativeHeaders — it is NOT
84+
* hoisted into React.framework by R1. ReactNativeHeaders is the LOWER
85+
* layer (React.framework imports it: RCTCallInvoker.h pulls
86+
* <ReactCommon/CallInvoker.h>), so a framework-owned umbrella turns every
87+
* `#include <React/X.h>` inside a lowercase-namespace header into an import
88+
* of module React and closes a cycle:
89+
* React -> ReactNativeHeaders_react -> React
90+
* Found empirically: react/timing/primitives.h -> <React/Debug.h>. Kept in
91+
* ReactNativeHeaders the same spelling still resolves — framework lookup
92+
* misses and clang falls back to the header search path — textually and
93+
* within one artifact, so no module edge is created. These umbrellas are
94+
* objc-blocked by construction (they re-export their module's C++
95+
* surface), so they were never R4 umbrella or R5 module members anyway.
8296
*/
8397

8498
const fs = require('node:fs');
@@ -280,6 +294,13 @@ function renderNamespaceUmbrella(
280294
return `#ifdef __OBJC__\n#import <UIKit/UIKit.h>\n#endif\n\n${imports}\n`;
281295
}
282296

297+
// R12: a namespace module's own umbrella — physically nested inside the module
298+
// it re-exports (ReactCommon/react/debug/React/Debug.h), which is what tells it
299+
// apart from the ~310 genuine React.framework headers that also carry a
300+
// `React/` natural path but live under React/, Libraries/, ReactApple/, ...
301+
const NS_MODULE_UMBRELLA_RE /*: RegExp */ =
302+
/^ReactCommon\/.+\/React\/[^/]+\.h$/;
303+
283304
/**
284305
* Computes the full layout plan from the header inventory manifest
285306
* (build/header-inventory.json — regenerate with header-inventory.js).
@@ -307,7 +328,13 @@ function planFromInventory(
307328
let bucketKey;
308329
let entryList;
309330
let relPath;
310-
if (np.startsWith('React/')) {
331+
if (np.startsWith('React/') && NS_MODULE_UMBRELLA_RE.test(source)) {
332+
// R12: a namespace module's own umbrella stays in the LOWER layer, so
333+
// including it from that namespace cannot create a React module edge.
334+
relPath = np;
335+
bucketKey = `ReactNativeHeaders/${relPath}`;
336+
entryList = reactNativeHeaders;
337+
} else if (np.startsWith('React/')) {
311338
relPath = np.slice(6); // R1: hoist React/ to the framework Headers root
312339
bucketKey = `React.framework/${relPath}`;
313340
entryList = react;
@@ -332,8 +359,13 @@ function planFromInventory(
332359
seen.set(bucketKey, source);
333360
entryList.push({relPath, source, naturalPath: np});
334361

335-
// R4: React umbrella membership.
336-
if (np.startsWith('React/') && isUmbrellaSafe(h, root)) {
362+
// R4: React umbrella membership. Only headers the framework actually ships
363+
// (R12 umbrellas carry a React/ natural path but live in ReactNativeHeaders).
364+
if (
365+
entryList === react &&
366+
np.startsWith('React/') &&
367+
isUmbrellaSafe(h, root)
368+
) {
337369
umbrella.push(np);
338370
}
339371
// R5: namespace modules (only for ReactNativeHeaders namespaces). Every
@@ -346,6 +378,19 @@ function planFromInventory(
346378
if (entryList === reactNativeHeaders) {
347379
const ns = np.split('/')[0];
348380
if (isUmbrellaSafe(h, root)) {
381+
// R12 assert: an R12-routed umbrella lands under the `React` namespace
382+
// here, and renderNamespaceModuleMap only renames the lowercase `react`
383+
// one — so its module would be named `React` and alias the framework
384+
// module, reintroducing the cycle R12 exists to break. These umbrellas
385+
// are objc-blocked today so they never reach this branch; fail closed
386+
// if that ever changes.
387+
if (ns === 'React') {
388+
throw new Error(
389+
`R12: '${np}' is a modular candidate in ReactNativeHeaders. Its ` +
390+
`namespace module would be named 'React' and alias the React ` +
391+
`framework module. Keep it out of the modular surface.`,
392+
);
393+
}
349394
// R5 exemption assert: a namespace whose name is not a valid module
350395
// identifier cannot get a module, so a modular-candidate header in it
351396
// would be silently non-modular — consumers importing it from a

0 commit comments

Comments
 (0)