Skip to content

Avoid possible race condition issues with loading google and apple pa…#3130

Merged
Crabcyborg merged 4 commits into
masterfrom
avoid_possible_race_condition_issues_loading_google_and_apple_pay_sdks
Jun 11, 2026
Merged

Avoid possible race condition issues with loading google and apple pa…#3130
Crabcyborg merged 4 commits into
masterfrom
avoid_possible_race_condition_issues_loading_google_and_apple_pay_sdks

Conversation

@Crabcyborg

@Crabcyborg Crabcyborg commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

…y sdks

Related ticket https://secure.helpscout.net/conversation/3348528710/253332

This update waits for up to 3 seconds for Google Pay and Apple Pay to load so we're less likely to run into cases where the options never appear because PayPal loaded first.

Pre-release
formidable-6.31.1b.zip

Summary by CodeRabbit

  • Refactor

    • More reliable payment-method initialization and selector construction for smoother checkout UI
    • Unified handling of Pay Later option markup for consistent display
  • New Features

    • Google Pay and Apple Pay eligibility are now detected in parallel so their buttons appear more responsively when available

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d31aca00-b431-4c62-8a13-96bf3433447f

📥 Commits

Reviewing files that changed from the base of the PR and between 842f515 and c26b9f9.

📒 Files selected for processing (2)
  • paypal/controllers/FrmPayPalLiteActionsController.php
  • paypal/js/frontend.js
✅ Files skipped from review due to trivial changes (1)
  • paypal/controllers/FrmPayPalLiteActionsController.php
🚧 Files skipped from review as they are similar to previous changes (1)
  • paypal/js/frontend.js

📝 Walkthrough

Walkthrough

PayPal init now clears method state, discovers base payment methods, then awaits a deferred, parallel eligibility check for Google Pay and Apple Pay before building the radio selector. Radio option DOM creation is centralized via a new helper. PHP exposes an include flag for deferred discovery.

Changes

PayPal Deferred Discovery & UI Refactor

Layer / File(s) Summary
Init flow orchestration with deferred discovery
paypal/js/frontend.js (134–151)
paypalInit() clears and reinitializes payment method state, runs base discovery, awaits discoverDeferredPaymentMethods() for Google Pay/Apple Pay, then proceeds to build the selector UI.
Deferred payment method eligibility resolution
paypal/js/frontend.js (277–387)
Adds discoverDeferredPaymentMethods(), resolveGooglePayEligibility(), resolveApplePayEligibility(), and waitFor() to poll SDK readiness and perform parallel eligibility checks; eligible google_pay/apple_pay methods are registered in order.
Radio selector UI building refactor
paypal/js/frontend.js (423–517)
Introduces buildMethodOption(key, method) and updates buildRadioGroup() to append returned option nodes; centralizes mark/icon rendering and Pay Later wrapper/message handling.
Expose include flag to frontend
paypal/controllers/FrmPayPalLiteActionsController.php (1412–1419)
Adds includeGooglePayApplePay to frmPayPalVars localized script data to signal whether deferred discovery should attempt Google Pay / Apple Pay.

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • Strategy11/formidable-forms#3102: Related Apple Pay changes — that PR adjusted Apple Pay button click handling while this PR defers Apple/Google eligibility discovery and refactors selector UI.

Poem

🐰 I hopped through scripts with careful art,
Polling SDKs until they’d start,
Google and Apple queued in line,
Radios built so options shine,
A tiny rabbit danced—deferred, but smart.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly addresses the main objective of the PR: implementing a mechanism to avoid race condition issues when loading Google Pay and Apple Pay SDKs by waiting for them to load.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch avoid_possible_race_condition_issues_loading_google_and_apple_pay_sdks

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.

@deepsource-io

deepsource-io Bot commented Jun 8, 2026

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in f1d5dd4...c26b9f9 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

