chore: sync upstream PR #8524 - fix(ios): remove Cordova.framework from Capacitor project - #82
chore: sync upstream PR #8524 - fix(ios): remove Cordova.framework from Capacitor project#82riderx wants to merge 4 commits into
Conversation
|
Claude Code could not resolve this upstream sync completely. Please review the branch carefully before merging. |
Beta npm buildMaintainers can publish one Capacitor Plus workspace package from this PR to npm for fast testing. Comment Examples: /publish-beta core
/publish-beta cli
/publish-beta @capacitor-plus/coreIf exactly one workspace package changed, Packages:
The workflow will:
Security note: beta publish is only enabled for branches inside this repository. |
|
Important Approval pendingCodeRabbit has no unresolved comments, but it has not reviewed the latest commit. Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.
📝 WalkthroughWalkthroughThis PR decouples Cordova support from Capacitor core on Android and iOS into standalone CordovaPlugin implementations using a new generic call-interceptor mechanism, bumps Android/iOS SDK and toolchain versions, refactors CLI iOS Cordova podspec/SPM logic into shared utilities, replaces CLI live-reload flags with a single ChangesCordova Decoupling (Core)
Estimated code review effort: 4 (Complex) | ~75 minutes Toolchain/SDK Bumps
CLI Cordova/SPM and Run Command
Estimated code review effort: 4 (Complex) | ~60 minutes Metadata & Release
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant WebView
participant Bridge
participant CordovaPlugin
participant PluginManager
WebView->>Bridge: postMessage(type="cordova", data)
Bridge->>Bridge: getCallInterceptor("cordova")
Bridge->>CordovaPlugin: intercept(service, action, callbackId, actionArgs)
CordovaPlugin->>PluginManager: exec(...)
PluginManager-->>CordovaPlugin: result
CordovaPlugin-->>WebView: callback response
sequenceDiagram
participant CLI
participant runCommand
participant CapLiveReloadHelper
participant AppConfig
CLI->>runCommand: run --url http://host:port
runCommand->>runCommand: validate URL protocol
runCommand->>CapLiveReloadHelper: editCapConfigForLiveReload(config, platform, url)
CapLiveReloadHelper->>AppConfig: write server.url to capacitor.config.json
runCommand-->>CLI: log "listening for: url"
Related PRs: None identified in the provided context. Suggested labels: breaking-change, cordova, android, ios, cli, release Suggested reviewers: None specified. 🐰 Cordova hops away, a plugin now apart, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Warning Review ran into problems🔥 ProblemsRepository analysis: Couldn't refresh Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
android/capacitor/src/main/java/com/getcapacitor/plugin/util/HttpRequestHandler.java (1)
163-173: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMissing
#delimiter corrupts URLs containing a fragment.
uri.getFragment()returns the fragment value without the leading#. Appending it directly after the query string produces a malformed URL (e.g....?foo=bar#sectionbecomes...?foo=barsection), silently corrupting any request whose URL contains a fragment.🐛 Proposed fix
String unEncodedUrlString = uri.getScheme() + "://" + uri.getAuthority() + uri.getPath() + (!urlQuery.equals("") ? "?" + urlQuery : "") + - (uri.getFragment() != null ? uri.getFragment() : ""); + (uri.getFragment() != null ? "#" + uri.getFragment() : "");🤖 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 `@android/capacitor/src/main/java/com/getcapacitor/plugin/util/HttpRequestHandler.java` around lines 163 - 173, The URL reconstruction in HttpRequestHandler is missing the fragment delimiter, so URLs with fragments are being concatenated incorrectly. Update the logic that builds unEncodedUrlString in the request setup path to append the fragment from uri.getFragment() with a leading # only when present, keeping the existing scheme, authority, path, and query handling intact. Use the URL/URI assembly block in HttpRequestHandler to locate the fix.android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java (1)
176-192: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRemove the API 35 gate around root insets
android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java:176-192—ViewCompat.getRootWindowInsets(v)is compat-safe on older Android versions too; this branch sets the safe-area CSS vars to0on API 24-34 until the listener runs. Call it unconditionally here, or move the Android 15-specific check to the code that actually needs it.🤖 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 `@android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java` around lines 176 - 192, The safe-area initialization in SystemBars.initSafeAreaCSSVariables() is unnecessarily gated by the Android 15 check, which forces zero insets on older versions before listeners update. Remove the API 35-specific branch around ViewCompat.getRootWindowInsets(v) and fetch root window insets unconditionally here; keep any Android 15-only logic only in the code path that truly depends on it.CHANGELOG.md (1)
6-119: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftCHANGELOG.md was manually edited, and merge conflict resolution left duplicated/inconsistent content.
Per repo guidelines,
CHANGELOG.mdshould not be manually edited since it's managed by CI/CD. This diff manually edits it as part of the merge sync, and the result contains clear artifacts of an unresolved conflict:
- The
9.0.0-alpha.4header/entry appears twice (Line 14 and Line 37).- The entire
9.0.0-alpha.0"Bug Fixes" section is duplicated verbatim (Lines 26-36 and Lines 61-71).- Lines 111-115 mix
ionic-team/capacitorcompare/issue/commit URLs withCap-go/capacitor-plusissue/commit URLs within the same entry, leaving a broken/inconsistent changelog history.This matches the PR's own note that "Claude Code could not fully resolve the upstream sync" and should be cleaned up (or regenerated via the normal CI/CD changelog process) before merging.
As per coding guidelines, "Do not manually edit
CHANGELOG.md; it is managed automatically by CI/CD."🤖 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 `@CHANGELOG.md` around lines 6 - 119, The CHANGELOG content has unresolved merge artifacts and inconsistent history that need to be removed. Clean up the duplicated 9.0.0-alpha.4 and 9.0.0-alpha.0 entries, then make the release notes consistent with the upstream Capacitor history by fixing the mixed compare/issue/commit references in the 8.3.1/8.3.2 area. Use the existing changelog section headers and release entries as the anchor points, and ensure the final file is either regenerated by the normal CI/CD changelog flow or left in a single, non-duplicated, internally consistent state.Source: Coding guidelines
🤖 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.
Inline comments:
In `@android/capacitor/src/main/java/com/getcapacitor/Bridge.java`:
- Around line 293-308: Guard the WebView package lookup in
isMinimumWebViewInstalled() by checking the result of
WebView.getCurrentWebViewPackage() before using it, and also handle a null
versionName safely. If the package info or version string is unavailable, return
false immediately so the version parsing logic only runs when info is present.
In `@android/CHANGELOG.md`:
- Around line 6-84: The android/CHANGELOG.md release history contains duplicate
entries for several versions, which creates conflicting compare links and
repeated release notes. Remove the repeated blocks for 9.0.0-alpha.4,
9.0.0-alpha.3, 9.0.0-alpha.0, 8.4.0, 8.3.4, and 8.3.3 so each release appears
only once, keeping the correct compare link and notes in the remaining canonical
entry.
In `@cli/src/ios/update.ts`:
- Around line 65-88: The `update` flow in `cli/src/ios/update.ts` is recomputing
`getCapacitorPackageVersion` for every plugin even though `iosPlatformVersion`
is independent of `plugin`; hoist that await out of the
`Promise.all`/`validSPMPackages.map` loop and reuse the single value inside the
per-plugin work. While updating the `Package.swift` parsing logic, make sure the
`major(version)` comparison in the `regex`/`content.match` path is guarded so
malformed or missing semver values do not throw during `update`.
In `@cli/src/tasks/migrate.ts`:
- Line 3: The migration task is importing rimraf as a default export, but rimraf
v6 only exposes named exports, so the current sync call will break at runtime.
Update the import in migrate.ts to use the named sync export (for example,
rimrafSync) and change the existing rimraf.sync(...) usage in the migration
logic to call that named function instead.
In `@cli/src/util/cordova-ios.ts`:
- Around line 299-318: In cordovaPodfileLines, remove the unnecessary async from
the cordovaPlugins.map callback because it does not await anything and the logic
is synchronous. Update the loop in cordova-ios.ts so the callback used to walk
cordovaPlugins, getPlatformElement, and podspec/pods/pod stays plain synchronous
code, preserving the current pods accumulation without implying async behavior.
- Around line 340-344: generateCordovaPackageFiles is returning before the async
per-plugin work completes because the cordovaPlugins.map callback does not await
generateCordovaPackageFile. Update generateCordovaPackageFiles to wait for each
generateCordovaPackageFile call to finish, so callers like ios/update.ts do not
continue into checkPluginsForPackageSwift and generatePackageFile until all
Package.swift files are written. Use the generateCordovaPackageFiles and
generateCordovaPackageFile symbols to locate the fix.
- Around line 22-27: The async podspec writes in generateCordovaPodspecs are not
awaited, so the function can finish before the .podspec files are written and
errors may be unhandled. Update generateCordovaPodspecs in cordova-ios.ts to
await both generateCordovaPodspec calls (or run them through a Promise-based
concurrency pattern) so the plugin podspec generation completes before
installCocoaPodsPlugins continues.
In `@ios/Capacitor/Capacitor/CAPBridgeProtocol.swift`:
- Around line 82-84: The new CAPBridgeProtocol requirement
registerCallInterceptor(_:handler:) is source-breaking for existing conformers.
Update the protocol so this API is not required for all adopters, either by
moving the behavior into a protocol extension with a default implementation or
by making the interceptor registration optional in CAPBridgeProtocol while
keeping CAPBridge-related call interception functionality available.
In `@ios/Capacitor/Capacitor/CAPBridgeViewController.swift`:
- Around line 94-110: In CAPBridgeViewController.updateAppLocationIfNeeded, stop
using perform(_:) on __CordovaPlugin for cordovaDeployDisabled because it is an
`@objc` Bool and the current call can misread the scalar value. Read the property
through KVC or a typed protocol/cast on the plugin instance instead, then keep
the existing conditional logic using cordovaDeployDisabled unchanged.
In `@ios/CapacitorCordova/CapacitorCordova/Classes/Public/Plugin.swift`:
- Line 43: The cast in Plugin should use a concrete array element type instead
of the untyped Array, since `as? Array` will not compile. Update the `args`
assignment in `Plugin` to cast to `[Any]` (or the correct known element type) so
the `actionArgs` value can be read safely.
---
Outside diff comments:
In `@android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java`:
- Around line 176-192: The safe-area initialization in
SystemBars.initSafeAreaCSSVariables() is unnecessarily gated by the Android 15
check, which forces zero insets on older versions before listeners update.
Remove the API 35-specific branch around ViewCompat.getRootWindowInsets(v) and
fetch root window insets unconditionally here; keep any Android 15-only logic
only in the code path that truly depends on it.
In
`@android/capacitor/src/main/java/com/getcapacitor/plugin/util/HttpRequestHandler.java`:
- Around line 163-173: The URL reconstruction in HttpRequestHandler is missing
the fragment delimiter, so URLs with fragments are being concatenated
incorrectly. Update the logic that builds unEncodedUrlString in the request
setup path to append the fragment from uri.getFragment() with a leading # only
when present, keeping the existing scheme, authority, path, and query handling
intact. Use the URL/URI assembly block in HttpRequestHandler to locate the fix.
In `@CHANGELOG.md`:
- Around line 6-119: The CHANGELOG content has unresolved merge artifacts and
inconsistent history that need to be removed. Clean up the duplicated
9.0.0-alpha.4 and 9.0.0-alpha.0 entries, then make the release notes consistent
with the upstream Capacitor history by fixing the mixed compare/issue/commit
references in the 8.3.1/8.3.2 area. Use the existing changelog section headers
and release entries as the anchor points, and ensure the final file is either
regenerated by the normal CI/CD changelog flow or left in a single,
non-duplicated, internally consistent state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3b6e7d24-8d64-4d89-832d-0d2cae0f09f9
⛔ Files ignored due to path filters (2)
android-template/gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jarandroid/gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jar
📒 Files selected for processing (87)
.github/workflows/test.ymlCHANGELOG.mdandroid-template/app/build.gradleandroid-template/build.gradleandroid-template/gradle/wrapper/gradle-wrapper.propertiesandroid-template/gradlewandroid-template/gradlew.batandroid-template/settings.gradleandroid-template/variables.gradleandroid/CHANGELOG.mdandroid/capacitor-cordova/build.gradleandroid/capacitor-cordova/proguard-rules.proandroid/capacitor-cordova/src/main/AndroidManifest.xmlandroid/capacitor-cordova/src/main/java/com/getcapacitor/cordova/CapacitorCordovaCookieManager.javaandroid/capacitor-cordova/src/main/java/com/getcapacitor/cordova/CordovaPlugin.javaandroid/capacitor-cordova/src/main/java/com/getcapacitor/cordova/MockCordovaInterfaceImpl.javaandroid/capacitor-cordova/src/main/java/com/getcapacitor/cordova/MockCordovaWebViewImpl.javaandroid/capacitor/build.gradleandroid/capacitor/proguard-rules.proandroid/capacitor/src/main/java/com/getcapacitor/Bridge.javaandroid/capacitor/src/main/java/com/getcapacitor/BridgeWebChromeClient.javaandroid/capacitor/src/main/java/com/getcapacitor/FileUtils.javaandroid/capacitor/src/main/java/com/getcapacitor/MessageHandler.javaandroid/capacitor/src/main/java/com/getcapacitor/PluginConfig.javaandroid/capacitor/src/main/java/com/getcapacitor/WebViewLocalServer.javaandroid/capacitor/src/main/java/com/getcapacitor/plugin/CapacitorCookieManager.javaandroid/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.javaandroid/capacitor/src/main/java/com/getcapacitor/plugin/util/CapacitorHttpUrlConnection.javaandroid/capacitor/src/main/java/com/getcapacitor/plugin/util/HttpRequestHandler.javaandroid/capacitor/src/main/java/com/getcapacitor/util/JSONUtils.javaandroid/capacitor/src/test/java/com/getcapacitor/plugin/SystemBarsTest.javaandroid/gradle/wrapper/gradle-wrapper.propertiesandroid/gradlewandroid/gradlew.batandroid/package.jsonandroid/settings.gradlecapacitor-cordova-android-plugins/build.gradlecli/CHANGELOG.mdcli/package.jsoncli/src/android/update.tscli/src/config.tscli/src/declarations.tscli/src/index.tscli/src/ios/update.tscli/src/ipc.tscli/src/tasks/migrate.tscli/src/tasks/run.tscli/src/telemetry.tscli/src/util/cordova-ios.tscli/src/util/iosplugin.tscli/src/util/livereload.tscli/src/util/spm.tscli/test/telemetry.spec.tscore/CHANGELOG.mdcore/package.jsonios-pods-template/App/App.xcodeproj/project.pbxprojios-pods-template/App/App/AppDelegate.swiftios-pods-template/App/Podfileios-spm-template/App/App.xcodeproj/project.pbxprojios-spm-template/App/App/AppDelegate.swiftios-spm-template/App/CapApp-SPM/Package.swiftios/CHANGELOG.mdios/Capacitor.podspecios/Capacitor/Capacitor.xcodeproj/project.pbxprojios/Capacitor/Capacitor/CAPApplicationDelegateProxy.swiftios/Capacitor/Capacitor/CAPBridgeProtocol.swiftios/Capacitor/Capacitor/CAPBridgeViewController+CDVScreenOrientationDelegate.hios/Capacitor/Capacitor/CAPBridgeViewController+CDVScreenOrientationDelegate.mios/Capacitor/Capacitor/CAPBridgeViewController.swiftios/Capacitor/Capacitor/CAPInstanceDescriptor.hios/Capacitor/Capacitor/CAPInstanceDescriptor.mios/Capacitor/Capacitor/CAPInstanceDescriptor.swiftios/Capacitor/Capacitor/CapacitorBridge.swiftios/Capacitor/Capacitor/JS.swiftios/Capacitor/Capacitor/JSExport.swiftios/Capacitor/Capacitor/PluginConfig.swiftios/Capacitor/Capacitor/TmpViewController.swiftios/Capacitor/Capacitor/WebViewDelegationHandler.swiftios/Capacitor/CapacitorTests/ConfigurationTests.swiftios/Capacitor/CapacitorTests/PluginCallAccessorTests.mios/CapacitorCordova.podspecios/CapacitorCordova/CapacitorCordova.xcodeproj/project.pbxprojios/CapacitorCordova/CapacitorCordova/Classes/Public/Plugin.swiftios/package.jsonios/scripts/pods_helpers.rblerna.jsonscripts/publish-android.sh
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
💤 Files with no reviewable changes (10)
- ios/Capacitor/Capacitor/CAPBridgeViewController+CDVScreenOrientationDelegate.h
- ios/Capacitor/Capacitor/CAPBridgeViewController+CDVScreenOrientationDelegate.m
- ios/Capacitor/Capacitor/JSExport.swift
- android-template/settings.gradle
- ios/Capacitor/Capacitor/CAPInstanceDescriptor.m
- android-template/app/build.gradle
- ios/Capacitor/CapacitorTests/ConfigurationTests.swift
- ios/Capacitor/Capacitor/TmpViewController.swift
- ios/Capacitor/Capacitor/CAPApplicationDelegateProxy.swift
- android/capacitor/proguard-rules.pro
RobinWitch
left a comment
There was a problem hiding this comment.
This upstream sync regresses the fork's package identity, which will break beta and release publishing for Capacitor Plus.
The PR changes all workspace package names from the fork namespace back to upstream names:
core/package.json @capacitor-plus/core -> @capacitor/core
cli/package.json @capacitor-plus/cli -> @capacitor/cli
android/package.json @capacitor-plus/android -> @capacitor/android
ios/package.json @capacitor-plus/ios -> @capacitor/ios
That is not just metadata. The PR beta publish workflow reads pkg.name from the selected workspace (.github/workflows/pr_beta_publish.yml lines 226-233 and 293), then runs npm publish in that package directory and adds a dist-tag using steps.version.outputs.package_name (lines 305-316). With these names, /publish-beta core would try to publish/tag @capacitor/core instead of @capacitor-plus/core, and the documented /publish-beta @capacitor-plus/core selector no longer matches any package name.
Please keep the @capacitor-plus/* package names while syncing the upstream code, otherwise the fork's beta publish path and downstream install commands point at the wrong npm namespace.
Rebuilt, but blocked: this is a Capacitor 9 changeWhat I did: the branch was cut from upstream Why it is still blocked: Upstream ionic-team/capacitor#8524 was merged into upstream's This belongs to the same bucket as #81: land it as part of a deliberate Posted by an AI agent (Cursor). |
…ework for Cordova
Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
cfe72f0 to
201427e
Compare
Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
|
@coderabbitai review |
|
Resolved 6 stale CodeRabbit review threads on files not in this PR’s current diff at HEAD
Current diff (3 files only): Cordova.framework removal in Those findings targeted the old upstream-preferred mega-merge, not this cherry-pick rebase. No code changes were made in the commented files — resolving threads only. Still needed before merge: CodeRabbit APPROVED on HEAD |
Merge Conflict Resolution Required
The sync of upstream PR ionic-team#8524 from @jcesarmobile encountered merge conflicts.
Original PR: ionic-team#8524
What happened
Synced from upstream by Capacitor+ Bot
Summary by CodeRabbit
New Features
run.Bug Fixes
Chores