Skip to content

Commit 934e8fe

Browse files
chrfalchclaude
andcommitted
fix(ios): serve the react/ namespace as a module for SPM consumers
React.framework is a clang module; when an SPM consumer precompiles it, a modular React/ header that #imports <react/...> hit -Wnon-modular-include-in-framework-module because the lowercase react/ namespace (served from ReactNativeHeaders, per R1's Linux/Windows-safe layout) was deliberately kept out of any module. Give react/ a module where it already lives instead of relocating it (relocation would require case-folding react.framework -> React.framework, which only works on case-insensitive filesystems): - headers-spec.js: drop the react/ namespace-module exemption so its objc-modular-candidates get a module; emit that module as ReactNativeHeaders_react (a module literally named 'react' would alias the React framework module on a case-insensitive filesystem). Module names are internal; <react/...> still resolves by header path and is now modular. - headers-inventory.js: classify C++ default member initializers in aggregates (e.g. struct { NSString *family = nil; } in RCTFontProperties.h) as ObjC++ so these are not misclassified objc-modular-candidate and pulled into a plain ObjC module they cannot compile in. The unguarded ObjC/C react/ headers (e.g. JSRuntimeFactoryCAPI.h) now resolve modularly; the C++ react/renderer/* includes are #ifdef __cplusplus-guarded and skipped during the ObjC module emit, so they need no module. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 6fedaed commit 934e8fe

2 files changed

Lines changed: 34 additions & 6 deletions

File tree

packages/react-native/scripts/ios-prebuild/headers-inventory.js

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ function scanHeader(text /*: string */) /*: {
148148
const objcRe =
149149
/^\s*(@(interface|protocol|implementation|class\s|end)|NS_ASSUME_NONNULL_BEGIN)/;
150150
const cxxRe =
151-
/^\s*(namespace\s+[A-Za-z_]|template\s*<|extern\s+"C\+\+"|using\s+(namespace\s|[A-Za-z_]\w*\s*=))/;
151+
/^\s*(namespace\s+[A-Za-z_]|template\s*<|extern\s+"C\+\+"|enum\s+class\b|constexpr\b|using\s+(namespace\s|[A-Za-z_]\w*\s*=))/;
152152

153153
for (const rawLine of text.split('\n')) {
154154
const line = rawLine.replace(/\/\/.*$/, '');
@@ -197,6 +197,22 @@ function scanHeader(text /*: string */) /*: {
197197
}
198198
}
199199
}
200+
201+
// C++ default member initializer inside an aggregate, e.g.
202+
// struct RCTFontProperties { NSString *family = nil; CGFloat size = NAN; };
203+
// Illegal in C/ObjC, so the header is really ObjC++ and cannot compile in a
204+
// plain ObjC module. The keyword scan above misses it (no namespace/template/
205+
// class keyword). Detect a `struct`/`class` body that contains a member
206+
// declaration carrying an `=` initializer. Whole-text (not per-line) so the
207+
// aggregate context is required, avoiding false positives on file-scope
208+
// definitions. Unguarded by construction (definitions can't sit under a
209+
// pure `#ifdef __cplusplus` and still be the ObjC surface).
210+
const aggregateMemberInitRe =
211+
/\b(?:struct|class)\s+[A-Za-z_]\w*[^;{}]*\{[^{}]*?\b[A-Za-z_][\w\s:<>,]*\**\s+\*?[A-Za-z_]\w*\s*=\s*[^;{}]+;/s;
212+
if (aggregateMemberInitRe.test(text)) {
213+
hasUnguardedCxx = true;
214+
}
215+
200216
return {includes, hasObjC, hasUnguardedCxx, hasGuardedCxx};
201217
}
202218

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

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -161,12 +161,16 @@ function planFromInventory(manifest /*: any */) /*: HeadersSpecPlan */ {
161161
if (np.startsWith('React/') && isUmbrellaSafe(h)) {
162162
umbrella.push(np);
163163
}
164-
// R5: namespace modules (only for ReactNativeHeaders namespaces). `react/`
165-
// is exempt — its few modular candidates stay textual so no `react` module
166-
// aliases the `React` framework module.
164+
// R5: namespace modules (only for ReactNativeHeaders namespaces). Every
165+
// namespace with modular candidates gets a module so that React.framework's
166+
// modular headers can `#import <ns/...>` as a MODULAR include (otherwise
167+
// clang's -Wnon-modular-include-in-framework-module rejects it). `react/` is
168+
// included here too — its module is renamed in renderNamespaceModuleMap so a
169+
// `react` module never aliases the `React` framework module on a
170+
// case-insensitive filesystem.
167171
if (entryList === reactNativeHeaders) {
168172
const ns = np.split('/')[0];
169-
if (ns !== 'react' && MODULE_IDENT_RE.test(ns) && isUmbrellaSafe(h)) {
173+
if (MODULE_IDENT_RE.test(ns) && isUmbrellaSafe(h)) {
170174
if (!namespaceModules[ns]) {
171175
namespaceModules[ns] = [];
172176
}
@@ -214,10 +218,18 @@ function renderUmbrellaHeader(umbrella /*: Array<string> */) /*: string */ {
214218
function renderNamespaceModuleMap(
215219
namespaceModules /*: {[string]: Array<string>} */,
216220
) /*: string */ {
221+
// The module NAME is internal to clang's module graph (consumers never
222+
// `@import` these; they `#import <ns/...>` and clang maps the header to its
223+
// module). It only has to be unique and must not alias the `React` framework
224+
// module on a case-insensitive filesystem — so the lowercase `react`
225+
// namespace is given a distinct module name. Header paths are unchanged, so
226+
// `<react/...>` still resolves and is now a modular include.
227+
const moduleNameFor = (ns /*: string */) /*: string */ =>
228+
ns === 'react' ? 'ReactNativeHeaders_react' : ns;
217229
const blocks = [];
218230
for (const ns of Object.keys(namespaceModules).sort()) {
219231
blocks.push(
220-
`module ${ns} {\n` +
232+
`module ${moduleNameFor(ns)} {\n` +
221233
namespaceModules[ns].map(hh => ` header "${hh}"`).join('\n') +
222234
`\n export *\n}`,
223235
);

0 commit comments

Comments
 (0)