Important

Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
PHP Jun 11, 2026 3:37p.m. Review ↗
JavaScript Jun 11, 2026 3:37p.m. Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@paypal/js/frontend.js`:
- Around line 134-150: The async interleaving in paypalInit() can let stale runs
mutate module-scoped state (paymentMethods, thisForm, cardFieldsValid, SDK
caches) after a newer run has already reset things; add a monotonically
increasing init token (e.g., module-scoped initCounter and local runToken) and
increment/init it before any awaits, reset per-run globals
(paymentMethods.clear(), selectedMethod=null, thisForm=null,
cardFieldsValid=false, clear relevant SDK caches) immediately before the first
await, then in any async continuations (after await discoverPaymentMethods and
await discoverDeferredPaymentMethods and the other affected block at 294-308)
check that the current module initCounter still equals the runToken and bail out
early if not, ensuring only the latest paypalInit run mutates shared state.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9d84e0ae-7480-46bc-a0a5-351c95d7f7b5

📥 Commits

Reviewing files that changed from the base of the PR and between 0bd6314 and 842f515.

📒 Files selected for processing (1)
  • paypal/js/frontend.js

Comment thread paypal/js/frontend.js
Comment on lines +134 to +150
// Reset state so each run rediscovers cleanly (the DOM above was cleared).
paymentMethods.clear();
selectedMethod = null;

// 1. Discover synchronous methods (Card, PayPal, alternative funding).
await discoverPaymentMethods( {
cardFieldsAreSupported,
buttonsAreEnabled,
isRecurring
} );

// 1b. Discover Google Pay / Apple Pay. Their SDKs (pay.js / PayPal applepay
// component) load asynchronously and are frequently not ready during the first
// run, which previously left these buttons missing on first load. We wait
// (bounded) for them here so every eligible method appears together in one
// render instead of popping in afterward.
await discoverDeferredPaymentMethods( { buttonsAreEnabled, isRecurring } );

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Guard paypalInit() against stale async reruns.

These new deferred awaits create a real interleaving window where an older paypalInit() can resume after a later run has already cleared and rebuilt the shared module state. Because paymentMethods, thisForm, cardFieldsValid, and the SDK caches are all module-scoped, the stale run can register methods into the new registry, append UI against the wrong form state, or re-enable card submit using validity from the previous render. Add a monotonically increasing init token and reset the rest of the per-run globals before the first await.

Suggested direction
+	let initRunId = 0;
+
 	async function paypalInit() {
+		const runId = ++initRunId;
 		const cardElement = document.querySelector( '.frm-card-element' );
 		if ( ! cardElement ) {
 			return;
 		}
@@
 		// Reset state so each run rediscovers cleanly (the DOM above was cleared).
 		paymentMethods.clear();
 		selectedMethod = null;
+		cardFieldsValid = false;
+		submitEvent = null;
+		googlePayConfig = null;
+		applePayConfig = null;
+		applePayInstance = null;
 
 		// 1. Discover synchronous methods (Card, PayPal, alternative funding).
 		await discoverPaymentMethods( {
 			cardFieldsAreSupported,
 			buttonsAreEnabled,
 			isRecurring
 		} );
+		if ( runId !== initRunId ) {
+			return;
+		}
@@
 		await discoverDeferredPaymentMethods( { buttonsAreEnabled, isRecurring } );
+		if ( runId !== initRunId ) {
+			return;
+		}
+
+		// Apply the same stale-run guard after later awaited init steps that mutate DOM/state.

Also applies to: 294-308

🤖 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 `@paypal/js/frontend.js` around lines 134 - 150, The async interleaving in
paypalInit() can let stale runs mutate module-scoped state (paymentMethods,
thisForm, cardFieldsValid, SDK caches) after a newer run has already reset
things; add a monotonically increasing init token (e.g., module-scoped
initCounter and local runToken) and increment/init it before any awaits, reset
per-run globals (paymentMethods.clear(), selectedMethod=null, thisForm=null,
cardFieldsValid=false, clear relevant SDK caches) immediately before the first
await, then in any async continuations (after await discoverPaymentMethods and
await discoverDeferredPaymentMethods and the other affected block at 294-308)
check that the current module initCounter still equals the runToken and bail out
early if not, ensuring only the latest paypalInit run mutates shared state.

@Crabcyborg
Crabcyborg merged commit 1e8df4e into master Jun 11, 2026
20 of 22 checks passed
@Crabcyborg
Crabcyborg deleted the avoid_possible_race_condition_issues_loading_google_and_apple_pay_sdks branch June 11, 2026 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant