Skip to content
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

WIP - feat: Add support for SPM plugins #1430

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

jcesarmobile
Copy link
Member

@jcesarmobile jcesarmobile commented Apr 27, 2024

Remove CordovaLib from the template and instead add a local "empty" swift package, in which cordova-plugin-add will add the plugins as dependencies if they are swift package manager compatible.

Plugin example to test with: apache/cordova-plugin-device#196

Using this approach plugins that want to add Swift Package Manager dependencies will have to be converted to be Swift Package Manager compatible and add dependences as they would to a regular Swift Package instead of relying in a new tag in plugin.xml

@dpogue
Copy link
Member

dpogue commented Apr 27, 2024

There's a bit of Xcode mangling the project file (stripping licence comments, removing quotes around __PROJECT_NAME__ and __PROJECT_ID__, etc.) that will probably need to be fixed by hand.

The idea of a SwiftPM subproject is a good one and will make the process of adding/removing plugins significantly easier than trying to deal with the Xcode project directly. That said, would it make sense for CordovaLib to be referenced directly from the Xcode project rather than through the SwiftPM library?

One worry is that we won't be able to ship with CordovaLib pointing at master, we'll need to update that to point at a tag (which is annoying because that needs to happen as part of creating the tag to which it will point)

@jcesarmobile
Copy link
Member Author

Keeping CordovaLib was a good call as it allows plugins to not have a dependency to cordova-ios, I tried several things like using an environment variable for setting the cordova-ios versions to use based on the installed version, and worked when running the app from the CLI, but was not working if running from Xcode since the variable is unset when the node process stops.
A similar option was to set the version on the Package.swift on platfor/plugin add, but as you mentioned, the tag has to exists, which is a problem for dev versions.

Also tried using cordova-ios from node_modules directly and works fine for the CordovaPluginsSPM project, but for plugins it would require to add cordova-ios as a dev dependency on plugins and then adjusting paths once installed in an app since the path to cordova-ios would be different when used from an app and from the plugin directory itself.

While I would have loved using the local version so we have the privacy manifest linked and on plugins we could run xcodebuild command to see if the plugin code builds without adding it to an app, keeping CordovaLib is the easier and less conflicting option.

@dpogue
Copy link
Member

dpogue commented Apr 29, 2024

hmmm, I'd been hoping that we would be able to reference CordovaLib using SwiftPM, but directly in the app's Xcode project rather than through the CordovaPluginsSPM project's Package.swift... but if I'm reading your comment correctly that still causes issues for plugins? 😞

@jcesarmobile
Copy link
Member Author

jcesarmobile commented Apr 29, 2024

I didn't try that, but still, it would be a complex thing to do since if you add it to the template, when the template is turned into an app the path to cordova-ios's Package.swift will be different, so should be adjusted in the xcodeproj file.

Could be possible by having a Package.swift inside CordovaLib folder so the path is always the same, but would require having two Package.swift slightly different from each other, which could cause confusion and bugs if somebody edits one but not the other.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 15.38462% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 77.91%. Comparing base (df4b8a6) to head (29fbbff).
Report is 1 commits behind head on master.

Files Patch % Lines
lib/Api.js 15.38% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1430      +/-   ##
==========================================
- Coverage   78.35%   77.91%   -0.45%     
==========================================
  Files          16       16              
  Lines        1825     1838      +13     
==========================================
+ Hits         1430     1432       +2     
- Misses        395      406      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants