Skip to content

Commit d96a94a

Browse files
Saadnajmiclaude
andcommitted
refactor: address PR review comments (diff tags, code cleanup)
- Add missing [macOS] diff tags across Package.swift, hermes.js, and shell build scripts per the diff tag guide - Move macosx case to bottom of get_deployment_target elif chain to minimize upstream diff in build-ios-framework.sh - Remove hermes.js re-exports; import directly from macosVersionResolver - Extract {stdio: 'inherit'} to const in buildFromHermesCommit - Add HERMES_PATH override comment in build-apple-framework.sh - Increase CI artifact retention-days from 1 to 30 - Add commented-out lines for removed upstream excludes in Package.swift Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 21f812c commit d96a94a

5 files changed

Lines changed: 33 additions & 36 deletions

File tree

.github/workflows/microsoft-build-spm.yml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ jobs:
3434
id: resolve
3535
working-directory: packages/react-native
3636
run: |
37-
COMMIT=$(node -e "const {hermesCommitAtMergeBase} = require('./scripts/ios-prebuild/hermes'); console.log(hermesCommitAtMergeBase().commit);" 2>&1 | grep -E '^[0-9a-f]{40}$')
37+
COMMIT=$(node -e "const {hermesCommitAtMergeBase} = require('./scripts/ios-prebuild/macosVersionResolver'); console.log(hermesCommitAtMergeBase().commit);" 2>&1 | grep -E '^[0-9a-f]{40}$')
3838
echo "hermes-commit=$COMMIT" >> "$GITHUB_OUTPUT"
3939
echo "Resolved Hermes commit: $COMMIT"
4040
@@ -51,7 +51,7 @@ jobs:
5151
with:
5252
name: hermes-artifacts
5353
path: hermes-destroot
54-
retention-days: 1
54+
retention-days: 30
5555

5656
build-hermesc:
5757
name: "Build hermesc"
@@ -89,7 +89,7 @@ jobs:
8989
with:
9090
name: hermesc
9191
path: hermes/build_host_hermesc
92-
retention-days: 1
92+
retention-days: 30
9393

9494
build-hermes-slice:
9595
name: "Hermes ${{ matrix.slice }}"
@@ -151,7 +151,7 @@ jobs:
151151
with:
152152
name: hermes-slice-${{ matrix.slice }}
153153
path: hermes/destroot
154-
retention-days: 1
154+
retention-days: 30
155155

156156
assemble-hermes:
157157
name: "Assemble Hermes xcframework"
@@ -207,7 +207,7 @@ jobs:
207207
with:
208208
name: hermes-artifacts
209209
path: hermes/destroot
210-
retention-days: 1
210+
retention-days: 30
211211

212212
build-spm:
213213
name: "SPM ${{ matrix.platform }}"

