-
Notifications
You must be signed in to change notification settings - Fork 25k
Allow library podspec to declare Swift Package Manager dependencies #44627
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
Conversation
|
Hi @mfazekas! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
cipolleschi
left a comment
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.
Thanks for the amazing PR. 👏
I left some comments in the code to simplify it a little bit and to make it easier to use.
I also have a couple of questions:
1.) only works
USE_FRAMEWORKS=dynamic pod install
Does it works with static frameworks?
2.) .xcworkspace needs to be reopened after pod install - this could be worked around by not removing/readding spm dependencies
Do you think there is a way to detect whether dependencies changed so that we can emit a red message to prompt the user to close and reopen the .xcworkspace project?
Alternatively, we can kill Xcode, but if a person has multiple project open, that could be annoying.
| def self.spm_dependency(s, url:, requirement:, products:) | ||
| SPM.dependency(s, url: url, requirement: requirement, products: products) | ||
| end |
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.
To simplify the usage for 3rd party dependencies, I'd properly move this function from the utils.rb directly to the react_native_pods.
In this way, instead of the check:
if const_defined?(:ReactNativePodsUtils) && ReactNativePodsUtils.respond_to?(:spm_dependency)
ReactNativePodsUtils.spm_dependency(s,
url: 'https://github.com/apple/swift-atomics.git',
requirement: {kind: 'upToNextMajorVersion', minimumVersion: '1.1.0'},
products: ['Atomics']
)
else
raise "Please upgrade React Native to >=0.75.0 to use SPM dependencies."
end
they can just:
if defined?(:spm_dependency)
spm_dependency(s,
url: 'https://github.com/apple/swift-atomics.git',
requirement: {kind: 'upToNextMajorVersion', minimumVersion: '1.1.0'},
products: ['Atomics']
)
else
raise "Please upgrade React Native to >=0.75.0 to use SPM dependencies."
end
I would also consider backporting this to all the RN versions in the supported window (right now: 0.72, 0.73, 0.74) so that they don't even need to if-else.
What do you think?
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.
Added as spm_dependency.
As noted #44627 (comment) this approach does depend on what linking (static/dynamic) the RN framework uses and what linking does the SPM product uses. This is something to consider with backporting, as it might require some further changes to be usefull in all scenarios
It's a bit complicated. I've tested with Alamofire Swift Package Manager package that has a dynamic and static product.
Now
According to testing in Xcode 15.4, if there is a single swift package manager dependency, the regenerating the project might loose the dependency in the UI. (Not 100% reproducible) spm-reload.movWe can write an osascript that reloads the project if it's open, but it's a bit hacky #!/usr/bin/osascript -l JavaScript
function main() {
const app = Application("Xcode")
const active = app.activeWorkspaceDocument();
console.log("Active", app.activeWorkspaceDocument().file());
const worspacePath = "/Users/boga/Work/OSS/react-native-sim/rn-spm-rfc-poc/example/ios/RnSpmRfcPocExample.xcworkspace"
if (app.activeWorkspaceDocument().file() == filePath) {
app.activeWorkspaceDocument().close();
app.open(filePath);
}
}
main() |
… code update based on review
|
Sorry for the late reply. I'm still catching up after the conferences of the past weeks. One thing we can add is a big warning message that runs only if SPM dependencies are found.
What do you think? |
I've tested this with new Xcode 16.0 beta and close/reopen issue is solved there. So likely some warning that
The SPM might be a simple Swift Macro and in that case it works with all linking. If it's a static library then static linking causes link errors with duplicate symbols. If it's a dyanamic library, then static linking only works if the app also links with the dynamic library, otherwise they'll see undefined symbols. So not sure what's the good message here.
Maybe we also add a url to this issue, or a new one created just for explaining the potential issues with SPM regarding linking?
Yes given that the reload issue seems to get's solved by new Xcode version, it don't think it's worth to mess with osascript workaround, so message is good there. The linking part is a bit complicated, not sure what's a good message there. |
… show warning after install
|
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@cipolleschi merged this pull request in f903f34. |
|
This pull request was successfully merged by @mfazekas in f903f34. When will my fix make it into a release? | How to file a pick request? |
…44627) Summary: React-Native uses Cocapods for native dependency management on iOS. While CocoaPods is flexible and popular, Apple's Swift Package Manager is the new standard. Currently consuming packages available only via Swift Package Manager is not possible. This change implements a single extension so .podspec files can declare Swift Package Manager dependencies via ```ruby ReactNativePodsUtils.spm_dependency(s, url: 'https://github.com/apple/swift-atomics.git', requirement: {kind: 'upToNextMajorVersion', minimumVersion: '1.1.0'}, products: ['Atomics'] ) ``` bypass-github-export-checks ## Changelog: [IOS] [ADDED] - libraries can now declare Swift Package Manager dependencies in their .podspec with `ReactNativePodsUtils.spm_dependency` Pull Request resolved: #44627 Test Plan: https://github.com/mfazekas/rn-spm-rfc-poc/ Is a simple demo for the feature: 1. Podspec declare dependency with: ```ruby if const_defined?(:ReactNativePodsUtils) && ReactNativePodsUtils.respond_to?(:spm_dependency) ReactNativePodsUtils.spm_dependency(s, url: 'https://github.com/apple/swift-atomics.git', requirement: {kind: 'upToNextMajorVersion', minimumVersion: '1.1.0'}, products: ['Atomics'] ) else raise "Please upgrade React Native to >=0.75.0 to use SPM dependencies." end ``` 2. [`import Atomics`](https://github.com/mfazekas/rn-spm-rfc-poc/blob/e4eb1034f7498dedee4cb673d327c34a6048bda2/ios/MultiplyInSwift.swift#L1C2-L1C15) and [`ManagedAtomic`](https://github.com/mfazekas/rn-spm-rfc-poc/blob/e4eb1034f7498dedee4cb673d327c34a6048bda2/ios/MultiplyInSwift.swift#L7-L13) is used in the code 3.) `spm_dependency` causes the dependency to be added via `post_install` hook in the workspace <img width="261" alt="image" src="https://github.com/facebook/react-native/assets/52435/ad6aee1c-ac88-4c84-8aa3-50e148c4f5b2"> 4.) `spm_dependecy` causes the library to be linked with `Atomics` library <img width="817" alt="image" src="https://github.com/facebook/react-native/assets/52435/bfc8dfc0-aeb7-4c75-acbd-937eab1cbf80"> Limitations: 1.) only works `USE_FRAMEWORKS=dynamic pod install` otherwise the linker fails [with known Xcode issue - duplicate link issue](https://forums.swift.org/t/objc-flag-causes-duplicate-symbols-with-swift-packages/27926) 2.) .xcworkspace needs to be reopened after `pod install` - this could be worked around by not removing/readding spm dependencies ### See also: react-native-community/discussions-and-proposals#587 (comment) react-native-community/discussions-and-proposals#787 Reviewed By: cortinico Differential Revision: D58947066 Pulled By: cipolleschi fbshipit-source-id: ae3bf955cd36a02cc78472595fa003cc9e843dd5

