-
Notifications
You must be signed in to change notification settings - Fork 62
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
(draft) (feat) Migrate to SPM (Swift Package Manager) #131
base: master
Are you sure you want to change the base?
Conversation
I noticed you closed this PR. Do you not want it merged? |
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 doing this work. Please feel under no obligation to implement this review, I can take it from here if necessary.
I just have one concern. Will this PR break support for the Capacitor versions listed here? I am assuming yes, because previous versions of Capacitor used the Objective-C bridging files that have been deleted as part of this PR.
s.source_files = 'ios/Sources/**/*.{swift,h,m,c,cc,mm,cpp}' | ||
s.ios.deployment_target = '14.0' | ||
s.dependency 'Capacitor' | ||
s.swift_version = '5.1' |
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.
Is it necessary to specify a Swift version?
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.
Probably yes (but not sure) due to the new SPM structure.
ios/Tests/BackgroundGeolocationPluginTests/BackgroundGeolocationPluginTests.swift
Outdated
Show resolved
Hide resolved
ios/Sources/BackgroundGeolocationPlugin/BackgroundGeolocation.swift
Outdated
Show resolved
Hide resolved
I opened this PR as a Work in Progress (WIP) or a draft because the proposed feature is not yet complete. I closed it to avoid confusion, but I plan to reopen it once it's ready for review. Let me know if you have any concerns! |
Yes, this PR would likely break support for the listed Capacitor versions, which is one of the reasons I decided to close it. Additionally, this plugin has an atypical structure. Specifically, it includes a precompiled definitions.d.ts instead of the more common src/ folder with definitions.ts, web.ts, and index.ts. If you’re interested, I have locally recreated this structure. I initially attempted to convert the plugin using the official Capacitor Plugin Converter, but it didn’t work. So, I rebuilt the entire plugin structure using Create Capacitor Plugin and then manually adjusted the iOS implementation accordingly. Additionally, several expected files were missing from the original structure. Let me know if you’d like more details or if you have any suggestions! |
I re-open as a Draft. |
ios/Sources/BackgroundGeolocationPlugin/BackgroundGeolocation.swift
Outdated
Show resolved
Hide resolved
ios/Sources/BackgroundGeolocationPlugin/BackgroundGeolocation.swift
Outdated
Show resolved
Hide resolved
I attempted to strip it down to the bare minimum, to reduce its dependencies and make it easier to understand.
This plugin does not require a JS component at all, and I only added the TypeScript definitions at the request of TypeScript users.
The changes to Plugin.swift look backwards compatible. It should be possible to reintroduce the Objective-C and project files to avoid breakage. What do you think? |
To verify whether the older versions of Capacitor (prior to version 6) require these changes, we need to test on those specific versions. This is because the new SPM already has its methods (for the JavaScript part) declared directly in BackgroundGeolocation.swift (CAPBridgedPlugin): public let pluginMethods: [CAPPluginMethod] = [
CAPPluginMethod(name: "addWatcher", returnType: CAPPluginReturnCallback),
CAPPluginMethod(name: "removeWatcher", returnType: CAPPluginReturnPromise),
CAPPluginMethod(name: "openSettings", returnType: CAPPluginReturnPromise)
] |
If |
(DRAFT)
Description
This pull request migrates the iOS portion of the Capacitor plugin from CocoaPods to Swift Package Manager (SPM) while preserving all previous functionality.
No breaking changes have been introduced.
Why?
Let me know if further changes or testing are required!