chore: sync upstream PR #8521 - chore(cli): migrate changes for Capacitor 9#81
Open
riderx wants to merge 83 commits into
Open
chore: sync upstream PR #8521 - chore(cli): migrate changes for Capacitor 9#81riderx wants to merge 83 commits into
riderx wants to merge 83 commits into
Conversation
…m#8346) Co-authored-by: jwo <jonathan.worreth@protonmail.ch> Co-authored-by: Mark Anderson <mark.anderson@outsystems.com>
Co-authored-by: Steven Sherry <steven.r.sherry@gmail.com> Co-authored-by: jcesarmobile <jcesarmobile@gmail.com> Co-authored-by: Joseph Pender <joey.pender@outsystems.com>
Co-authored-by: jcesarmobile <jcesarmobile@gmail.com>
…preferred fallback)
Member
Author
|
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. |
📝 WalkthroughWalkthroughThis PR rebrands the project from capacitor-plus to ChangesPackage Rebrand and Release Metadata
Android Cordova Extraction and Bridge Refactor
iOS Cordova Extraction and Bridge Refactor
CLI Cordova Gating, Run/Live-Reload, and Telemetry
Build Tooling and SDK Version Upgrades
SystemBars Insets Handling and PluginConfig.getDouble
Minor Formatting and Logic Cleanups
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant WebView
participant WebViewDelegationHandler
participant CapacitorBridge
participant CordovaPlugin
CordovaPlugin->>CapacitorBridge: registerCallInterceptor("cordova", handler)
WebView->>WebViewDelegationHandler: postMessage({type:"cordova", service, action, ...})
WebViewDelegationHandler->>CapacitorBridge: lookup callInterceptors[type]
CapacitorBridge->>CordovaPlugin: invoke registered handler(payload)
CordovaPlugin->>CordovaPlugin: resolve plugin command via selector
CordovaPlugin-->>WebView: execute Cordova plugin command
sequenceDiagram
participant CLI
participant AndroidUpdate as android/update.ts
participant Gradle as capacitor.settings.gradle
participant Manifest as AndroidManifest
CLI->>AndroidUpdate: updateAndroid(config, plugins)
AndroidUpdate->>AndroidUpdate: enableCordova = cordovaPlugins.length > 0
alt enableCordova true
AndroidUpdate->>Gradle: include capacitor-cordova-android(-plugins)
AndroidUpdate->>Manifest: generate Cordova manifest entries
else enableCordova false
AndroidUpdate->>Gradle: skip Cordova subprojects
end
AndroidUpdate-->>CLI: writePluginsJson(includeCordova)
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
cli/src/tasks/run.ts (1)
101-123: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRestore the Android manifest when live-reload startup fails.
Line 104 enables the Android live-reload manifest state before
run(...), but Lines 121-123 only revertcapacitor.config.jsonon errors. Ifrun(...)fails, the manifest can remain with the temporary cleartext/live-reload changes.Proposed fix
+ let cordovaPlugins: Awaited<ReturnType<typeof getCordovaPlugins>> | undefined; + let androidManifestUpdated = false; try { if (options.sync) { await sync(config, platformName, false, true); } - const cordovaPlugins = await getCordovaPlugins(config, platformName); + cordovaPlugins = await getCordovaPlugins(config, platformName); if (options.url) { await CapLiveReloadHelper.editCapConfigForLiveReload(config, platformName, options); if (platformName === config.android.name) { - await await writeCordovaAndroidManifest(cordovaPlugins, config, platformName, true); + await writeCordovaAndroidManifest(cordovaPlugins, config, platformName, true); + androidManifestUpdated = true; } } await run(config, platformName, options); @@ } catch (e: any) { if (options.url) { await CapLiveReloadHelper.revertCapConfigForLiveReload(); + if (androidManifestUpdated && platformName === config.android.name && cordovaPlugins) { + await writeCordovaAndroidManifest(cordovaPlugins, config, platformName, false); + } }🤖 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/tasks/run.ts` around lines 101 - 123, The live-reload startup path in run() only reverts the Capacitor config on failure, leaving the temporary Android manifest changes applied if run(config, platformName, options) throws. Update the catch block in cli/src/tasks/run.ts to also restore the Android manifest via writeCordovaAndroidManifest(...) when platformName matches config.android.name and options.url is set, mirroring the cleanup already done in the SIGINT path.CHANGELOG.md (1)
6-116: 🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy liftUnresolved merge conflict: duplicate/conflicting changelog entries.
Same pattern as
android/CHANGELOG.md:9.0.0-alpha.4heading duplicated (Line 14 vs Line 37),9.0.0-alpha.3duplicated with conflicting compare ranges (Line 20 vs Line 49), the entire9.0.0-alpha.0block duplicated verbatim (Lines 20-35 vs 61-70), and8.4.0duplicated with conflicting compare links/content (Line 47 vs Line 72). As per coding guidelines, "Do not manually editCHANGELOG.md; it is managed automatically by CI/CD" — revert this manual reconciliation and let CI regenerate 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 `@CHANGELOG.md` around lines 6 - 116, The CHANGELOG contains unresolved merge-conflict duplication with repeated and conflicting release sections. Remove the manually reconciled duplicates in the changelog blocks for the repeated version headings and restore the file to the automatically generated state; use the existing version headings such as 9.0.0-alpha.4, 9.0.0-alpha.3, 9.0.0-alpha.0, and 8.4.0 to locate the conflicting entries, and do not hand-edit the changelog beyond reverting the manual merge.Source: Coding guidelines
core/CHANGELOG.md (1)
6-102: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRegenerate the changelog instead of keeping duplicate release blocks.
This file repeats the
9.0.0-alpha.4,8.4.0, and8.3.4sections, so the release history is no longer a single linear record. Please regenerate it from the release metadata source rather than hand-editing merged sections. As per coding guidelines,CHANGELOG.md: Do not manually editCHANGELOG.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 `@core/CHANGELOG.md` around lines 6 - 102, The changelog contains duplicated release sections, so restore a single linear history by regenerating core/CHANGELOG.md from the automated release metadata source instead of hand-editing it. Remove the repeated 9.0.0-alpha.4, 8.4.0, and 8.3.4 blocks and ensure the generated output matches the standard changelog flow used for `@capacitor/core`.Sources: Coding guidelines, Linters/SAST tools
ios/CHANGELOG.md (1)
6-102: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRegenerate the changelog instead of keeping duplicate release blocks.
This file repeats the
9.0.0-alpha.4,8.4.0, and8.3.4sections, so the release history is inconsistent. Please regenerate it from the release metadata source rather than hand-editing merged sections. As per coding guidelines,CHANGELOG.md: Do not manually editCHANGELOG.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 `@ios/CHANGELOG.md` around lines 6 - 102, The ios/CHANGELOG.md entry has duplicated release sections, so the history is inconsistent. Regenerate the changelog from the release metadata source instead of manually merging or editing sections, and ensure the duplicate blocks for the repeated versions are removed so the final output is a single, correctly ordered release history.Sources: Coding guidelines, Linters/SAST tools
android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java (1)
177-192: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUndocumented SDK-gated insets acquisition — clarify rationale.
The choice to call
ViewCompat.getRootWindowInsets(v)only onVANILLA_ICE_CREAM+and fall back toWindowInsetsCompat.CONSUMED(i.e., zero insets) on older devices means the initial safe-area CSS values will always be0on older Android until the window insets listener later fires with real values. This is a legitimate workaround ifgetRootWindowInsets()is unreliable pre-attach on older APIs, but the behavior is non-obvious and undocumented.Consider adding a short comment explaining why older SDKs intentionally skip the real insets query here, to avoid future maintainers "fixing" this into a regression.
🤖 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 177 - 192, The SDK-gated insets lookup in SystemBars is intentionally falling back to zero initial safe-area values on older Android, but the rationale is not documented. Add a short inline comment near the Build.VERSION.SDK_INT branch in SystemBars’s insets handling to explain why ViewCompat.getRootWindowInsets(v) is only used on VANILLA_ICE_CREAM+ and why older devices deliberately use WindowInsetsCompat.CONSUMED until the listener updates the CSS.
🤖 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 @.github/workflows/test.yml:
- Around line 129-131: The CI workflow is missing the native podspec validation
step, so restore the `Validate native podspec` job in the test workflow or add
an equivalent explicit lint step elsewhere. Use the existing `test-android`
workflow section to reintroduce the `sh ./scripts/native-podspec.sh lint` check
so both `Capacitor.podspec` and `CapacitorCordova.podspec` are validated again,
since `ios/package.json` verify only covers Xcode builds.
In `@android-template/gradle/wrapper/gradle-wrapper.properties`:
- Around line 3-6: The Gradle wrapper settings in gradle-wrapper.properties
still leave wrapper-download retries disabled, which can make CI more fragile
during distribution fetches. Update the wrapper configuration by changing the
existing retries setting to a non-zero value and keep the current backoff tuning
consistent with it. Use the Gradle wrapper properties keys already present in
this file, especially retries and retryBackOffMs, so the download behavior is
resilient without changing the distribution URL or timeout.
In
`@android/capacitor-cordova/src/main/java/com/getcapacitor/cordova/CordovaPlugin.java`:
- Around line 69-74: Persist the Cordova callback bundle in CordovaPlugin by
overriding saveInstanceState() alongside restoreState(Bundle), since
restoreState currently only restores whatever Plugin.saveInstanceState()
provides. Update the CordovaPlugin class to store the cordovaInterface state
into the Bundle during saving, then read it back in restoreState before calling
cordovaInterface.restoreInstanceState(state), so activity-result callbacks
survive process death.
In `@android/capacitor/src/main/java/com/getcapacitor/Bridge.java`:
- Around line 1512-1557: The core interceptors are being attached too late: new
Bridge(...) starts WebView loading before "message" and "js.error" are
registered, so early bridge messages can be ignored. Move the
bridge.registerInterceptor(...) setup for both handlers to happen before any
WebView loading starts inside Bridge initialization, keeping the existing
behavior in the Bridge constructor and MessageHandler flow intact.
- Around line 293-307: The WebView version check in
Bridge.isMinimumWebViewInstalled currently assumes
WebView.getCurrentWebViewPackage() and its versionName are always present, which
can crash during startup. Add a null guard at the start of
isMinimumWebViewInstalled to return false when the PackageInfo or versionName is
missing, then keep the existing parsing and min-version comparison logic
unchanged for the valid case.
In `@android/capacitor/src/test/java/com/getcapacitor/plugin/SystemBarsTest.java`:
- Around line 22-47: The current tests cover only the show path in
SystemBarsTest, but the refactored hide branch in SystemBars is not exercised.
Update invokeSetHidden to accept a hide boolean so tests can call the same logic
for both paths, then add hide-path tests that verify the correct
WindowInsetsControllerCompat.hide calls for empty, StatusBar, and NavigationBar
inputs while keeping the existing show-path assertions intact.
In `@android/CHANGELOG.md`:
- Around line 6-84: The changelog edit has unresolved merge-conflict duplication
with repeated version headings and conflicting compare links in
android/CHANGELOG.md. Revert the manual changes to this file rather than trying
to reconcile entries by hand, since it is CI/CD-managed. Use the duplicated
release blocks like the repeated 9.0.0-alpha.4, 9.0.0-alpha.3, 9.0.0-alpha.0,
8.4.0, and 8.3.4 sections as the indicators that this should be reset to the
auto-generated version.
In `@cli/CHANGELOG.md`:
- Around line 6-103: The changelog contains duplicated release sections for the
same versions, which makes the history inconsistent and can trigger lint
failures. Regenerate the entire cli/CHANGELOG.md from the release metadata
source instead of editing it manually, and ensure the duplicate blocks for
versions like 9.0.0-alpha.4, 8.4.0, and 8.3.4 are removed. Keep the file aligned
with the automated changelog process used by CI/CD.
In `@cli/src/ios/update.ts`:
- Around line 95-99: The incompatible Cordova plugin warning is currently gated
by enableCordova inside updateIos behavior, so it never runs when only
incompatible Cordova plugins are installed. Move the incompatible plugin
detection and printPlugins call out of the enableCordova block in the update
flow, while keeping logCordovaManualSteps and other Cordova-only steps inside
it. Use the existing getPluginType, PluginType.Incompatible, and printPlugins
symbols in cli/src/ios/update.ts to place the warning in the right spot.
In `@cli/src/tasks/migrate.ts`:
- Around line 125-127: The migration confirmation text in migrate and the
logPrompt message is mixing Android platform version wording with SDK/minVersion
details. Update the prompt shown around migrateconfirm to use consistent Android
terminology that matches the config default of Android minVersion 26, and avoid
mentioning “Android 17 (SDK 37)” in the confirmation copy. Keep the iOS version
reference, but make the Android wording clearly refer to the migration target
defined by the migration flow rather than target SDK or platform version.
- Around line 92-95: The version gate in migrate.ts is too loose because
`capMajor < 8` still allows Capacitor 9+ and parse failures (`NaN`) to continue
into a destructive migration. Update the `checkCapacitorMajorVersion` check in
`migrate` to require exactly Capacitor 8 before any project file mutations, and
keep the `fatal` path for anything other than 8, including invalid or missing
version parses.
In `@cli/src/telemetry.ts`:
- Around line 85-90: The debug log in telemetry currently prints the full `data`
payload, which can expose sensitive `cmd.opts()` values through `data.options`.
Update `cli/src/telemetry.ts` so the `debug('metric payload: %O', ...)` call
logs a redacted/sanitized copy of the payload instead of the raw `data` object.
Use the `data` assembly area and the `debug` statement to locate the change, and
ensure secrets like keystore and signing credentials are masked before logging.
In `@cli/src/util/cordova-ios.ts`:
- Around line 22-27: The Cordova podspec/package generation flow is starting
asynchronous file writes and returning early, so consumers can continue before
the files exist. Update generateCordovaPodspecs and the underlying
generateCordovaPodspec helper(s) to await all async writes (for example by
collecting the per-plugin write promises and awaiting them before returning) so
pod install and SPM generation only proceed after the files are fully written.
- Around line 271-278: Escape the preference name before constructing the RegExp
in replaceFrameworkVariables, since only the leading $ is currently handled and
names with other regex metacharacters can break matching or replace the wrong
text. Update the replacement logic in replaceFrameworkVariables so it safely
escapes the full preference.$.name before passing it to new RegExp, while
keeping the existing default substitution behavior unchanged.
In `@ios/Capacitor/Capacitor/CAPInstanceDescriptor.swift`:
- Around line 144-147: The public `cordovaDeployDisabled` value in
`InstanceDescriptor` is hardcoded and no longer reflects the configuration
contract exposed through `CAPInstanceConfiguration`. Update the
`InstanceDescriptor` property and its initialization flow so the flag is still
parsed and propagated from the descriptor, or remove/deprecate the corresponding
public configuration surface consistently. Use the `CAPInstanceDescriptor` and
`CAPInstanceConfiguration` symbols to trace the property through the
descriptor-to-configuration path and keep the contract aligned.
In `@ios/Capacitor/Capacitor/JS.swift`:
- Around line 17-22: The JSCall type is unnecessarily public even though it is
only used by bridge internals. Change JSCall in JS.swift to internal unless you
explicitly intend to expose it as part of the public API, and keep its members
aligned with that visibility. If external access is not required, reduce the
type’s access level so the bridge implementation remains encapsulated.
In `@ios/CapacitorCordova/CapacitorCordova/Classes/Public/Plugin.swift`:
- Line 43: The Cordova args cast in the plugin argument parsing is too generic;
update the `actionArgs` extraction in `Plugin.swift` to use a concrete `[Any]`
type instead of `Array`, keeping the rest of the `dict["actionArgs"]` handling
unchanged. This applies to the `args` assignment inside the `Plugin` code path
and should preserve the existing default empty array behavior.
In `@lerna.json`:
- Line 7: The release branch guard in lerna.json is too restrictive because
allowBranch only permits next, which blocks the automated versioning flow on
plus. Update the Lerna configuration so the allowBranch setting includes plus
alongside next, keeping npx lerna version compatible with the bump_version
workflow.
---
Outside diff comments:
In `@android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java`:
- Around line 177-192: The SDK-gated insets lookup in SystemBars is
intentionally falling back to zero initial safe-area values on older Android,
but the rationale is not documented. Add a short inline comment near the
Build.VERSION.SDK_INT branch in SystemBars’s insets handling to explain why
ViewCompat.getRootWindowInsets(v) is only used on VANILLA_ICE_CREAM+ and why
older devices deliberately use WindowInsetsCompat.CONSUMED until the listener
updates the CSS.
In `@CHANGELOG.md`:
- Around line 6-116: The CHANGELOG contains unresolved merge-conflict
duplication with repeated and conflicting release sections. Remove the manually
reconciled duplicates in the changelog blocks for the repeated version headings
and restore the file to the automatically generated state; use the existing
version headings such as 9.0.0-alpha.4, 9.0.0-alpha.3, 9.0.0-alpha.0, and 8.4.0
to locate the conflicting entries, and do not hand-edit the changelog beyond
reverting the manual merge.
In `@cli/src/tasks/run.ts`:
- Around line 101-123: The live-reload startup path in run() only reverts the
Capacitor config on failure, leaving the temporary Android manifest changes
applied if run(config, platformName, options) throws. Update the catch block in
cli/src/tasks/run.ts to also restore the Android manifest via
writeCordovaAndroidManifest(...) when platformName matches config.android.name
and options.url is set, mirroring the cleanup already done in the SIGINT path.
In `@core/CHANGELOG.md`:
- Around line 6-102: The changelog contains duplicated release sections, so
restore a single linear history by regenerating core/CHANGELOG.md from the
automated release metadata source instead of hand-editing it. Remove the
repeated 9.0.0-alpha.4, 8.4.0, and 8.3.4 blocks and ensure the generated output
matches the standard changelog flow used for `@capacitor/core`.
In `@ios/CHANGELOG.md`:
- Around line 6-102: The ios/CHANGELOG.md entry has duplicated release sections,
so the history is inconsistent. Regenerate the changelog from the release
metadata source instead of manually merging or editing sections, and ensure the
duplicate blocks for the repeated versions are removed so the final output is a
single, correctly ordered release history.
🪄 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: 636475cf-e9e4-42b1-b588-2d979ddfb583
⛔ 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/CapacitorTests/ConfigurationTests.swift
- ios/Capacitor/Capacitor/TmpViewController.swift
- ios/Capacitor/Capacitor/JSExport.swift
- ios/Capacitor/Capacitor/CAPApplicationDelegateProxy.swift
- ios/Capacitor/Capacitor/CAPBridgeViewController+CDVScreenOrientationDelegate.m
- ios/Capacitor/Capacitor/CAPBridgeViewController+CDVScreenOrientationDelegate.h
- android-template/settings.gradle
- ios/Capacitor/Capacitor/CAPInstanceDescriptor.m
- android/capacitor/proguard-rules.pro
- android-template/app/build.gradle
Comment on lines
+129
to
131
| # - name: Validate native podspec | ||
| # run: sh ./scripts/native-podspec.sh lint | ||
| test-android: |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== workflow snippet ==\n'
sed -n '110,145p' .github/workflows/test.yml
printf '\n== podspec lint script ==\n'
fd -a 'native-podspec.sh|podspec' scripts ios .github -t f 2>/dev/null || true
printf '\n== references to native-podspec ==\n'
rg -n "native-podspec|podspec lint|Validate native podspec|CapacitorCordova.podspec|Capacitor.podspec" .github scripts ios -SRepository: Cap-go/capacitor-plus
Length of output: 2134
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== scripts/native-podspec.sh ==\n'
cat -n scripts/native-podspec.sh
printf '\n== ios/package.json ==\n'
cat -n ios/package.json
printf '\n== any podspec-related scripts in package.json files ==\n'
rg -n '"verify"|podspec|lint' package.json ios/package.json . -g 'package.json' -SRepository: Cap-go/capacitor-plus
Length of output: 4208
Restore podspec lint in CI — commenting out Validate native podspec removes checks for both ios/Capacitor.podspec and ios/CapacitorCordova.podspec; ios/package.json’s verify script only runs Xcode builds, so add this step back or replace it with an explicit lint elsewhere.
🧰 Tools
🪛 zizmor (1.26.1)
[info] 131-131: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
🤖 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 @.github/workflows/test.yml around lines 129 - 131, The CI workflow is
missing the native podspec validation step, so restore the `Validate native
podspec` job in the test workflow or add an equivalent explicit lint step
elsewhere. Use the existing `test-android` workflow section to reintroduce the
`sh ./scripts/native-podspec.sh lint` check so both `Capacitor.podspec` and
`CapacitorCordova.podspec` are validated again, since `ios/package.json` verify
only covers Xcode builds.
Comment on lines
+3
to
+6
| distributionUrl=https\://services.gradle.org/distributions/gradle-9.5.1-all.zip | ||
| networkTimeout=10000 | ||
| retries=0 | ||
| retryBackOffMs=500 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Retries left at the Gradle-generated default (disabled) — consider enabling for CI resilience.
retries=0 is Gradle's stock default (retrying is disabled by default), so this isn't a misconfiguration. Given this same PR is bumping multiple CI job timeouts (likely due to observed flakiness), enabling wrapper-download retries could reduce transient network failures during distribution download in CI.
♻️ Enable wrapper download retries
networkTimeout=10000
-retries=0
+retries=3
retryBackOffMs=500📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| distributionUrl=https\://services.gradle.org/distributions/gradle-9.5.1-all.zip | |
| networkTimeout=10000 | |
| retries=0 | |
| retryBackOffMs=500 | |
| distributionUrl=https\://services.gradle.org/distributions/gradle-9.5.1-all.zip | |
| networkTimeout=10000 | |
| retries=3 | |
| retryBackOffMs=500 |
🤖 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-template/gradle/wrapper/gradle-wrapper.properties` around lines 3 -
6, The Gradle wrapper settings in gradle-wrapper.properties still leave
wrapper-download retries disabled, which can make CI more fragile during
distribution fetches. Update the wrapper configuration by changing the existing
retries setting to a non-zero value and keep the current backoff tuning
consistent with it. Use the Gradle wrapper properties keys already present in
this file, especially retries and retryBackOffMs, so the download behavior is
resilient without changing the distribution URL or timeout.
Comment on lines
+69
to
+74
| @Override | ||
| protected void restoreState(Bundle state) { | ||
| if (state != null) { | ||
| cordovaInterface.restoreInstanceState(state); | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check base Plugin contract for saveInstanceState/restoreState pairing.
rg -n "saveInstanceState|restoreState" android/capacitor/src/main/java/com/getcapacitor/Plugin.java android/capacitor/src/main/java/com/getcapacitor/Bridge.javaRepository: Cap-go/capacitor-plus
Length of output: 1047
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the CordovaPlugin class and the base Plugin lifecycle methods.
sed -n '1,180p' android/capacitor-cordova/src/main/java/com/getcapacitor/cordova/CordovaPlugin.java
printf '\n--- Plugin lifecycle ---\n'
sed -n '900,970p' android/capacitor/src/main/java/com/getcapacitor/Plugin.java
printf '\n--- Bridge restore/save paths ---\n'
sed -n '1010,1055p' android/capacitor/src/main/java/com/getcapacitor/Bridge.java
sed -n '1510,1535p' android/capacitor/src/main/java/com/getcapacitor/Bridge.javaRepository: Cap-go/capacitor-plus
Length of output: 10818
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find any saveInstanceState override in the Cordova plugin package.
rg -n "saveInstanceState\(" android/capacitor-cordova/src/main/java/com/getcapacitor/cordovaRepository: Cap-go/capacitor-plus
Length of output: 159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the Cordova plugin class and lifecycle hooks directly.
sed -n '1,140p' android/capacitor-cordova/src/main/java/com/getcapacitor/cordova/CordovaPlugin.java
printf '\n---\n'
sed -n '900,960p' android/capacitor/src/main/java/com/getcapacitor/Plugin.java
printf '\n---\n'
sed -n '1018,1048p' android/capacitor/src/main/java/com/getcapacitor/Bridge.java
printf '\n---\n'
sed -n '1518,1530p' android/capacitor/src/main/java/com/getcapacitor/Bridge.javaRepository: Cap-go/capacitor-plus
Length of output: 9291
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether CordovaPlugin defines a saveInstanceState override anywhere in the file.
rg -n "saveInstanceState|restoreState" android/capacitor-cordova/src/main/java/com/getcapacitor/cordova/CordovaPlugin.javaRepository: Cap-go/capacitor-plus
Length of output: 210
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the Cordova interface implementation for any saved-state handling.
rg -n "class MockCordovaInterfaceImpl|restoreInstanceState|saveInstanceState|getActivityResultCallback|onActivityResult" android/capacitor-cordova/src/main/javaRepository: Cap-go/capacitor-plus
Length of output: 1007
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the Cordova interface implementation if present.
fd -a "MockCordovaInterfaceImpl.java" android/capacitor-cordova/src/main/javaRepository: Cap-go/capacitor-plus
Length of output: 273
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the Cordova plugin package and inspect any related implementation files.
find android/capacitor-cordova/src/main/java -type f | sortRepository: Cap-go/capacitor-plus
Length of output: 531
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the mock Cordova interface implementation and any saved-state methods.
fd -a "MockCordovaInterfaceImpl.java|MockCordovaWebViewImpl.java" android/capacitor-cordova/src/main/javaRepository: Cap-go/capacitor-plus
Length of output: 385
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the implementation of restoreInstanceState to see what bundle shape it expects.
rg -n "restoreInstanceState\\(" -g'*.java' -g'*.kt' androidRepository: Cap-go/capacitor-plus
Length of output: 561
Persist the Cordova callback bundle here
restoreState(Bundle) only gets the base Plugin.saveInstanceState() payload unless this plugin overrides saveInstanceState() to persist cordovaInterface state, so Cordova activity-result callbacks can be lost after process death.
🤖 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-cordova/src/main/java/com/getcapacitor/cordova/CordovaPlugin.java`
around lines 69 - 74, Persist the Cordova callback bundle in CordovaPlugin by
overriding saveInstanceState() alongside restoreState(Bundle), since
restoreState currently only restores whatever Plugin.saveInstanceState()
provides. Update the CordovaPlugin class to store the cordovaInterface state
into the Bundle during saving, then read it back in restoreState before calling
cordovaInterface.restoreInstanceState(state), so activity-result callbacks
survive process death.
Comment on lines
293
to
307
| @SuppressLint("WebViewApiAvailability") | ||
| public boolean isMinimumWebViewInstalled() { | ||
| PackageManager pm = getContext().getPackageManager(); | ||
|
|
||
| // Check getCurrentWebViewPackage() directly if above Android 8 | ||
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { | ||
| PackageInfo info = WebView.getCurrentWebViewPackage(); | ||
| Pattern pattern = Pattern.compile("(\\d+)"); | ||
| Matcher matcher = pattern.matcher(info.versionName); | ||
| if (matcher.find()) { | ||
| String majorVersionStr = matcher.group(0); | ||
| int majorVersion = Integer.parseInt(majorVersionStr); | ||
| if (info.packageName.equals("com.huawei.webview")) { | ||
| return majorVersion >= config.getMinHuaweiWebViewVersion(); | ||
| } | ||
| return majorVersion >= config.getMinWebViewVersion(); | ||
| } else { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| // Otherwise manually check WebView versions | ||
| try { | ||
| PackageInfo info = InternalUtils.getPackageInfo(pm, "com.android.chrome"); | ||
| String majorVersionStr = info.versionName.split("\\.")[0]; | ||
| int majorVersion = Integer.parseInt(majorVersionStr); | ||
| return majorVersion >= config.getMinWebViewVersion(); | ||
| } catch (Exception ex) { | ||
| Logger.warn("Unable to get package info for 'com.google.android.webview'" + ex.toString()); | ||
| } | ||
|
|
||
| try { | ||
| PackageInfo info = InternalUtils.getPackageInfo(pm, "com.android.webview"); | ||
| String majorVersionStr = info.versionName.split("\\.")[0]; | ||
| PackageInfo info = WebView.getCurrentWebViewPackage(); | ||
| Pattern pattern = Pattern.compile("(\\d+)"); | ||
| Matcher matcher = pattern.matcher(info.versionName); | ||
| if (matcher.find()) { | ||
| String majorVersionStr = matcher.group(0); | ||
| int majorVersion = Integer.parseInt(majorVersionStr); | ||
| if (info.packageName.equals("com.huawei.webview")) { | ||
| return majorVersion >= config.getMinHuaweiWebViewVersion(); | ||
| } | ||
| return majorVersion >= config.getMinWebViewVersion(); | ||
| } catch (Exception ex) { | ||
| Logger.warn("Unable to get package info for 'com.android.webview'" + ex.toString()); | ||
| } | ||
|
|
||
| final int amazonFireMajorWebViewVersion = extractWebViewMajorVersion(pm, "com.amazon.webview.chromium"); | ||
| if (amazonFireMajorWebViewVersion >= config.getMinWebViewVersion()) { | ||
| return true; | ||
| } | ||
|
|
||
| // Could not detect any webview, return false | ||
| return false; | ||
| } | ||
|
|
||
| private int extractWebViewMajorVersion(final PackageManager pm, final String webViewPackageName) { | ||
| try { | ||
| final PackageInfo info = InternalUtils.getPackageInfo(pm, webViewPackageName); | ||
| final String majorVersionStr = info.versionName.split("\\.")[0]; | ||
| final int majorVersion = Integer.parseInt(majorVersionStr); | ||
| return majorVersion; | ||
| } catch (Exception ex) { | ||
| Logger.warn(String.format("Unable to get package info for '%s' with err '%s'", webViewPackageName, ex)); | ||
| } else { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## Bridge.java around isMinimumWebViewInstalled()\n'
sed -n '260,330p' android/capacitor/src/main/java/com/getcapacitor/Bridge.java
printf '\n## Search for getCurrentWebViewPackage() and versionName parsing\n'
rg -n "getCurrentWebViewPackage|versionName|isMinimumWebViewInstalled" android/capacitor/src/main/java -S
printf '\n## Search for any existing null guards around WebView package info\n'
rg -n "PackageInfo info = WebView.getCurrentWebViewPackage|info == null|versionName == null" android/capacitor/src/main/java -SRepository: Cap-go/capacitor-plus
Length of output: 4627
🌐 Web query:
Android WebView.getCurrentWebViewPackage null return PackageInfo.versionName null API contract
💡 Result:
The method WebView.getCurrentWebViewPackage returns a PackageInfo object describing the package currently providing WebView functionality [1]. According to the Android API contract, this method can return null [2][3]. Common reasons for a null return value include: - Device Configuration: The device may be set up in a way that does not support WebView, such as certain Wear OS devices or specific stripped-down system images where no WebView provider is installed or enabled [2]. - API Level Compatibility: The API was introduced in Android 8.0 (API level 26) [4][5]. While it is available in later versions, calling it on devices below this level (without compatibility handling) will not function as intended via the framework method [3]. Regarding PackageInfo.versionName returning null: Even when WebView.getCurrentWebViewPackage returns a non-null PackageInfo object, the versionName field within that object can be null [6]. This occurs because the versionName is a string field declared in the AndroidManifest.xml of the WebView provider package. If the provider package developer does not explicitly define a versionName in their manifest, or if the system fails to retrieve it, the field remains null. Best Practices: - Always null-check the result of WebView.getCurrentWebViewPackage [2]. - If you need to access WebView package information in a backward-compatible and safer way, use WebViewCompat.getCurrentWebViewPackage(Context) from the AndroidX Webkit library [7]. This library handles internal complexities across different Android API levels [3][8]. - When accessing fields like versionName, perform defensive coding to handle potential null values to avoid NullPointerExceptions [6].
Citations:
- 1: https://developer.android.com/reference/android/webkit/WebView
- 2: https://developer.android.com/develop/ui/views/layout/webapps/managing-webview
- 3: https://android.googlesource.com/platform/prebuilts/fullsdk/sources/android-28/+/refs/heads/androidx-lifecycle-release/androidx/webkit/WebViewCompat.java
- 4: https://stackoverflow.com/questions/47577525/get-webview-implementation-programmatically
- 5: https://stackoverflow.com/questions/41234678/determine-webview-implementation-system-webview-or-chrome
- 6: Captcha screen does not disappear completely plaid/plaid-link-android#80
- 7: https://developer.android.com/develop/ui/views/layout/webapps/jetpack-webkit-overview
- 8: https://android.googlesource.com/platform/frameworks/support/+/23d37c132dada3f0483de46c500550979b455aea
Guard WebView package info before parsing
WebView.getCurrentWebViewPackage() and info.versionName can both be null here; return false before parsing so startup falls back to the error page instead of crashing.
🤖 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/Bridge.java` around lines
293 - 307, The WebView version check in Bridge.isMinimumWebViewInstalled
currently assumes WebView.getCurrentWebViewPackage() and its versionName are
always present, which can crash during startup. Add a null guard at the start of
isMinimumWebViewInstalled to return false when the PackageInfo or versionName is
missing, then keep the existing parsing and min-version comparison logic
unchanged for the valid case.
Comment on lines
+1512
to
1557
| Bridge bridge = new Bridge(activity, serverPath, fragment, webView, plugins, pluginInstances, config); | ||
|
|
||
| if (webView instanceof CapacitorWebView capacitorWebView) { | ||
| capacitorWebView.setBridge(bridge); | ||
| } | ||
|
|
||
| bridge.setCordovaWebView(mockWebView); | ||
| bridge.setWebViewListeners(webViewListeners); | ||
| bridge.setRouteProcessor(routeProcessor); | ||
|
|
||
| if (instanceState != null) { | ||
| PluginHandle maybeCordova = bridge.getPlugin("__CordovaPlugin"); | ||
| if (maybeCordova != null) { | ||
| maybeCordova.getInstance().restoreState(instanceState); | ||
| } | ||
| bridge.restoreInstanceState(instanceState); | ||
| } | ||
|
|
||
| bridge.registerInterceptor("message", (postData) -> { | ||
| try { | ||
| String callbackId = postData.getString("callbackId"); | ||
| String pluginId = postData.getString("pluginId"); | ||
| String methodName = postData.getString("methodName"); | ||
| JSObject methodData = postData.getJSObject("options", new JSObject()); | ||
|
|
||
| Logger.verbose( | ||
| Logger.tags("Plugin"), | ||
| "To native (Capacitor plugin): callbackId: " + | ||
| callbackId + | ||
| ", pluginId: " + | ||
| pluginId + | ||
| ", methodName: " + | ||
| methodName | ||
| ); | ||
|
|
||
| PluginCall call = new PluginCall(bridge.msgHandler, pluginId, callbackId, methodName, methodData); | ||
| bridge.callPluginMethod(pluginId, methodName, call); | ||
| } catch (JSONException e) { | ||
| Logger.error(e.getMessage()); | ||
| } | ||
| }); | ||
|
|
||
| bridge.registerInterceptor("js.error", (postData) -> { | ||
| Logger.error("JavaScript Error: " + postData.toString()); | ||
| }); | ||
|
|
||
| return bridge; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Register core interceptors before starting WebView loading.
new Bridge(...) starts loading the WebView from the constructor, but "message" and "js.error" interceptors are registered only afterward. Early JS bridge calls can be dropped because MessageHandler.postMessage() ignores messages with no registered interceptor.
Proposed fix
- this.loadWebView();
} bridge.registerInterceptor("js.error", (postData) -> {
Logger.error("JavaScript Error: " + postData.toString());
});
+ bridge.loadWebView();
return bridge;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Bridge bridge = new Bridge(activity, serverPath, fragment, webView, plugins, pluginInstances, config); | |
| if (webView instanceof CapacitorWebView capacitorWebView) { | |
| capacitorWebView.setBridge(bridge); | |
| } | |
| bridge.setCordovaWebView(mockWebView); | |
| bridge.setWebViewListeners(webViewListeners); | |
| bridge.setRouteProcessor(routeProcessor); | |
| if (instanceState != null) { | |
| PluginHandle maybeCordova = bridge.getPlugin("__CordovaPlugin"); | |
| if (maybeCordova != null) { | |
| maybeCordova.getInstance().restoreState(instanceState); | |
| } | |
| bridge.restoreInstanceState(instanceState); | |
| } | |
| bridge.registerInterceptor("message", (postData) -> { | |
| try { | |
| String callbackId = postData.getString("callbackId"); | |
| String pluginId = postData.getString("pluginId"); | |
| String methodName = postData.getString("methodName"); | |
| JSObject methodData = postData.getJSObject("options", new JSObject()); | |
| Logger.verbose( | |
| Logger.tags("Plugin"), | |
| "To native (Capacitor plugin): callbackId: " + | |
| callbackId + | |
| ", pluginId: " + | |
| pluginId + | |
| ", methodName: " + | |
| methodName | |
| ); | |
| PluginCall call = new PluginCall(bridge.msgHandler, pluginId, callbackId, methodName, methodData); | |
| bridge.callPluginMethod(pluginId, methodName, call); | |
| } catch (JSONException e) { | |
| Logger.error(e.getMessage()); | |
| } | |
| }); | |
| bridge.registerInterceptor("js.error", (postData) -> { | |
| Logger.error("JavaScript Error: " + postData.toString()); | |
| }); | |
| return bridge; | |
| Bridge bridge = new Bridge(activity, serverPath, fragment, webView, plugins, pluginInstances, config); | |
| if (webView instanceof CapacitorWebView capacitorWebView) { | |
| capacitorWebView.setBridge(bridge); | |
| } | |
| bridge.setWebViewListeners(webViewListeners); | |
| bridge.setRouteProcessor(routeProcessor); | |
| if (instanceState != null) { | |
| PluginHandle maybeCordova = bridge.getPlugin("__CordovaPlugin"); | |
| if (maybeCordova != null) { | |
| maybeCordova.getInstance().restoreState(instanceState); | |
| } | |
| bridge.restoreInstanceState(instanceState); | |
| } | |
| bridge.registerInterceptor("message", (postData) -> { | |
| try { | |
| String callbackId = postData.getString("callbackId"); | |
| String pluginId = postData.getString("pluginId"); | |
| String methodName = postData.getString("methodName"); | |
| JSObject methodData = postData.getJSObject("options", new JSObject()); | |
| Logger.verbose( | |
| Logger.tags("Plugin"), | |
| "To native (Capacitor plugin): callbackId: " + | |
| callbackId + | |
| ", pluginId: " + | |
| pluginId + | |
| ", methodName: " + | |
| methodName | |
| ); | |
| PluginCall call = new PluginCall(bridge.msgHandler, pluginId, callbackId, methodName, methodData); | |
| bridge.callPluginMethod(pluginId, methodName, call); | |
| } catch (JSONException e) { | |
| Logger.error(e.getMessage()); | |
| } | |
| }); | |
| bridge.registerInterceptor("js.error", (postData) -> { | |
| Logger.error("JavaScript Error: " + postData.toString()); | |
| }); | |
| bridge.loadWebView(); | |
| return bridge; |
🤖 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/Bridge.java` around lines
1512 - 1557, The core interceptors are being attached too late: new Bridge(...)
starts WebView loading before "message" and "js.error" are registered, so early
bridge messages can be ignored. Move the bridge.registerInterceptor(...) setup
for both handlers to happen before any WebView loading starts inside Bridge
initialization, keeping the existing behavior in the Bridge constructor and
MessageHandler flow intact.
Comment on lines
+271
to
+278
| async function replaceFrameworkVariables(config: Config, prefsArray: any[], frameworkString: string): Promise<string> { | ||
| prefsArray.map((preference: any) => { | ||
| frameworkString = frameworkString.replace( | ||
| new RegExp(('$' + preference.$.name).replace('$', '\\$&'), 'g'), | ||
| preference.$.default, | ||
| ); | ||
| }); | ||
| return frameworkString; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Escape preference names before building the replacement RegExp.
Only $ is escaped today; preference names containing regex metacharacters can match the wrong framework text or throw.
Proposed fix
+function escapeRegExp(value: string): string {
+ return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+}
+
async function replaceFrameworkVariables(config: Config, prefsArray: any[], frameworkString: string): Promise<string> {
prefsArray.map((preference: any) => {
frameworkString = frameworkString.replace(
- new RegExp(('$' + preference.$.name).replace('$', '\\$&'), 'g'),
+ new RegExp(escapeRegExp(`$${preference.$.name}`), 'g'),
preference.$.default,
);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function replaceFrameworkVariables(config: Config, prefsArray: any[], frameworkString: string): Promise<string> { | |
| prefsArray.map((preference: any) => { | |
| frameworkString = frameworkString.replace( | |
| new RegExp(('$' + preference.$.name).replace('$', '\\$&'), 'g'), | |
| preference.$.default, | |
| ); | |
| }); | |
| return frameworkString; | |
| function escapeRegExp(value: string): string { | |
| return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); | |
| } | |
| async function replaceFrameworkVariables(config: Config, prefsArray: any[], frameworkString: string): Promise<string> { | |
| prefsArray.map((preference: any) => { | |
| frameworkString = frameworkString.replace( | |
| new RegExp(escapeRegExp(`$${preference.$.name}`), 'g'), | |
| preference.$.default, | |
| ); | |
| }); | |
| return frameworkString; | |
| } |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 273-273: Do not use variable for regular expressions
Context: new RegExp(('$' + preference.$.name).replace('$', '\$&'), 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
🤖 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/util/cordova-ios.ts` around lines 271 - 278, Escape the preference
name before constructing the RegExp in replaceFrameworkVariables, since only the
leading $ is currently handled and names with other regex metacharacters can
break matching or replace the wrong text. Update the replacement logic in
replaceFrameworkVariables so it safely escapes the full preference.$.name before
passing it to new RegExp, while keeping the existing default substitution
behavior unchanged.
Source: Linters/SAST tools
Comment on lines
144
to
147
| extension InstanceDescriptor { | ||
| @objc public var cordovaDeployDisabled: Bool { | ||
| return (cordovaConfiguration.settings?["DisableDeploy".lowercased()] as? NSString)?.boolValue ?? false | ||
| return false | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Don’t leave cordovaDeployDisabled as a stale public value.
CAPInstanceConfiguration still exposes and initializes this property from the descriptor, so hardcoding false breaks callers that read the configuration contract. Either preserve parsing of this flag or remove/deprecate the public configuration property as part of the Cordova extraction.
🤖 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 `@ios/Capacitor/Capacitor/CAPInstanceDescriptor.swift` around lines 144 - 147,
The public `cordovaDeployDisabled` value in `InstanceDescriptor` is hardcoded
and no longer reflects the configuration contract exposed through
`CAPInstanceConfiguration`. Update the `InstanceDescriptor` property and its
initialization flow so the flag is still parsed and propagated from the
descriptor, or remove/deprecate the corresponding public configuration surface
consistently. Use the `CAPInstanceDescriptor` and `CAPInstanceConfiguration`
symbols to trace the property through the descriptor-to-configuration path and
keep the contract aligned.
Comment on lines
+17
to
22
| public struct JSCall { | ||
| public let options: [String: Any] | ||
| public let pluginId: String | ||
| public let method: String | ||
| public let callbackId: String | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if any code outside Capacitor/Capacitor constructs JSCall directly.
rg -n 'JSCall\(' --type=swiftRepository: Cap-go/capacitor-plus
Length of output: 159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n '\bJSCall\b' ios . --type swift --type objc --type h || true
printf '\n--- JS.swift ---\n'
cat -n ios/Capacitor/Capacitor/JS.swift | sed -n '1,120p'Repository: Cap-go/capacitor-plus
Length of output: 6398
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'CapacitorCordova|JSCall|handleJSCall|public struct JSCall|public init\(' ios . --type swift --type xcodeproj --type plist --type text || trueRepository: Cap-go/capacitor-plus
Length of output: 193
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '430,470p' ios/Capacitor/Capacitor/CapacitorBridge.swift | cat -n
printf '\n--- JS.swift refs ---\n'
sed -n '1,140p' ios/Capacitor/Capacitor/JS.swift | cat -nRepository: Cap-go/capacitor-plus
Length of output: 6902
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- CapacitorBridge.swift (around handleJSCall) ---'
sed -n '440,465p' ios/Capacitor/Capacitor/CapacitorBridge.swift | cat -n
printf '\n%s\n' '--- public declarations mentioning JSCall or bridge entrypoints ---'
rg -n 'public .*JSCall|public .*handleJSCall|handleJSCall\(' ios/Capacitor/Capacitor --type swift || trueRepository: Cap-go/capacitor-plus
Length of output: 1953
Make JSCall internal unless you plan to expose it as API. The type is only used by bridge internals, and public still leaves the synthesized initializer non-public, so this widens the surface without giving outside callers a way to create values.
🤖 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 `@ios/Capacitor/Capacitor/JS.swift` around lines 17 - 22, The JSCall type is
unnecessarily public even though it is only used by bridge internals. Change
JSCall in JS.swift to internal unless you explicitly intend to expose it as part
of the public API, and keep its members aligned with that visibility. If
external access is not required, reduce the type’s access level so the bridge
implementation remains encapsulated.
| let method = dict["action"] as? String ?? "" | ||
| let callbackId = dict["callbackId"] as? String ?? "" | ||
|
|
||
| let args = dict["actionArgs"] as? Array ?? [] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify no unspecialized Array casts remain in Swift sources.
rg -n 'as\?\s*Array(\s|$)' ios --glob '*.swift'Repository: Cap-go/capacitor-plus
Length of output: 286
Specialize the Cordova args cast
Array needs an element type here; cast the Cordova args as [Any] instead of Array.
🤖 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 `@ios/CapacitorCordova/CapacitorCordova/Classes/Public/Plugin.swift` at line
43, The Cordova args cast in the plugin argument parsing is too generic; update
the `actionArgs` extraction in `Plugin.swift` to use a concrete `[Any]` type
instead of `Array`, keeping the rest of the `dict["actionArgs"]` handling
unchanged. This applies to the `args` assignment inside the `Plugin` code path
and should preserve the existing default empty array behavior.
| "main", | ||
| "plus" | ||
| ], | ||
| "allowBranch": "next", |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'lerna version|allowBranch' .github/workflows/*.yml scripts/*.sh 2>/dev/nullRepository: Cap-go/capacitor-plus
Length of output: 233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== lerna.json ==\n'
cat -n lerna.json
printf '\n== bump_version workflow excerpt ==\n'
sed -n '1,140p' .github/workflows/bump_version.ymlRepository: Cap-go/capacitor-plus
Length of output: 3215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== branch references ==\n'
rg -n '\bplus\b|\bnext\b|allowBranch|bump_version|lerna version' .github lerna.json package.json . 2>/dev/null || trueRepository: Cap-go/capacitor-plus
Length of output: 50378
lerna.json should allow the plus release branch
npx lerna version runs from .github/workflows/bump_version.yml on plus, so allowBranch: "next" will block automated releases unless plus is added here.
🤖 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 `@lerna.json` at line 7, The release branch guard in lerna.json is too
restrictive because allowBranch only permits next, which blocks the automated
versioning flow on plus. Update the Lerna configuration so the allowBranch
setting includes plus alongside next, keeping npx lerna version compatible with
the bump_version workflow.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge Conflict Resolution Required
The sync of upstream PR ionic-team#8521 from @jcesarmobile encountered merge conflicts.
Original PR: ionic-team#8521
What happened
Synced from upstream by Capacitor+ Bot
Summary by CodeRabbit
New Features
Bug Fixes
Breaking Changes