Skip to content

Commit 24353da

Browse files
chrfalchreact-native-bot
authored andcommitted
fix(iOS): keep prebuilt Headers/ in place on a Debug/Release swap (#57814)
Summary: Fixes #57803. An iOS Release build can fail in `PrecompileModule React` with seven `include of non-modular header inside framework module` errors — but only when the build follows a Debug/Release configuration switch. `replace-rncore-version.js` deleted and recreated `Pods/React-Core-prebuilt/Headers/` on a swap. That directory holds `module.modulemap`, which `rncore.rb` activates on every target through `-fmodule-map-file`. Nothing orders an unrelated target's dependency scan against this script phase, so a scan can run while the module map is missing. The React module is then precompiled without it, and `<yoga/...>`, `<react/...>` and `<RCTDeprecation/...>` resolve non-modularly. Those headers never needed replacing. The prebuild compose job emits one set of ReactNativeHeaders for both configurations, so they are identical in the Debug and Release tarballs — only the compiled framework differs. This replaces `React.xcframework` and nothing else. ## Changelog: [IOS] [FIXED] - Keep the prebuilt `Headers/` in place on a Debug/Release configuration switch so the React explicit module still resolves its module map Pull Request resolved: #57814 Test Plan: The premise, on the published 0.87.0-rc.3 artifacts (`ios-arm64_x86_64-simulator`): | compared between the Debug and Release tarballs | result | | --- | --- | | `ReactNativeHeaders…/Headers/module.modulemap` | identical | | `React.framework/Modules/module.modulemap` | identical | | `ReactNativeHeaders…/Headers` tree (`diff -rq`) | 0 differences | | `React.framework/Headers` tree (`diff -rq`) | 0 differences | The reproducer from #57803, on Xcode 26.3 with CocoaPods 1.15.2: | build | result | | --- | --- | | 0.87.0-rc.3 | **FAIL** — exit 65, 7 errors | | 0.87.0-rc.3 + this PR | **PASS** — `** BUILD SUCCEEDED **`, 0 errors | The swap still does its job in the passing build — it logs `Replacing React-Core-prebuilt/React.xcframework`, and the installed binary is the Release one: ``` installed: 55225ccbc283c57c614ff4caf263cb63bad3828240e62cee8893e7001774bd6c rc3 release: 55225ccbc283c57c614ff4caf263cb63bad3828240e62cee8893e7001774bd6c rc3 debug: 516215801a6f8a86640aae13c2f2de1bbdb95189edf124e208f528b1497c7e4c ``` A Release→Debug swap was verified the same way. Across a swap, `Headers/module.modulemap` keeps its inode while `React.xcframework` gets a new one. ## Unit tests Adds a unit test for the script, 4 cases: correct framework installed, `Headers/module.modulemap` untouched, an Expo-generated `React-use-frameworks.modulemap` left in place, and a fail-closed case on a tarball with no `React.xcframework`. The script needed a `require.main === module` guard and one export to be importable. ``` js1 test xplat/js/react-native-github/packages/react-native/scripts/__tests__/replace-rncore-version-test.js → 4 passed, 4 total ``` The module-map case is a real regression test, not just a pin. Restoring the pre-fix delete-and-recreate makes it fail on the inode assertion while the other three keep passing: ``` ✕ leaves Headers/module.modulemap untouched Expected: 735095485 Received: 735095515 ``` That only works because the fixture tarball also ships `ReactNativeHeaders.xcframework`. Without it the pre-fix code throws its fail-closed error before reaching the assertion, so the test would go red for the wrong reason and would not actually be guarding #57803. The Expo case covers behaviour this diff removes the explicit protection for. The old save-and-restore of `React-use-frameworks.modulemap` (ada3922) is unnecessary now that only `React.xcframework` is replaced, but nothing else pins it. ## Argument parsing `yargs` parsing moved inside the `require.main === module` guard, so importing the module no longer parses `process.argv`. Verified in both directions. The command line still performs the swap end to end: ``` $ node replace-rncore-version.js -c Release -r <version> -p <podsRoot> Replacing React-Core-prebuilt/React.xcframework Updating React-Core-prebuilt/.last_build_configuration with Release Done replacing React Native prebuilt installed binary: binary-Release module.modulemap inode before=735141703 after=735141703 last_build marker: Release ``` Importing with hostile argv (`-c` collides with jest's `--config`) has no side effects: ``` $ node -e "process.argv = ['node','jest','-c','jest.config.js','--version']; require('./replace-rncore-version.js')" imported OK, exports: replaceRNCoreConfiguration ``` `arc lint` is clean on both files. Reviewed By: zeyap Differential Revision: D114735639 Pulled By: fabriziocucci fbshipit-source-id: 35ead7dae9ce7ad7160005a15ecb3975817ae728
1 parent 329f864 commit 24353da

2 files changed

Lines changed: 192 additions & 127 deletions

File tree

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @format
8+
* @noflow
9+
*/
10+
11+
'use strict';
12+
13+
const {replaceRNCoreConfiguration} = require('../replace-rncore-version');
14+
const {execFileSync} = require('node:child_process');
15+
const fs = require('node:fs');
16+
const os = require('node:os');
17+
const path = require('node:path');
18+
19+
const VERSION = '0.87.0-test';
20+
const SLICE = 'ios-arm64_x86_64-simulator';
21+
const BINARY = path.join(SLICE, 'React.framework', 'React');
22+
23+
function writeFile(filePath, contents) {
24+
fs.mkdirSync(path.dirname(filePath), {recursive: true});
25+
fs.writeFileSync(filePath, contents);
26+
}
27+
28+
function buildTarball(podsRoot, configuration) {
29+
const stage = fs.mkdtempSync(path.join(podsRoot, `stage-${configuration}-`));
30+
writeFile(path.join(stage, 'React.xcframework', 'Info.plist'), '<plist/>');
31+
writeFile(
32+
path.join(stage, 'React.xcframework', BINARY),
33+
`binary-${configuration}`,
34+
);
35+
// The tarball must also ship ReactNativeHeaders: without it the pre-fix code
36+
// throws its fail-closed error before reaching the module map, so the
37+
// regression test below would pass for the wrong reason.
38+
writeFile(
39+
path.join(
40+
stage,
41+
'ReactNativeHeaders.xcframework',
42+
SLICE,
43+
'Headers',
44+
'module.modulemap',
45+
),
46+
'module yoga {}\n',
47+
);
48+
const artifacts = path.join(podsRoot, 'ReactNativeCore-artifacts');
49+
fs.mkdirSync(artifacts, {recursive: true});
50+
execFileSync('tar', [
51+
'-czf',
52+
path.join(
53+
artifacts,
54+
`reactnative-core-${VERSION.toLowerCase()}-${configuration.toLowerCase()}.tar.gz`,
55+
),
56+
'-C',
57+
stage,
58+
'.',
59+
]);
60+
fs.rmSync(stage, {recursive: true, force: true});
61+
}
62+
63+
describe('replaceRNCoreConfiguration', () => {
64+
let podsRoot;
65+
let pod;
66+
let cwd;
67+
68+
beforeEach(() => {
69+
podsRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'rncore-test-'));
70+
pod = path.join(podsRoot, 'React-Core-prebuilt');
71+
// What the podspec prepare_command leaves behind after `pod install`.
72+
writeFile(
73+
path.join(pod, 'Headers', 'module.modulemap'),
74+
'module yoga {}\n',
75+
);
76+
writeFile(path.join(pod, 'React.xcframework', 'Info.plist'), '<plist/>');
77+
writeFile(path.join(pod, 'React.xcframework', BINARY), 'binary-Debug');
78+
buildTarball(podsRoot, 'Release');
79+
cwd = process.cwd();
80+
// The script phase runs with Pods/ as its working directory.
81+
process.chdir(podsRoot);
82+
});
83+
84+
afterEach(() => {
85+
process.chdir(cwd);
86+
fs.rmSync(podsRoot, {recursive: true, force: true});
87+
});
88+
89+
it('installs the framework for the requested configuration', () => {
90+
replaceRNCoreConfiguration('Release', VERSION, podsRoot);
91+
92+
expect(
93+
fs.readFileSync(path.join(pod, 'React.xcframework', BINARY), 'utf8'),
94+
).toBe('binary-Release');
95+
});
96+
97+
// Regression test for #57803: recreating the module map mid-build lets a
98+
// concurrent dependency scan miss it, and the React module then precompiles
99+
// without -fmodule-map-file and fails on non-modular includes.
100+
it('leaves Headers/module.modulemap untouched', () => {
101+
const moduleMap = path.join(pod, 'Headers', 'module.modulemap');
102+
const before = fs.statSync(moduleMap).ino;
103+
const contentsBefore = fs.readFileSync(moduleMap, 'utf8');
104+
105+
replaceRNCoreConfiguration('Release', VERSION, podsRoot);
106+
107+
expect(fs.statSync(moduleMap).ino).toBe(before);
108+
expect(fs.readFileSync(moduleMap, 'utf8')).toBe(contentsBefore);
109+
});
110+
111+
// The swap used to purge every directory and restore this file by hand. It
112+
// now replaces React.xcframework alone, so the file is never disturbed.
113+
it('leaves an Expo-generated React-use-frameworks.modulemap in place', () => {
114+
const expoModuleMap = path.join(pod, 'React-use-frameworks.modulemap');
115+
writeFile(expoModuleMap, 'module React {}\n');
116+
117+
replaceRNCoreConfiguration('Release', VERSION, podsRoot);
118+
119+
expect(fs.readFileSync(expoModuleMap, 'utf8')).toBe('module React {}\n');
120+
});
121+
122+
it('fails when the tarball has no React.xcframework', () => {
123+
const stage = fs.mkdtempSync(path.join(podsRoot, 'stage-bad-'));
124+
writeFile(path.join(stage, 'unrelated.txt'), 'nope');
125+
execFileSync('tar', [
126+
'-czf',
127+
path.join(
128+
podsRoot,
129+
'ReactNativeCore-artifacts',
130+
`reactnative-core-${VERSION.toLowerCase()}-release.tar.gz`,
131+
),
132+
'-C',
133+
stage,
134+
'.',
135+
]);
136+
137+
expect(() =>
138+
replaceRNCoreConfiguration('Release', VERSION, podsRoot),
139+
).toThrow(/Extraction verification failed/);
140+
});
141+
});

