chore: sync plus with upstream main (upstream-preferred conflicts) - #113
Open
riderx wants to merge 31 commits into
Open
chore: sync plus with upstream main (upstream-preferred conflicts)#113riderx wants to merge 31 commits into
riderx wants to merge 31 commits into
Conversation
Co-authored-by: jcesarmobile <jcesarmobile@gmail.com>
Co-authored-by: Joey Pender <joey.pender@outsystems.com> Co-authored-by: Pedro Bilro <pedro.gustavo.bilro@outsystems.com> Co-authored-by: Mark Anderson <mark.anderson@outsystems.com>
…#8476) Co-authored-by: jcesarmobile <jcesarmobile@gmail.com>
…am#8492) Co-authored-by: Mark Anderson <mark.anderson@outsystems.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…-team#8271) Co-authored-by: Eric Horodyski <horodyski@ionic.io>
# Conflicts: # .github/workflows/ci.yml
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 15 minutes Limit details: You’ve used all 2 included reviews currently available. Your 56 included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. You’re in a promotional period — use the checkbox below to run this review for free:
On-demand reviews are free for the next 30 days. After that, they cost $0.25 per reviewed file. How can I continue?Run this review now using the option above, or comment You can also wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (57)
Comment |
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. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
40 issues found across 57 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="core/package.json">
<violation number="1" location="core/package.json:2">
P0: The upstream-preferred sync overwrote the package name back to `@capacitor/core`, but this repo's README rules require the `@capacitor-plus` scope for the package name. Publishing under `@capacitor/core` would push this fork under the official Ionic package name instead of the plus scope, breaking the plus distribution. Keep the name as `@capacitor-plus/core`; the version/description/homepage/author can follow upstream.</violation>
</file>
<file name="ios/package.json">
<violation number="1" location="ios/package.json:2">
P0: This upstream-preferred sync reverted the package scope from `@capacitor-plus/ios` to upstream `@capacitor/ios`. The plus fork publishes under the `@capacitor-plus` scope (per the repo README and `# npm install @capacitor-plus/ios`), so this name change would publish `@capacitor/ios`, which is owned by Ionic on npm, and breaks the drop-in replacement guarantee. Restore `@capacitor-plus/ios` and the Capgo description/homepage/author.</violation>
<violation number="2" location="ios/package.json:28">
P1: The sync dropped the `@capacitor-plus/core` peer dependency from ios, leaving only `@capacitor/core`. This fork package declares a peer dependency on the fork's core via `scripts/sync-peer-dependencies.mjs`, so it should keep `@capacitor-plus/core`. Note the package name was also reverted to `@capacitor/ios`. Restore the `@capacitor-plus/core` peer dependency.</violation>
</file>
<file name="cli/package.json">
<violation number="1" location="cli/package.json:2">
P0: This sync conflict resolved to upstream's package name, reverting the CLI from `@capacitor-plus/cli` to `@capacitor/cli`. That breaks the fork's package identity: `bun.lock` still maps the `cli` workspace to `@capacitor-plus/cli` (so `bun install` regenerates a mismatched lockfile / workspace), the release workflow and README publish `@capacitor-plus/cli`, and `cli/src/config.ts` generates `import type { CapacitorConfig } from '@capacitor-plus/cli'` which would point at a package that no longer ships. Publishing under `@capacitor/cli` also collides with the real upstream npm package. Keep the plus-scoped name while merging the rest of the upstream sync.</violation>
</file>
<file name="android/package.json">
<violation number="1" location="android/package.json:2">
P1: The upstream-preferred conflict resolution overwrote the plus-specific package identity with upstream's. The package is now named `@capacitor/android` (owned by Ionic on npm) instead of `@capacitor-plus/android`, and the `@capacitor-plus/core` peerDependency was dropped. The whole release pipeline depends on the plus name: `scripts/sync-peer-dependencies.mjs` (the root `version` script) matches `@capacitor-plus/android`/`@capacitor-plus/ios` and syncs the `@capacitor-plus/core` peer range from `@capacitor-plus/core` — after this change `lerna ls` no longer finds those names, so the core peer sync silently stops and `npm publish` will target the `@capacitor` scope instead of `@capacitor-plus`. Restore the `@capacitor-plus` package name, description/homepage/author, and the `@capacitor-plus/core` peerDependency (keeping `@capacitor/core` as well, as prior versions did).</violation>
<violation number="2" location="android/package.json:26">
P1: The sync dropped the `@capacitor-plus/core` peer dependency from android, leaving only `@capacitor/core`. This fork package requires the fork's core, and `scripts/sync-peer-dependencies.mjs` explicitly maintains `@capacitor-plus/core` as a peer dependency of android/ios. Also note the package name was reverted to `@capacitor/android` (see core/package.json). Restore the `@capacitor-plus/core` peer dependency alongside the upstream one.</violation>
</file>
<file name="android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java">
<violation number="1" location="android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java:275">
P2: On API versions where an intermediate view consumes window insets, this injects safe-area CSS from the consumed listener insets. Use the root-derived `safeAreaSource` so passthrough mode preserves the actual system-bar and cutout insets.</violation>
<violation number="2" location="android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java:300">
P1: When safe-area passthrough is disabled, this calculates CSS variables from `newInsets`, which deliberately zeros system-bar and cutout insets. Apps using CSS inset handling therefore receive zero safe-area values on Android 11+, so calculate from `safeAreaSource` instead.</violation>
<violation number="3" location="android/capacitor/src/main/java/com/getcapacitor/plugin/SystemBars.java:363">
P2: On API <30, default `hide()` and `show()` now leave `navBarVisible` unchanged even though `calcSafeAreaInsets()` uses it for the navigation-bar fallback, causing incorrect bottom safe-area values. Update the flag in the empty-bar branches as well.</violation>
</file>
<file name="cli/src/util/xcode.ts">
<violation number="1" location="cli/src/util/xcode.ts:23">
P1: When another target already references `fileRelPath`, `hasFile` makes this function report success without adding the file to the App target's Sources phase. Check target membership separately and add the existing file reference to the requested target instead of treating any project-wide reference as a no-op.</violation>
<violation number="2" location="cli/src/util/xcode.ts:32">
P1: When the App target is not the first native target, this registers `SceneDelegate.swift` in a different target and leaves the App target unable to compile the migrated delegate. Select the native target by its name or UUID rather than using `getFirstTarget()`.</violation>
<violation number="3" location="cli/src/util/xcode.ts:49">
P2: When multiple PBXGroups share `groupName`, this picks the first one and can place the file under the wrong group or path. Require a unique match or identify the group using its path/UUID.</violation>
</file>
<file name="cli/src/util/node.ts">
<violation number="1" location="cli/src/util/node.ts:41">
P1: When a TypeScript 7 config uses CommonJS `require()` or an extensionless relative import, this branch loads it with native ESM semantics and fails. Preserve the previous CommonJS/TypeScript resolution behavior or provide an equivalent compatibility loader.</violation>
<violation number="2" location="cli/src/util/node.ts:41">
P2: When the same config is loaded more than once in a process, this branch returns the cached ESM module and ignores subsequent file changes. Preserve the existing reload behavior with a cache-busting or equivalent ESM cache strategy.</violation>
<violation number="3" location="cli/src/util/node.ts:43">
P2: When TypeScript lacks the classic compiler API, the config is loaded via Node's native type stripping, which only supports erasable syntax. A config file using an enum, namespace, or parameter property makes Node throw ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX, which doesn't match this check, so it rethrows a cryptic parse failure instead of the actionable guidance, while the old transpileModule path handled all TS syntax. Catch that error code as well (or add a specific message about the config using non-erasable syntax). Verified on Node 22.22: importing a .ts file containing `enum Foo { A }` rejects with code 'ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX'.</violation>
</file>
<file name="android/capacitor/src/main/java/com/getcapacitor/BridgeWebChromeClient.java">
<violation number="1" location="android/capacitor/src/main/java/com/getcapacitor/BridgeWebChromeClient.java:414">
P2: When the host activity is recreated during image capture, the new client has no instance listener and this path has no static recovery state, so the result is dropped. Store the callback, URI, and `IMAGE_CAPTURE` type before launching, as the video and file-picker paths do.</violation>
</file>
<file name="ios/Capacitor/Capacitor/CapacitorBridge.swift">
<violation number="1" location="ios/Capacitor/Capacitor/CapacitorBridge.swift:266">
P2: When an existing app has not adopted `UIScene`, these observers never fire, so document `resume` and `pause` events stop reaching JavaScript. Keep the `UIApplication` observers for app-delegate-only apps, or gate the scene-only path behind scene lifecycle adoption.</violation>
</file>
<file name="cli/src/ios/update.ts">
<violation number="1" location="cli/src/ios/update.ts:64">
P3: Inside the `Promise.all` loop over `validSPMPackages`, `getCapacitorPackageVersion(config, config.ios.name)` recomputes the same value once per plugin. It reads the Capacitor package.json from disk on every parallel iteration. Hoist the call above the loop and pass the resolved version into the mapping so it is read a single time.</violation>
<violation number="2" location="cli/src/ios/update.ts:75">
P2: When a mismatched plugin has trailing package arguments such as `traits:`, this call removes them while replacing the version. Replace only the version token so the plugin's other dependency arguments remain intact.</violation>
</file>
<file name="cli/src/util/spm.ts">
<violation number="1" location="cli/src/util/spm.ts:141">
P2: On Windows, `join('symlinks', plugin.name)` produces a backslash, so `relPath` writes a malformed path into `Package.swift`; normalize it before interpolation.</violation>
<violation number="2" location="cli/src/util/spm.ts:177">
P2: When `moduleAliases` is configured for a Cordova package dependency, the later Cordova branch replaces this `pluginText` and drops `aliasText`; preserve the aliases on that product.</violation>
</file>
<file name="cli/src/tasks/migrate-uiscene.ts">
<violation number="1" location="cli/src/tasks/migrate-uiscene.ts:108">
P2: `scanAndWarn` reads every Swift file in the platform directory with no error handling and runs after all native files have already been written. If any matched file can't be read, the error travels up to `migrateCommand`'s catch and calls `fatal`, failing the whole migration after the project was already partially rewritten. Wrap the read in try/catch and skip unreadable files.</violation>
<violation number="2" location="cli/src/tasks/migrate-uiscene.ts:146">
P2: When an existing `AppDelegate.swift` contains braces in a string or comment, this delimiter scan can inject `configurationForConnecting` at the wrong position and break the generated project. Use a Swift-aware lexer/parser, or at minimum ignore braces in strings and comments while locating the class boundary.</violation>
<violation number="3" location="cli/src/tasks/migrate-uiscene.ts:174">
P2: When the shipped template archive is missing or corrupt, `extractTemplate` throws before `loadTemplateAssets` can return `null`, so the migration command aborts instead of logging the skip message. Catch extraction and file-read errors, clean up the temporary directory, and return `null` for this recoverable migration failure.</violation>
</file>
<file name="ios/Capacitor/Capacitor/CAPSceneDelegateProxy.swift">
<violation number="1" location="ios/Capacitor/Capacitor/CAPSceneDelegateProxy.swift:18">
P2: On a cold start, plugins cannot observe `capacitorSceneWillConnect` because this post occurs before registration, and the deferred block never re-emits it. Re-post the scene-connect notification after plugins load, or defer its initial post.</violation>
<violation number="2" location="ios/Capacitor/Capacitor/CAPSceneDelegateProxy.swift:24">
P2: When multiple scenes connect before their views appear, this global observer replays pending URL and activity events on the first appearance, causing early or duplicate delivery. Scope readiness to each scene to prevent cross-scene replay.</violation>
<violation number="3" location="ios/Capacitor/Capacitor/CAPSceneDelegateProxy.swift:24">
P2: The one-shot observer registered in `willConnectTo` is only removed when `.capacitorViewDidAppear` fires. `.capacitorViewDidAppear` is posted exactly once per `CAPBridgeViewController.viewDidAppear` (CAPBridgeViewController.swift:79), so if `willConnectTo` runs but that notification never posts — a scene connecting a non-Capacitor window, a bridge load that fails, or a scene session that is disconnected before its view appears — the observer stays registered forever, retaining the captured `scene`, `connectionOptions`, and closure. If `willConnectTo` runs more than once before the view appears (scene reconfiguration/multi-scene), an observer is registered each time and, when `viewDidAppear` finally fires, every one of them runs and re-delivers the same cold-start URL/activity, producing duplicate `.capacitorOpenURL`/`.capacitorSceneOpenURL` posts. Remove the observer on scene disconnect (e.g. in `sceneDidDisconnect`) instead of relying solely on the notification firing.</violation>
</file>
<file name="ios/CHANGELOG.md">
<violation number="1" location="ios/CHANGELOG.md:14">
P2: The upstream-preferred merge replaced the entire @capacitor-plus/ios changelog with upstream @capacitor/ios history, wiping the fork's actually-published patch releases (8.3.5-8.3.12, 8.0.4-8.0.8) and every @capacitor-plus/ios package reference. Anyone reading this CHANGELOG now sees versions that were never published for this package and cannot tell which releases the fork shipped. Preserve the plus-specific release history (and use the @capacitor-plus/ios identity) on top of the upstream entries rather than dropping it, since the sync doc explicitly requires community/plus inclusions not be lost.</violation>
</file>
<file name="CHANGELOG.md">
<violation number="1" location="CHANGELOG.md:69">
P2: The upstream-preferred merge mangled the plus changelog: it removed the plus `## [8.3.12]` entry at the top (which carried the `a63975d` SystemBars fix) and the upstream `## [8.3.1]` header now swallows that plus fix, misattributing it. The result also breaks version ordering — upstream goes 8.5.0→8.4.2→...→8.3.1, then jumps back up to plus 8.3.11→8.3.10→...→8.3.5, and the top-of-file version (8.5.0) no longer reflects the current plus release (8.3.x). Since this changelog is published to npm, the sync output misreports the current release, loses plus's 8.3.12 entry, and misattributes its fix. Re-run the sync keeping the plus-history top entries (8.3.x/8.3.12) intact and merging upstream content below them instead of replacing them.</violation>
</file>
<file name=".github/workflows/ci.yml">
<violation number="1" location=".github/workflows/ci.yml:19">
P3: The `setup` job is a no-op: it only runs setup-node, checkout, and a cache restore of `~/.npm`, but never runs `npm install`, so it neither produces an artifact nor populates/registers the `~/.npm` cache for downstream jobs. Every test job (`test-cli`, `test-core`, `test-ios`, `test-android`) already runs its own setup-node, checkout, cache-restore, and `npm install` independently, and they already gate on `lint`. `setup` only delays job start and adds a `needs` dependency with no value; remove it and drop `setup` from each job's `needs` list.</violation>
<violation number="2" location=".github/workflows/ci.yml:21">
P2: Every job in this workflow sets `timeout-minutes: 30`, contradicting this repo's documented CI timeout policy. AGENTS.md states "Keep CI, script, and runtime timeouts at 10 minutes or less. Use `timeout-minutes: 10` or lower in GitHub Actions", and the repo's other workflows (build.yml, test.yml) enforce it with `# Keep this job capped at 10 minutes; never raise it unless explicitly asked.` Apply `timeout-minutes: 10` to all six jobs.</violation>
<violation number="3" location=".github/workflows/ci.yml:46">
P2: The CI installs with `npm install` although AGENTS.md mandates Bun exclusively and the repo's lockfile is bun.lock (package-lock is disabled in .npmrc, so no lockfile drives the npm install). Replaced in place, `npm install` resolves dependency ranges without the lockfile, so installs can drift from bun.lock; and because the cache key only hashes package.json, it will never invalidate when bun.lock or resolved versions change, serving stale node_modules. Use `bun install`/`bun run` and hash bun.lock in the cache key.</violation>
</file>
<file name="cli/src/tasks/migrate.ts">
<violation number="1" location="cli/src/tasks/migrate.ts:449">
P3: When the project uses `@capacitor-plus/ios`, this condition is false even though the UIScene migration runs for that package. Include the plus package so those users receive the 8.5 migration guidance too.</violation>
</file>
<file name="cli/src/tasks/run.ts">
<violation number="1" location="cli/src/tasks/run.ts:120">
P2: On an Android live-reload run that fails inside `run()`, the new catch reverts only the Capacitor config but not the Android manifest, so `android:usesCleartextTraffic="true"` stays written. The SIGINT handler reverts both, so error cleanup is inconsistent with normal cleanup. Revert the manifest here too (guarded to android) so the cleartext flag does not leak until the next sync.</violation>
<violation number="2" location="cli/src/tasks/run.ts:120">
P3: If the cleanup `revertCapConfigForLiveReload()` throws here, the original error `e` is lost and the caller sees the cleanup failure instead of the real run failure, because the rest of the catch (fatal handling) never runs. Wrap the revert in its own try/catch so a cleanup error cannot mask the error being handled.</violation>
</file>
<file name="ios-pods-template/App/App/AppDelegate.swift">
<violation number="1" location="ios-pods-template/App/App/AppDelegate.swift:49">
P2: After enabling the scene lifecycle here, the retained `application(_:open:options:)` and `application(_:continue:restorationHandler:)` methods (which call `ApplicationDelegateProxy.shared`) are dead code: in scene-based apps UIKit delivers URL opens and universal links to the scene delegate, not to these AppDelegate methods. The new SceneDelegate already handles them via `SceneDelegateProxy`. This also diverges from the SPM template in this same PR, which correctly removed these two methods. Remove them from the pods AppDelegate for consistency.</violation>
</file>
<file name="cli/CHANGELOG.md">
<violation number="1" location="cli/CHANGELOG.md:6">
P2: The upstream-preferred merge replaced the entire top of the changelog with upstream's history, dropping the fork's own published release notes (8.3.12→8.0.1). Fork-only changes are now undocumented, including the @capacitor-plus namespace migration, the Capacitor package-resolution fallbacks, and the homepage URL updates across packages. Re-apply the fork-specific entries on top of the upstream section so readers of @capacitor-plus/cli can still trace the fork's own releases.</violation>
</file>
<file name="cli/test/migrate-uiscene-scene-delegate.spec.ts">
<violation number="1" location="cli/test/migrate-uiscene-scene-delegate.spec.ts:43">
P3: The roundtrip test derives both its input fixture and its expected output from the same shipped template, and `stripConfigurationForConnecting` hardcodes the template's exact formatting (4-space indent, 21-space continuation, specific closing-brace layout). This makes the test self-consistent rather than independent: it cannot catch a regression in `extractConfigurationForConnecting`/`insertBeforeAppDelegateClassEnd` that stays symmetric with the strip, and it never exercises a genuine 8.4-era AppDelegate (the real migration input). Consider asserting against a hand-written standalone pre-UIScene fixture instead of deriving it from the shipped template.</violation>
</file>
<file name="cli/test/migrate-uiscene-scan.spec.ts">
<violation number="1" location="cli/test/migrate-uiscene-scan.spec.ts:156">
P3: The test titled 'skips Pods/, build/, and DerivedData/ directories' never creates a DerivedData directory (or a .build dir), so the DerivedData/ and .build/ exclusions in scanAndWarn are untested. Add a DerivedData (and ideally .build) directory under the ios dir with an offending file so the test actually covers the behavior its name promises.</violation>
</file>
<file name="cli/src/telemetry.ts">
<violation number="1" location="cli/src/telemetry.ts:85">
P3: getIOSPackageManager resolves `config.ios.packageManager` on every telemetry-tracked command for iOS projects, even when telemetry is off or the terminal is non-interactive (CI) so no metric is sent. This forces `determinePackageManager` (filesystem stats plus `git rev-parse --show-toplevel`/`isInstalled('bundle')` subprocesses) to run on the main command path where it previously stayed lazy and was only resolved by iOS subcommands. Compute the package manager only inside the `if (isInteractive())` block, immediately before `sendMetric`, so it's resolved only when a metric will actually be transmitted.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| "description": "Capacitor+: Enhanced Capacitor with automated upstream sync - Cross-platform apps with JavaScript and the web", | ||
| "homepage": "https://capgo.app/docs/plugins/capacitor-plus/", | ||
| "author": "Capgo Team <support@capgo.app> (https://capgo.app)", | ||
| "name": "@capacitor/core", |
There was a problem hiding this comment.
P0: The upstream-preferred sync overwrote the package name back to @capacitor/core, but this repo's README rules require the @capacitor-plus scope for the package name. Publishing under @capacitor/core would push this fork under the official Ionic package name instead of the plus scope, breaking the plus distribution. Keep the name as @capacitor-plus/core; the version/description/homepage/author can follow upstream.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At core/package.json, line 2:
<comment>The upstream-preferred sync overwrote the package name back to `@capacitor/core`, but this repo's README rules require the `@capacitor-plus` scope for the package name. Publishing under `@capacitor/core` would push this fork under the official Ionic package name instead of the plus scope, breaking the plus distribution. Keep the name as `@capacitor-plus/core`; the version/description/homepage/author can follow upstream.</comment>
<file context>
@@ -1,9 +1,9 @@
- "description": "Capacitor+: Enhanced Capacitor with automated upstream sync - Cross-platform apps with JavaScript and the web",
- "homepage": "https://capgo.app/docs/plugins/capacitor-plus/",
- "author": "Capgo Team <support@capgo.app> (https://capgo.app)",
+ "name": "@capacitor/core",
+ "version": "8.5.0",
+ "description": "Capacitor: Cross-platform apps with JavaScript and the web",
</file context>
Suggested change
| "name": "@capacitor/core", | |
| "name": "@capacitor-plus/core", |
| "description": "Capacitor+: Enhanced Capacitor with automated upstream sync - Cross-platform apps with JavaScript and the web", | ||
| "homepage": "https://capgo.app/docs/plugins/capacitor-plus/", | ||
| "author": "Capgo Team <support@capgo.app> (https://capgo.app)", | ||
| "name": "@capacitor/ios", |
There was a problem hiding this comment.
P0: This upstream-preferred sync reverted the package scope from @capacitor-plus/ios to upstream @capacitor/ios. The plus fork publishes under the @capacitor-plus scope (per the repo README and # npm install @capacitor-plus/ios), so this name change would publish @capacitor/ios, which is owned by Ionic on npm, and breaks the drop-in replacement guarantee. Restore @capacitor-plus/ios and the Capgo description/homepage/author.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ios/package.json, line 2:
<comment>This upstream-preferred sync reverted the package scope from `@capacitor-plus/ios` to upstream `@capacitor/ios`. The plus fork publishes under the `@capacitor-plus` scope (per the repo README and `# npm install @capacitor-plus/ios`), so this name change would publish `@capacitor/ios`, which is owned by Ionic on npm, and breaks the drop-in replacement guarantee. Restore `@capacitor-plus/ios` and the Capgo description/homepage/author.</comment>
<file context>
@@ -1,9 +1,9 @@
- "description": "Capacitor+: Enhanced Capacitor with automated upstream sync - Cross-platform apps with JavaScript and the web",
- "homepage": "https://capgo.app/docs/plugins/capacitor-plus/",
- "author": "Capgo Team <support@capgo.app> (https://capgo.app)",
+ "name": "@capacitor/ios",
+ "version": "8.5.0",
+ "description": "Capacitor: Cross-platform apps with JavaScript and the web",
</file context>
Suggested change
| "name": "@capacitor/ios", | |
| "name": "@capacitor-plus/ios", |
| "description": "Capacitor+: Enhanced Capacitor with automated upstream sync - Cross-platform apps with JavaScript and the web", | ||
| "homepage": "https://capgo.app/docs/plugins/capacitor-plus/", | ||
| "author": "Capgo Team <support@capgo.app> (https://capgo.app)", | ||
| "name": "@capacitor/cli", |
There was a problem hiding this comment.
P0: This sync conflict resolved to upstream's package name, reverting the CLI from @capacitor-plus/cli to @capacitor/cli. That breaks the fork's package identity: bun.lock still maps the cli workspace to @capacitor-plus/cli (so bun install regenerates a mismatched lockfile / workspace), the release workflow and README publish @capacitor-plus/cli, and cli/src/config.ts generates import type { CapacitorConfig } from '@capacitor-plus/cli' which would point at a package that no longer ships. Publishing under @capacitor/cli also collides with the real upstream npm package. Keep the plus-scoped name while merging the rest of the upstream sync.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/package.json, line 2:
<comment>This sync conflict resolved to upstream's package name, reverting the CLI from `@capacitor-plus/cli` to `@capacitor/cli`. That breaks the fork's package identity: `bun.lock` still maps the `cli` workspace to `@capacitor-plus/cli` (so `bun install` regenerates a mismatched lockfile / workspace), the release workflow and README publish `@capacitor-plus/cli`, and `cli/src/config.ts` generates `import type { CapacitorConfig } from '@capacitor-plus/cli'` which would point at a package that no longer ships. Publishing under `@capacitor/cli` also collides with the real upstream npm package. Keep the plus-scoped name while merging the rest of the upstream sync.</comment>
<file context>
@@ -1,9 +1,9 @@
- "description": "Capacitor+: Enhanced Capacitor with automated upstream sync - Cross-platform apps with JavaScript and the web",
- "homepage": "https://capgo.app/docs/plugins/capacitor-plus/",
- "author": "Capgo Team <support@capgo.app> (https://capgo.app)",
+ "name": "@capacitor/cli",
+ "version": "8.5.0",
+ "description": "Capacitor: Cross-platform apps with JavaScript and the web",
</file context>
Suggested change
| "name": "@capacitor/cli", | |
| "name": "@capacitor-plus/cli", |
| "peerDependencies": { | ||
| "@capacitor-plus/core": "^8.3.0", | ||
| "@capacitor/core": "^8.3.0" | ||
| "@capacitor/core": "^8.5.0" |
There was a problem hiding this comment.
P1: The sync dropped the @capacitor-plus/core peer dependency from ios, leaving only @capacitor/core. This fork package declares a peer dependency on the fork's core via scripts/sync-peer-dependencies.mjs, so it should keep @capacitor-plus/core. Note the package name was also reverted to @capacitor/ios. Restore the @capacitor-plus/core peer dependency.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ios/package.json, line 28:
<comment>The sync dropped the `@capacitor-plus/core` peer dependency from ios, leaving only `@capacitor/core`. This fork package declares a peer dependency on the fork's core via `scripts/sync-peer-dependencies.mjs`, so it should keep `@capacitor-plus/core`. Note the package name was also reverted to `@capacitor/ios`. Restore the `@capacitor-plus/core` peer dependency.</comment>
<file context>
@@ -25,8 +25,7 @@
"peerDependencies": {
- "@capacitor-plus/core": "^8.3.0",
- "@capacitor/core": "^8.3.0"
+ "@capacitor/core": "^8.5.0"
},
"publishConfig": {
</file context>
| "peerDependencies": { | ||
| "@capacitor-plus/core": "^8.3.0", | ||
| "@capacitor/core": "^8.3.0" | ||
| "@capacitor/core": "^8.5.0" |
There was a problem hiding this comment.
P1: The sync dropped the @capacitor-plus/core peer dependency from android, leaving only @capacitor/core. This fork package requires the fork's core, and scripts/sync-peer-dependencies.mjs explicitly maintains @capacitor-plus/core as a peer dependency of android/ios. Also note the package name was reverted to @capacitor/android (see core/package.json). Restore the @capacitor-plus/core peer dependency alongside the upstream one.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At android/package.json, line 26:
<comment>The sync dropped the `@capacitor-plus/core` peer dependency from android, leaving only `@capacitor/core`. This fork package requires the fork's core, and `scripts/sync-peer-dependencies.mjs` explicitly maintains `@capacitor-plus/core` as a peer dependency of android/ios. Also note the package name was reverted to `@capacitor/android` (see core/package.json). Restore the `@capacitor-plus/core` peer dependency alongside the upstream one.</comment>
<file context>
@@ -23,8 +23,7 @@
"peerDependencies": {
- "@capacitor-plus/core": "^8.3.0",
- "@capacitor/core": "^8.3.0"
+ "@capacitor/core": "^8.5.0"
},
"publishConfig": {
</file context>
| const validSPMPackages = await checkPluginsForPackageSwift(config, plugins); | ||
| await Promise.all( | ||
| validSPMPackages.map(async (plugin) => { | ||
| const iosPlatformVersion = await getCapacitorPackageVersion(config, config.ios.name); |
There was a problem hiding this comment.
P3: Inside the Promise.all loop over validSPMPackages, getCapacitorPackageVersion(config, config.ios.name) recomputes the same value once per plugin. It reads the Capacitor package.json from disk on every parallel iteration. Hoist the call above the loop and pass the resolved version into the mapping so it is read a single time.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/src/ios/update.ts, line 64:
<comment>Inside the `Promise.all` loop over `validSPMPackages`, `getCapacitorPackageVersion(config, config.ios.name)` recomputes the same value once per plugin. It reads the Capacitor package.json from disk on every parallel iteration. Hoist the call above the loop and pass the resolved version into the mapping so it is read a single time.</comment>
<file context>
@@ -58,6 +59,30 @@ async function updatePluginFiles(config: Config, plugins: Plugin[], deployment:
const validSPMPackages = await checkPluginsForPackageSwift(config, plugins);
+ await Promise.all(
+ validSPMPackages.map(async (plugin) => {
+ const iosPlatformVersion = await getCapacitorPackageVersion(config, config.ios.name);
+ const packageSwiftPath = join(plugin.rootPath, 'Package.swift');
+ let content = await readFile(packageSwiftPath, { encoding: 'utf-8' });
</file context>
| }); | ||
|
|
||
| it('skips Pods/, build/, and DerivedData/ directories', async () => { | ||
| const podsDir = join(iosDir, 'App', 'Pods'); |
There was a problem hiding this comment.
P3: The test titled 'skips Pods/, build/, and DerivedData/ directories' never creates a DerivedData directory (or a .build dir), so the DerivedData/ and .build/ exclusions in scanAndWarn are untested. Add a DerivedData (and ideally .build) directory under the ios dir with an offending file so the test actually covers the behavior its name promises.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/test/migrate-uiscene-scan.spec.ts, line 156:
<comment>The test titled 'skips Pods/, build/, and DerivedData/ directories' never creates a DerivedData directory (or a .build dir), so the DerivedData/ and .build/ exclusions in scanAndWarn are untested. Add a DerivedData (and ideally .build) directory under the ios dir with an offending file so the test actually covers the behavior its name promises.</comment>
<file context>
@@ -0,0 +1,167 @@
+ });
+
+ it('skips Pods/, build/, and DerivedData/ directories', async () => {
+ const podsDir = join(iosDir, 'App', 'Pods');
+ const buildDir = join(iosDir, 'App', 'build');
+ await mkdirp(podsDir);
</file context>
| } | ||
| } catch (e: any) { | ||
| if (options.liveReload) { | ||
| await CapLiveReloadHelper.revertCapConfigForLiveReload(); |
There was a problem hiding this comment.
P3: If the cleanup revertCapConfigForLiveReload() throws here, the original error e is lost and the caller sees the cleanup failure instead of the real run failure, because the rest of the catch (fatal handling) never runs. Wrap the revert in its own try/catch so a cleanup error cannot mask the error being handled.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/src/tasks/run.ts, line 120:
<comment>If the cleanup `revertCapConfigForLiveReload()` throws here, the original error `e` is lost and the caller sees the cleanup failure instead of the real run failure, because the rest of the catch (fatal handling) never runs. Wrap the revert in its own try/catch so a cleanup error cannot mask the error being handled.</comment>
<file context>
@@ -116,6 +116,9 @@ export async function runCommand(
}
} catch (e: any) {
+ if (options.liveReload) {
+ await CapLiveReloadHelper.revertCapConfigForLiveReload();
+ }
if (!isFatal(e)) {
</file context>
| error: error ? (error.message ? error.message : String(error)) : null, | ||
| node_version: process.version, | ||
| os: config.cli.os, | ||
| ios_package_manager: await getIOSPackageManager(config), |
There was a problem hiding this comment.
P3: getIOSPackageManager resolves config.ios.packageManager on every telemetry-tracked command for iOS projects, even when telemetry is off or the terminal is non-interactive (CI) so no metric is sent. This forces determinePackageManager (filesystem stats plus git rev-parse --show-toplevel/isInstalled('bundle') subprocesses) to run on the main command path where it previously stayed lazy and was only resolved by iOS subcommands. Compute the package manager only inside the if (isInteractive()) block, immediately before sendMetric, so it's resolved only when a metric will actually be transmitted.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/src/telemetry.ts, line 85:
<comment>getIOSPackageManager resolves `config.ios.packageManager` on every telemetry-tracked command for iOS projects, even when telemetry is off or the terminal is non-interactive (CI) so no metric is sent. This forces `determinePackageManager` (filesystem stats plus `git rev-parse --show-toplevel`/`isInstalled('bundle')` subprocesses) to run on the main command path where it previously stayed lazy and was only resolved by iOS subcommands. Compute the package manager only inside the `if (isInteractive())` block, immediately before `sendMetric`, so it's resolved only when a metric will actually be transmitted.</comment>
<file context>
@@ -80,9 +82,12 @@ export function telemetryAction(config: Config, action: CommanderAction): Comman
error: error ? (error.message ? error.message : String(error)) : null,
node_version: process.version,
os: config.cli.os,
+ ios_package_manager: await getIOSPackageManager(config),
...Object.fromEntries(versions),
};
</file context>
| cancel-in-progress: ${{ github.ref != 'refs/heads/main' }} | ||
|
|
||
| jobs: | ||
| setup: |
There was a problem hiding this comment.
P3: The setup job is a no-op: it only runs setup-node, checkout, and a cache restore of ~/.npm, but never runs npm install, so it neither produces an artifact nor populates/registers the ~/.npm cache for downstream jobs. Every test job (test-cli, test-core, test-ios, test-android) already runs its own setup-node, checkout, cache-restore, and npm install independently, and they already gate on lint. setup only delays job start and adds a needs dependency with no value; remove it and drop setup from each job's needs list.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yml, line 19:
<comment>The `setup` job is a no-op: it only runs setup-node, checkout, and a cache restore of `~/.npm`, but never runs `npm install`, so it neither produces an artifact nor populates/registers the `~/.npm` cache for downstream jobs. Every test job (`test-cli`, `test-core`, `test-ios`, `test-android`) already runs its own setup-node, checkout, cache-restore, and `npm install` independently, and they already gate on `lint`. `setup` only delays job start and adds a `needs` dependency with no value; remove it and drop `setup` from each job's `needs` list.</comment>
<file context>
@@ -0,0 +1,142 @@
+ cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}
+
+jobs:
+ setup:
+ runs-on: ubuntu-latest
+ timeout-minutes: 30
</file context>
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.
Upstream Plus Sync
The automatic sync of the
plusbranch encountered merge conflicts.What happened
This PR was created automatically by the Capacitor+ sync workflow
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.