chore: sync upstream PR #8463 - fix(cli): mark Cordova CocoaPods plugins as incompatible in SPM#74
chore: sync upstream PR #8463 - fix(cli): mark Cordova CocoaPods plugins as incompatible in SPM#74riderx wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughThe change adds Swift Package Manager (SPM) support to iOS plugin handling in ChangesSPM Package Manager Support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4577bd063
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| incompatibleCordovaPlugins = incompatibleCordovaPlugins.concat(cordovaPlugins.filter((p) => needsSPMDep(p))); | ||
| cordovaPlugins = cordovaPlugins.filter((p) => !needsSPMDep(p)); |
There was a problem hiding this comment.
When SPM is enabled, this code removes some Cordova plugins from cordovaPlugins for installation, but those plugins keep PluginType.Cordova. checkPluginDependencies() later recomputes Cordova plugins from type and still treats these skipped plugins as installed, so dependents will not be reported as missing dependencies even though the plugin was excluded from native integration. In projects where a compatible Cordova plugin depends on one of these skipped CocoaPods-based plugins, this can produce unresolved native references without any warning.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
cli/src/ios/update.ts (2)
693-693: ⚡ Quick winUse the
platformconstant for consistency.The function uses a hardcoded
'ios'string instead of theplatformconstant defined at line 33. Using the constant improves consistency and maintainability.♻️ Proposed fix
- const podspecs = getPlatformElement(plugin, 'ios', 'podspec'); + const podspecs = getPlatformElement(plugin, platform, 'podspec');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/src/ios/update.ts` at line 693, The call that computes platformTag uses a hardcoded string instead of the existing platform constant; update the invocation of getPluginPlatform to pass the platform variable (platform) rather than the literal 'ios' so getPluginPlatform(plugin, platform) uses the defined constant consistently (change the place that currently hardcodes 'ios' to use the platform constant and ensure platformTag is assigned from getPluginPlatform(plugin, platform)).
691-695: ⚡ Quick winConsider renaming function for clarity.
The function name
needsSPMDepis potentially misleading—it suggests the plugin needs SPM dependencies, but it actually identifies plugins that require CocoaPods (have podspecs but no SPM package) and are therefore incompatible with SPM. Consider a more descriptive name likerequiresCocoaPods,hasOnlyCocoaPodsDeps, orisIncompatibleWithSPM.Additionally, adding a JSDoc comment would help clarify the function's purpose:
/** * Checks if a plugin requires CocoaPods dependencies and is incompatible with SPM. * Returns true if the plugin has podspec elements but no SPM package defined. */🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/src/ios/update.ts` around lines 691 - 695, Rename the function needsSPMDep to a clearer name such as requiresCocoaPods or isIncompatibleWithSPM and add a short JSDoc above it explaining it "Checks if a plugin requires CocoaPods dependencies and is incompatible with SPM; returns true if the plugin has podspec elements but no SPM package defined." Update any call sites to use the new function name and keep the internal logic that uses getPlatformElement(plugin, 'ios', 'podspec') and getPluginPlatform(plugin, platform) (checking podspecs.length > 0 && !platformTag.$?.package) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cli/src/ios/update.ts`:
- Line 693: The call that computes platformTag uses a hardcoded string instead
of the existing platform constant; update the invocation of getPluginPlatform to
pass the platform variable (platform) rather than the literal 'ios' so
getPluginPlatform(plugin, platform) uses the defined constant consistently
(change the place that currently hardcodes 'ios' to use the platform constant
and ensure platformTag is assigned from getPluginPlatform(plugin, platform)).
- Around line 691-695: Rename the function needsSPMDep to a clearer name such as
requiresCocoaPods or isIncompatibleWithSPM and add a short JSDoc above it
explaining it "Checks if a plugin requires CocoaPods dependencies and is
incompatible with SPM; returns true if the plugin has podspec elements but no
SPM package defined." Update any call sites to use the new function name and
keep the internal logic that uses getPlatformElement(plugin, 'ios', 'podspec')
and getPluginPlatform(plugin, platform) (checking podspecs.length > 0 &&
!platformTag.$?.package) unchanged.
|
@riderx can you please don't ping me? |
2 participants
Upstream PR Sync
This PR syncs changes from an external contributor's PR on the official Capacitor repository.
Original PR
Automation
Synced from upstream by Capacitor+ Bot
Summary by CodeRabbit