Summary:
React-Native uses Cocapods for native dependency management on iOS. While CocoaPods is flexible and popular, Apple's Swift Package Manager is the new standard. Currently consuming packages available only via Swift Package Manager is not possible. This change implements a single extension so .podspec files can declare Swift Package Manager dependencies via
Changelog:
[IOS] [ADDED] - libraries can now declare Swift Package Manager dependencies in their .podspec with
spm_dependencyTest Plan:
https://github.com/mfazekas/rn-spm-rfc-poc/
Is a simple demo for the feature:
Podspec declare dependency with:
import AtomicsandManagedAtomicis used in the code3.)
spm_dependencycauses the dependency to be added viapost_installhook in the workspace4.)
spm_dependecycauses the library to be linked withAtomicslibraryLimitations: #
1.) Some package target types will work with only some USE_FRAMEWORKS settings:
1️⃣: more complex swift macros usually have runtime component - limitation might apply, as table above will apply
2️⃣: caused by a Xcode linking issue ObjC flag - duplicate link issue
3️⃣: can be fixed by linking the library to the app itself
2.) .xcworkspace needs to be reopened after
pod install- this seems to be an Xcode bug and fixed in Xcode 16 beta.See also:
react-native-community/discussions-and-proposals#587 (comment)
react-native-community/discussions-and-proposals#787