-
Notifications
You must be signed in to change notification settings - Fork 25.2k
refactor(iOS): include resources like bundles into the precompiled XCFrameworks #57305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
chrfalch
wants to merge
28
commits into
main
Choose a base branch
from
chrfalch/prebuilt-resources
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+4,131
−1,079
Open
Changes from all commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
68f9046
feat(ios): headers-spec layout + ReactNativeHeaders compose
chrfalch 705b310
refactor(ios): remove clang VFS overlay, resolve headers via frameworks
chrfalch 9ee4199
fix(ios): serve the react/ namespace as a module for SPM consumers
chrfalch 18735a0
fix(cocoapods): install prebuilt RNCore pods as header-less facades
chrfalch 02f3500
style: apply prettier to ios-prebuild scripts and docs
chrfalch 977f1ae
ci(ios): include ReactNativeHeaders.xcframework in the prebuilt core …
chrfalch ec979d3
feat(ios): expose allowlisted private React headers in the React modu…
chrfalch 556abab
feat(ios): emit per-namespace umbrella for React_RCTAppDelegate (R10)
chrfalch e53e6b2
fix(ios): re-vend RCTFabricComponentsPlugins.h from the React-RCTFabr…
chrfalch 42d8310
fix(ios-prebuild): fail closed when a deps namespace is missing from …
chrfalch 51d4e7e
fix(ios-prebuild): stage deps headers in compose-xcframework + sort t…
chrfalch 7b3be97
fix(ios): harden prebuilt header layout consumption (review findings)
chrfalch cd67dee
feat(ios-prebuild): generator-time headers gate + drift hardening (R1…
chrfalch e6ab638
style: prettier-format headers-rules.md (format-check lints md too)
chrfalch 6648f90
style: add language tags to headers-rules.md fenced blocks (markdownl…
chrfalch dcb5ad8
style: sort requires in headers-inventory.js (eslint sort-imports, ma…
chrfalch b4b1a94
fix(ios-prebuild): do not relocate SocketRocket headers into ReactNat…
chrfalch 24a65c9
fix(ios-prebuild): address review — enforce inventory collisions, har…
chrfalch 1e995d2
fix(ios-prebuild): sort child_process requires alphabetically (eslint…
chrfalch 052b7f9
fix(ios-prebuild): make cp clonefile flag platform-conditional
chrfalch f8f5eb2
fix(ios-prebuild): execFileSync arg arrays for the stub-xcframework t…
chrfalch 2603d7f
feat(ios-prebuild): embed React.framework's non-header resources for …
chrfalch 6326810
fix(cocoapods): define podspec_dir in RNCoreFacades.generate
chrfalch 0c7a28a
fix(ios-prebuild): address review on prebuilt framework resources
chrfalch 1d64d26
fix(ios-prebuild): resolve Flow errors in framework-resources.js
chrfalch 7b29638
fix(ios-prebuild): use platform-conditional cp flags for the i18n bun…
chrfalch 3aa78b0
fix(ios): warn in dev when RCTI18nStrings.bundle cannot be resolved
chrfalch c337763
fix(ios-prebuild): allowlist + drift gate for React.framework privacy…
chrfalch File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocking — needs verification before merge.
This design ships the privacy manifest and
RCTI18nStrings.bundleonly from insideReact.xcframework'sReact.framework; the prebuilt facade carries no pod-levelresource_bundles/resources(seerncore_facades.rb: "the facade carries NO resources"). We still install the prebuilt via CocoaPods, and most apps link their pod graph statically (the New Architecture default: static libraries, nouse_frameworks!).CocoaPods copies resources nested inside a
.frameworkonly when it embeds that framework. It does embed vendored dynamic frameworks (via the[CP] Embed Pods Frameworksphase), and the prebuilt is currently builttype: .dynamic(scripts/releases/ios-prebuild/swift-package.js), so in principle the nested bundle should be copied even in a static-pods app. But this is silent if it breaks — you'd get untranslated strings (RCTLocalizedStringFromKeyfalling back to the English default) and a missingPrivacyInfo.xcprivacyin the app's privacy report, with no build error.Could we verify on a real app with the default CocoaPods setup (static libraries, no
use_frameworks!) that:React.framework/RCTI18nStrings.bundleis actually copied intoApp.app/Frameworks/React.framework/andRCTLocalizedStringFromKeyreturns translated strings, andPrivacyInfo.xcprivacyis aggregated into the generated privacy report?Also worth checking
use_frameworks! :linkage => :staticand app-extension / Mac Catalyst targets, which don't always get an embed-frameworks phase.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified on a real app against the locally-built prebuilt, in both configurations you asked for plus an archive — harness and logs retained.
1. Default CocoaPods (static libraries, no
use_frameworks!):App.app/Frameworks/React.framework/containsRCTI18nStrings.bundle(all 37 lprojs) andPrivacyInfo.xcprivacy;codesign --verify --deepis clean on the embedded framework. CocoaPods embeds vendored dynamic frameworks via[CP] Embed Pods Frameworksregardless of the pod graph's linkage, and the embed carries nested resources. A runtime probe replicatingRCTI18nStringsBundle()'s exact chain resolves both resources from the embedded framework at launch (codeBundle=…/HelloWorld.app/Frameworks/React.framework,locales=37).2.
use_frameworks! :linkage => :static: identical results on every check — the linkage option affects how CocoaPods builds source pods; a vendored dynamic binary is still embedded.3. Privacy aggregation:
PrivacyInfo.xcprivacysits at the embedded framework root inside the.xcarchive's app — the documented input for Apple's report aggregation (Organizer's "Generate Privacy Report" itself is GUI-only, so manifest-in-archive is the automatable evidence).4. Extensions / Catalyst: app extensions intentionally get no embed phase of their own — the framework embeds once into the host app and extensions load it via
@rpath, andbundleForClass:resolves to wherever the code actually loaded from. The maccatalyst slice carries its own copies of both resources.On "returns translated strings" — a finding worth having on the record: this is not demonstrable on OSS with or without this PR, because every
Localizable.stringsunderReact/I18n/strings/*.lproj(onmaintoo) is a generated stub with zero key/value pairs; the fbt binary packs are only consumed by the internalWITH_FBI18Npath. OSSRCTLocalizedStringFromKeyhas therefore always returned the untranslated default, and this PR preserves exactly that behavior while making the bundle (and any future translations) actually ship. The probe proves the resolution mechanism end-to-end: the default came back from the resolved bundle'slocalizedStringForKey:, not from the bundle-missing fallback.To de-silence the failure mode you flagged, I've also pushed a hardening commit:
RCTI18nStringsBundle()now emits a dev-only, once-per-processRCTLogWarnwhen the bundle cannot be resolved, naming both consequences (untranslated strings + manifest missing from the privacy report). Release builds are unchanged.