packages/react-native/scripts/replace-rncore-version.js

Lines changed: 51 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ function replaceRNCoreConfiguration(
5959
configuration /*: string */,
6060
version /*: string */,
6161
podsRoot /*: string */,
62-
) {
62+
) /*: void */ {
6363
// Filename comes from rncore.rb
6464
const tarballURLPath = `${podsRoot}/ReactNativeCore-artifacts/reactnative-core-${version.toLowerCase()}-${configuration.toLowerCase()}.tar.gz`;
6565

@@ -73,18 +73,6 @@ function replaceRNCoreConfiguration(
7373
const tmpExtractDir = path.join(tmpDir, 'React-Core-prebuilt');
7474
fs.mkdirSync(tmpExtractDir, {recursive: true});
7575

76-
// Preserve Expo-generated modulemap before replacing directories
77-
const useFrameworksModulemapName = 'React-use-frameworks.modulemap';
78-
const useFrameworksModulemapPath = path.join(
79-
finalLocation,
80-
useFrameworksModulemapName,
81-
);
82-
let savedModulemap = null;
83-
if (fs.existsSync(useFrameworksModulemapPath)) {
84-
console.log('Preserving', useFrameworksModulemapName);
85-
savedModulemap = fs.readFileSync(useFrameworksModulemapPath);
86-
}
87-
8876
try {
8977
console.log('Extracting the tarball to temp dir', tarballURLPath);
9078
const result = spawnSync(
@@ -110,98 +98,30 @@ function replaceRNCoreConfiguration(
11098
);
11199
}
112100

113-
// Delete only directories in finalLocation (e.g. the React.xcframework) -
114-
// not files, so any sibling files written during pod install are preserved.
115-
const dirs = fs
116-
.readdirSync(finalLocation, {withFileTypes: true})
117-
.filter(dirent => dirent.isDirectory());
118-
for (const dirent of dirs) {
119-
const direntName =
120-
typeof dirent.name === 'string' ? dirent.name : dirent.name.toString();
121-
const dirPath = `${finalLocation}/${direntName}`;
122-
console.log('Removing directory', dirPath);
123-
fs.rmSync(dirPath, {force: true, recursive: true});
124-
}
125-
126-
// Move extracted directories from temp to final location
127-
const extractedEntries = fs
128-
.readdirSync(tmpExtractDir, {withFileTypes: true})
129-
.filter(dirent => dirent.isDirectory());
130-
for (const dirent of extractedEntries) {
131-
const direntName =
132-
typeof dirent.name === 'string' ? dirent.name : dirent.name.toString();
133-
const src = path.join(tmpExtractDir, direntName);
134-
const dst = path.join(finalLocation, direntName);
135-
const mvResult = spawnSync('mv', [src, dst], {stdio: 'inherit'});
136-
if (mvResult.status !== 0) {
137-
// Fallback: copy recursively then remove source
138-
console.log(`mv failed for ${direntName}, falling back to cp -R`);
139-
const cpResult = spawnSync('cp', ['-R', src, dst], {
140-
stdio: 'inherit',
141-
});
142-
if (cpResult.status !== 0) {
143-
throw new Error(
144-
`cp fallback failed with exit code ${cpResult.status}`,
145-
);
146-
}
101+
// Replace only the compiled framework. Headers/ is flattened from
102+
// ReactNativeHeaders by the podspec prepare_command, and the prebuild
103+
// compose job emits one set of those headers for both configurations, so a
104+
// config switch leaves them identical. Leaving them alone keeps
105+
// Headers/module.modulemap — which consumers activate through
106+
// -fmodule-map-file — in place for the whole build; deleting and recreating
107+
// it mid-build lets a concurrent dependency scan miss it, and the React
108+
// module then precompiles without it (#57803).
109+
const dest = path.join(finalLocation, 'React.xcframework');
110+
console.log('Replacing', dest);
111+
fs.rmSync(dest, {force: true, recursive: true});
112+
const mvResult = spawnSync('mv', [xcfwPath, dest], {stdio: 'inherit'});
113+
if (mvResult.status !== 0) {
114+
// Fallback: copy recursively then remove source
115+
console.log('mv failed for React.xcframework, falling back to cp -R');
116+
const cpResult = spawnSync('cp', ['-R', xcfwPath, dest], {
117+
stdio: 'inherit',
118+
});
119+
if (cpResult.status !== 0) {
120+
throw new Error(`cp fallback failed with exit code ${cpResult.status}`);
147121
}
148122
}
149-
150-
// The podspec prepare_command flattens ReactNativeHeaders' headers into a
151-
// top-level Headers/ dir, but it does not re-run on a config swap. Mirror
152-
// it here: re-flatten the headers (identical across slices) and drop the
153-
// now-redundant xcframework so $(PODS_ROOT)/React-Core-prebuilt/Headers
154-
// keeps resolving <react/...>, <yoga/...>, etc.
155-
//
156-
// Fail closed when the swapped-in tarball lacks ReactNativeHeaders: the
157-
// directory purge above already deleted the previous Headers/, so
158-
// continuing silently would leave the injected -fmodule-map-file flag
159-
// dangling and break every <react/...> include only on a config switch —
160-
// with no pointer to the version-skewed artifact that caused it.
161-
const rnhXcfw = path.join(finalLocation, 'ReactNativeHeaders.xcframework');
162-
if (!fs.existsSync(rnhXcfw)) {
163-
throw new Error(
164-
`ReactNativeHeaders.xcframework not found in the extracted tarball at ${finalLocation}. ` +
165-
'The downloaded artifact predates the headers-spec layout (or is incomplete); ' +
166-
'use a prebuilt tarball matching this react-native version.',
167-
);
168-
}
169-
const slice = fs
170-
.readdirSync(rnhXcfw, {withFileTypes: true})
171-
.find(
172-
dirent =>
173-
dirent.isDirectory() &&
174-
fs.existsSync(path.join(rnhXcfw, dirent.name.toString(), 'Headers')),
175-
);
176-
if (!slice) {
177-
throw new Error(
178-
`No slice with a Headers directory found inside ${rnhXcfw}.`,
179-
);
180-
}
181-
const headersDest = path.join(finalLocation, 'Headers');
182-
fs.rmSync(headersDest, {force: true, recursive: true});
183-
const cpHeaders = spawnSync(
184-
'cp',
185-
['-R', path.join(rnhXcfw, slice.name.toString(), 'Headers'), headersDest],
186-
{stdio: 'inherit'},
187-
);
188-
if (cpHeaders.status !== 0) {
189-
throw new Error(
190-
`Flattening ReactNativeHeaders failed with exit code ${cpHeaders.status}`,
191-
);
192-
}
193-
fs.rmSync(rnhXcfw, {force: true, recursive: true});
194123
} finally {
195-
// Clean up temp directory
196124
fs.rmSync(tmpDir, {force: true, recursive: true});
197-
198-
// Restore Expo-generated modulemap after directory replacement.
199-
// Runs in finally so it is not skipped if mv/cp partially fails.
200-
if (savedModulemap != null) {
201-
const restoredPath = path.join(finalLocation, useFrameworksModulemapName);
202-
fs.writeFileSync(restoredPath, savedModulemap);
203-
console.log('Restored', useFrameworksModulemapName);
204-
}
205125
}
206126
}
207127

@@ -227,29 +147,33 @@ function main(
227147
console.log('Done replacing React Native prebuilt');
228148
}
229149

230-
// This script is executed in the Pods folder, which is usually not synched to Github, so it should be ok
231-
const argv = yargs
232-
.option('c', {
233-
alias: 'configuration',
234-
description:
235-
'Configuration to use to download the right React-Core prebuilt version. Allowed values are "Debug" and "Release".',
236-
})
237-
.option('r', {
238-
alias: 'reactNativeVersion',
239-
description:
240-
'The Version of React Native associated with the React-Core prebuilt tarball.',
241-
})
242-
.option('p', {
243-
alias: 'podsRoot',
244-
description: 'The path to the Pods root folder',
245-
})
246-
.usage('Usage: $0 -c Debug -r <version> -p <path/to/react-native>').argv;
247-
248-
// $FlowFixMe[prop-missing]
249-
const configuration = argv.configuration;
250-
// $FlowFixMe[prop-missing]
251-
const version = argv.reactNativeVersion;
252-
// $FlowFixMe[prop-missing]
253-
const podsRoot = argv.podsRoot;
254-
255-
main(configuration, version, podsRoot);
150+
if (require.main === module) {
151+
// This script is executed in the Pods folder, which is usually not synched to Github, so it should be ok
152+
const argv = yargs
153+
.option('c', {
154+
alias: 'configuration',
155+
description:
156+
'Configuration to use to download the right React-Core prebuilt version. Allowed values are "Debug" and "Release".',
157+
})
158+
.option('r', {
159+
alias: 'reactNativeVersion',
160+
description:
161+
'The Version of React Native associated with the React-Core prebuilt tarball.',
162+
})
163+
.option('p', {
164+
alias: 'podsRoot',
165+
description: 'The path to the Pods root folder',
166+
})
167+
.usage('Usage: $0 -c Debug -r <version> -p <path/to/react-native>').argv;
168+
169+
// $FlowFixMe[prop-missing]
170+
const configuration = argv.configuration;
171+
// $FlowFixMe[prop-missing]
172+
const version = argv.reactNativeVersion;
173+
// $FlowFixMe[prop-missing]
174+
const podsRoot = argv.podsRoot;
175+
176+
main(configuration, version, podsRoot);
177+
}
178+
179+
module.exports = {replaceRNCoreConfiguration};

0 commit comments

Comments
 (0)