packages/react-native/Package.swift

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ let reactJsErrorHandler = RNTarget(
246246
let reactGraphicsApple = RNTarget(
247247
name: .reactGraphicsApple,
248248
path: "ReactCommon/react/renderer/graphics/platform/ios",
249-
linkedFrameworks: ["CoreGraphics"],
249+
linkedFrameworks: ["CoreGraphics"], // [macOS] UIKit removed; linked conditionally via platformLinkerSettings below
250250
// [macOS: UIKit on iOS/visionOS, AppKit on macOS
251251
// Note: #if os(macOS) doesn't work here because Package.swift runs on the host,
252252
// not the target. Use .when(platforms:) for cross-compilation support.
@@ -368,8 +368,8 @@ let reactCore = RNTarget(
368368
"ReactCommon/react/runtime/platform/ios", // explicit header search path to break circular dependency. RCTHost imports `RCTDefines.h` in ReactCore, ReacCore needs to import RCTHost
369369
],
370370
linkedFrameworks: ["CoreServices"],
371-
excludedPaths: ["Fabric", "Tests", "Resources", "Runtime/RCTJscInstanceFactory.mm", "I18n/strings", "CxxBridge/JSCExecutorFactory.mm", "CoreModules", "RCTUIKit"],
372-
dependencies: [.reactNativeDependencies, .reactCxxReact, .reactPerfLogger, .jsi, .reactJsiExecutor, .reactUtils, .reactFeatureFlags, .reactRuntimeScheduler, .yoga, .reactJsInspector, .reactJsiTooling, .rctDeprecation, .reactCoreRCTWebsocket, .reactRCTImage, .reactTurboModuleCore, .reactRCTText, .reactRCTBlob, .reactRCTAnimation, .reactRCTNetwork, .reactFabric, .hermesPrebuilt, .reactRCTUIKit],
371+
excludedPaths: ["Fabric", "Tests", "Resources", "Runtime/RCTJscInstanceFactory.mm", "I18n/strings", "CxxBridge/JSCExecutorFactory.mm", "CoreModules", "RCTUIKit"], // [macOS] added RCTUIKit exclusion (separate target)
372+
dependencies: [.reactNativeDependencies, .reactCxxReact, .reactPerfLogger, .jsi, .reactJsiExecutor, .reactUtils, .reactFeatureFlags, .reactRuntimeScheduler, .yoga, .reactJsInspector, .reactJsiTooling, .rctDeprecation, .reactCoreRCTWebsocket, .reactRCTImage, .reactTurboModuleCore, .reactRCTText, .reactRCTBlob, .reactRCTAnimation, .reactRCTNetwork, .reactFabric, .hermesPrebuilt, .reactRCTUIKit], // [macOS] added .reactRCTUIKit
373373
sources: [".", "Runtime/RCTHermesInstanceFactory.mm"]
374374
)
375375

@@ -393,8 +393,7 @@ let reactFabric = RNTarget(
393393
"components/view/tests",
394394
"components/view/platform/android",
395395
"components/view/platform/windows",
396-
// "components/view/platform/cxx", // [macOS] excluded on macOS, included on iOS/visionOS (see reactFabricViewPlatformExcludes)
397-
// "components/view/platform/macos", // [macOS] excluded on iOS/visionOS, included on macOS (see reactFabricViewPlatformExcludes)
396+
// "components/view/platform/macos", // [macOS] moved to reactFabricViewPlatformExcludes for conditional exclusion
398397
"components/scrollview/tests",
399398
"components/scrollview/platform/android",
400399
"mounting/tests",
@@ -440,12 +439,14 @@ let reactFabricComponents = RNTarget(
440439
"components/view/platform/windows",
441440
// "components/view/platform/macos", // [macOS] not needed here — sources don't include components/view
442441
"components/textinput/platform/android",
442+
// "components/textinput/platform/macos", // [macOS] removed — directory does not exist
443443
"components/text/platform/android",
444444
"components/text/tests",
445445
"textlayoutmanager/tests",
446446
"textlayoutmanager/platform/android",
447447
"textlayoutmanager/platform/cxx",
448448
"textlayoutmanager/platform/windows",
449+
// "textlayoutmanager/platform/macos", // [macOS] removed — directory does not exist
449450
"conponents/rncore", // this was the old folder where RN Core Components were generated. If you ran codegen in the past, you might have some files in it that might make the build fail.
450451
],
451452
dependencies: [.reactNativeDependencies, .reactCore, .reactJsiExecutor, .reactTurboModuleCore, .jsi, .logger, .reactDebug, .reactFeatureFlags, .reactUtils, .reactRuntimeScheduler, .reactCxxReact, .yoga, .reactRendererDebug, .reactGraphics, .reactFabric, .reactTurboModuleBridging],

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

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ async function prepareHermesArtifactsAsync(
105105
const sourceType = await hermesSourceType(
106106
resolvedVersion,
107107
buildType,
108-
allowBuildFromSource,
108+
allowBuildFromSource, // [macOS]
109109
);
110110
localPath = await resolveSourceFromSourceType(
111111
sourceType,
@@ -156,7 +156,7 @@ type HermesEngineSourceType =
156156
| 'local_prebuilt_tarball'
157157
| 'download_prebuild_tarball'
158158
| 'download_prebuilt_nightly_tarball'
159-
| 'build_from_hermes_commit'
159+
| 'build_from_hermes_commit' // [macOS]
160160
*/
161161

162162
const HermesEngineSourceTypes = {
@@ -255,16 +255,11 @@ async function hermesArtifactExists(
255255

256256
/**
257257
* Determines the source type for Hermes based on availability
258-
*
259-
* @param version - The resolved version string
260-
* @param buildType - Debug or Release
261-
* @param allowBuildFromSource - If true (macOS main branch), fall back to BUILD_FROM_HERMES_COMMIT
262-
* when no prebuilt artifacts exist. If false, fall back to nightly download (original behavior).
263258
*/
264259
async function hermesSourceType(
265260
version /*: string */,
266261
buildType /*: BuildFlavor */,
267-
allowBuildFromSource /*: boolean */ = false,
262+
allowBuildFromSource /*: boolean */ = false, // [macOS]
268263
) /*: Promise<HermesEngineSourceType> */ {
269264
if (hermesEngineTarballEnvvarDefined()) {
270265
hermesLog('Using local prebuild tarball');
@@ -440,20 +435,22 @@ async function buildFromHermesCommit(
440435
const HERMES_GITHUB_URL = 'https://github.com/facebook/hermes.git';
441436
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-build-'));
442437
const hermesDir = path.join(tmpDir, 'hermes');
438+
const inheritStdio = {stdio: 'inherit'};
443439

444440
try {
445441
// Clone Hermes at the identified commit using the most efficient
446442
// single-fetch pattern (see https://github.com/actions/checkout)
447443
hermesLog(`Cloning Hermes at commit ${commit}...`);
448-
execSync(`git init "${hermesDir}"`, {stdio: 'inherit'});
449-
execSync(`git -C "${hermesDir}" remote add origin ${HERMES_GITHUB_URL}`, {
450-
stdio: 'inherit',
451-
});
444+
execSync(`git init "${hermesDir}"`, inheritStdio);
445+
execSync(
446+
`git -C "${hermesDir}" remote add origin ${HERMES_GITHUB_URL}`,
447+
inheritStdio,
448+
);
452449
execSync(
453450
`git -C "${hermesDir}" fetch --no-tags --depth 1 origin +${commit}:refs/remotes/origin/main`,
454-
{stdio: 'inherit', timeout: 300000},
451+
{...inheritStdio, timeout: 300000},
455452
);
456-
execSync(`git -C "${hermesDir}" checkout main`, {stdio: 'inherit'});
453+
execSync(`git -C "${hermesDir}" checkout main`, inheritStdio);
457454

458455
const reactNativeRoot = path.resolve(__dirname, '..', '..');
459456
const buildScript = path.join(
@@ -479,8 +476,8 @@ async function buildFromHermesCommit(
479476

480477
hermesLog(`Building Hermes frameworks (${buildType})...`);
481478
execSync(`bash "${buildScript}"`, {
479+
...inheritStdio,
482480
cwd: hermesDir,
483-
stdio: 'inherit',
484481
timeout: 3600000, // 60 minutes
485482
env: buildEnv,
486483
});
@@ -489,9 +486,7 @@ async function buildFromHermesCommit(
489486
const tarballName = `hermes-ios-${buildType.toLowerCase()}.tar.gz`;
490487
const tarballPath = path.join(artifactsPath, tarballName);
491488
hermesLog('Creating Hermes tarball from build output...');
492-
execSync(`tar -czf "${tarballPath}" -C "${hermesDir}" destroot`, {
493-
stdio: 'inherit',
494-
});
489+
execSync(`tar -czf "${tarballPath}" -C "${hermesDir}" destroot`, inheritStdio);
495490

496491
hermesLog(`Hermes built from source and packaged at ${tarballPath}`);
497492
return tarballPath;
@@ -535,6 +530,4 @@ function abort(message /*: string */) {
535530

536531
module.exports = {
537532
prepareHermesArtifactsAsync,
538-
findMatchingHermesVersion, // [macOS] re-exported from macosVersionResolver.js
539-
hermesCommitAtMergeBase, // [macOS] re-exported from macosVersionResolver.js
540533
};

packages/react-native/sdks/hermes-engine/utils/build-apple-framework.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ CURR_SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd -P)"
1212
IMPORT_HERMESC_PATH=${HERMES_OVERRIDE_HERMESC_PATH:-$PWD/build_host_hermesc/ImportHermesc.cmake}
1313
BUILD_TYPE=${BUILD_TYPE:-Debug}
1414

15+
# [macOS] Allow HERMES_PATH override for building from a standalone Hermes checkout
1516
HERMES_PATH=${HERMES_PATH:-"$CURR_SCRIPT_DIR/.."}
1617
REACT_NATIVE_PATH=${REACT_NATIVE_PATH:-$CURR_SCRIPT_DIR/../../..}
1718

packages/react-native/sdks/hermes-engine/utils/build-ios-framework.sh

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ function get_architecture {
2020
echo "arm64"
2121
elif [[ $1 == "appletvsimulator" ]]; then
2222
echo "x86_64;arm64"
23-
elif [[ $1 == "catalyst" || $1 == "macosx" ]]; then
23+
elif [[ $1 == "catalyst" || $1 == "macosx" ]]; then # [macOS] added macosx
2424
echo "x86_64;arm64"
2525
else
2626
echo "Error: unknown architecture passed $1"
@@ -29,10 +29,12 @@ function get_architecture {
2929
}
3030

3131
function get_deployment_target {
32-
if [[ $1 == "macosx" ]]; then
33-
echo "$(get_mac_deployment_target)"
34-
elif [[ $1 == "xros" || $1 == "xrsimulator" ]]; then
32+
if [[ $1 == "xros" || $1 == "xrsimulator" ]]; then
3533
echo "$(get_visionos_deployment_target)"
34+
# [macOS
35+
elif [[ $1 == "macosx" ]]; then
36+
echo "$(get_mac_deployment_target)"
37+
# macOS]
3638
else # tvOS and iOS use the same deployment target
3739
echo "$(get_ios_deployment_target)"
3840
fi
@@ -55,7 +57,7 @@ function build_framework {
5557
# group the frameworks together to create a universal framework
5658
function build_universal_framework {
5759
if [ ! -d destroot/Library/Frameworks/universal/hermes.xcframework ]; then
58-
create_universal_framework "macosx" "iphoneos" "iphonesimulator" "catalyst" "xros" "xrsimulator" "appletvos" "appletvsimulator"
60+
create_universal_framework "macosx" "iphoneos" "iphonesimulator" "catalyst" "xros" "xrsimulator" "appletvos" "appletvsimulator" # [macOS] added macosx
5961
else
6062
echo "Skipping; Clean \"destroot\" to rebuild".
6163
fi
@@ -65,7 +67,7 @@ function build_universal_framework {
6567
# this is used to preserve backward compatibility
6668
function create_framework {
6769
if [ ! -d destroot/Library/Frameworks/universal/hermes.xcframework ]; then
68-
build_framework "macosx"
70+
build_framework "macosx" # [macOS]
6971
build_framework "iphoneos"
7072
build_framework "iphonesimulator"
7173
build_framework "appletvos"

0 commit comments

Comments
 (0)