Skip to content

chore: sync upstream PR #8252 - Fix iOS CLI checks and Cordova Package.swift generation race#36

Open
riderx wants to merge 4 commits intoplusfrom
sync/upstream-pr-8252
Open

chore: sync upstream PR #8252 - Fix iOS CLI checks and Cordova Package.swift generation race#36
riderx wants to merge 4 commits intoplusfrom
sync/upstream-pr-8252

Conversation

@riderx
Copy link
Member

@riderx riderx commented Nov 28, 2025

Upstream PR Sync

This PR syncs changes from an external contributor's PR on the official Capacitor repository.

Original PR

  • PR: #8252
  • Title: Fix iOS CLI checks and Cordova Package.swift generation race
  • Author: @MOHITKOURAV01

Automation

  • CI will run automatically
  • Claude Code will review for security/breaking changes
  • If approved, this PR will be auto-merged

Synced from upstream by Capacitor+ Bot

Summary by CodeRabbit

  • Chores
    • Enhanced iOS build validation to ensure all prerequisite checks are consistently executed, improving build reliability.
    • Optimized iOS package generation performance through parallel processing of plugin dependencies.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 28, 2025

Walkthrough

The pull request refactors iOS platform check logic to execute checks sequentially and independently rather than short-circuiting on early success, and converts package file generation from sequential to parallel execution using Promise.all.

Changes

Cohort / File(s) Summary
iOS Check Logic Refactoring
cli/src/ios/doctor.ts, cli/src/tasks/add.ts, cli/src/tasks/update.ts
Replaced combined OR conditionals with separate sequential checks for bundler, CocoaPods, and related dependencies. Ensures all checks execute regardless of earlier results instead of short-circuiting.
iOS Update Parallelization
cli/src/ios/update.ts
Converted sequential Cordova package file generation into concurrent execution using async map and Promise.all, enabling parallel processing of plugin files.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Attention areas:
    • Verify that removing short-circuit logic in check functions doesn't cause unintended behavior when multiple checks fail (ensure error accumulation/handling is correct)
    • Review cli/src/ios/update.ts for potential race conditions or error propagation issues when promises execute in parallel
    • Confirm that the four-check flow in doctor.ts performs correctly with all checks running sequentially rather than early exit

Poem

🐰 Four checks now run, no shortcuts today,
Each bundler and pod gets its word to say!
While files fly parallel, swift as the breeze,
The CLI runs strong with concurrent ease! 🚀

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: fixing iOS CLI checks (removing short-circuit logic) and fixing a race condition in Cordova Package.swift generation (parallelizing file generation).
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sync/upstream-pr-8252

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@riderx riderx force-pushed the sync/upstream-pr-8252 branch from b34a3a9 to e4ab3c1 Compare November 28, 2025 15:00
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
cli/src/ios/update.ts (1)

70-76: Cordova Package.swift generation race is correctly fixed; async wrapper can be simplified

Using await Promise.all(...) here ensures all per-plugin Package.swift files are fully written before checkPluginsForPackageSwift runs, which removes the prior race while safely parallelizing independent file writes. The logic looks sound.

Optionally, you can drop the extra async/await layer for brevity:

-async function generateCordovaPackageFiles(cordovaPlugins: Plugin[], config: Config) {
-  await Promise.all(
-    cordovaPlugins.map(async (plugin) => {
-      await generateCordovaPackageFile(plugin, config);
-    }),
-  );
-}
+async function generateCordovaPackageFiles(cordovaPlugins: Plugin[], config: Config) {
+  await Promise.all(cordovaPlugins.map((plugin) => generateCordovaPackageFile(plugin, config)));
+}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 270d700 and e4ab3c1.

📒 Files selected for processing (4)
  • cli/src/ios/doctor.ts (1 hunks)
  • cli/src/ios/update.ts (1 hunks)
  • cli/src/tasks/add.ts (1 hunks)
  • cli/src/tasks/update.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
cli/src/tasks/update.ts (1)
cli/src/ios/common.ts (2)
  • checkBundler (28-43)
  • checkCocoaPods (45-53)
cli/src/ios/doctor.ts (2)
cli/src/common.ts (2)
  • check (16-22)
  • checkWebDir (24-56)
cli/src/ios/common.ts (2)
  • checkBundler (28-43)
  • checkCocoaPods (45-53)
cli/src/tasks/add.ts (1)
cli/src/ios/common.ts (3)
  • checkIOSPackage (15-17)
  • checkBundler (28-43)
  • checkCocoaPods (45-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test-android
  • GitHub Check: test-core
🔇 Additional comments (3)
cli/src/tasks/update.ts (1)

49-55: Fixes misuse of async checks; Bundler and CocoaPods are now both validated

Splitting the iOS branch into two separate checks ensures checkBundler and checkCocoaPods both execute and report independently, instead of short‑circuiting on the first Promise. This is the correct wiring for check() and will surface missing CocoaPods during update where it was previously silent.

cli/src/tasks/add.ts (1)

113-116: IOS add checks are now correctly split; all three validations will run

Returning [checkIOSPackage, checkBundler, checkCocoaPods] as separate check functions is the right pattern for the shared check() helper. This ensures that package presence, Bundler, and CocoaPods are each validated when running add ios, instead of accidentally short‑circuiting and skipping the CocoaPods check.

Given this is now always executed for iOS adds, it’s worth double‑checking that checkCocoaPods still behaves as intended on non‑macOS environments (i.e., doesn’t block add flows where CocoaPods isn’t required).

cli/src/ios/doctor.ts (1)

21-23: Doctor now runs all iOS checks independently, which is the correct pattern

Passing each of checkBundler, checkCocoaPods, checkWebDir, and checkXcode as separate CheckFunctions ensures the doctor command evaluates all iOS preconditions and reports all issues at once, instead of short‑circuiting on the first passing or failing check. The wiring into check() looks correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants