Skip to content

fix: premount loader race condition #896

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 18, 2025
Merged

fix: premount loader race condition #896

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
080eb7b
fix: premount loader race condition
ArushKapoorJuspay Feb 12, 2025
f2288a0
refactor: moved promise inside fetch functions in elements and pmmEle…
ArushKapoorJuspay Feb 17, 2025
72aaf7e
Merge branch 'main' into fix/pre-mount-loader-race
PritishBudhiraja Feb 18, 2025
10ba32f
fix: test hooks failure
PritishBudhiraja Feb 18, 2025
0b55322
Merge branch 'main' into fix/pre-mount-loader-race
PritishBudhiraja Feb 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 21 additions & 17 deletions src/Payments/PreMountLoader.res
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,30 +12,36 @@ let sendPromiseData = (promise, key) => {

let useMessageHandler = getPromisesAndHandler => {
React.useEffect(_ => {
let (promises, messageHandler) = getPromisesAndHandler()
let messageHandler = getPromisesAndHandler()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This variable used to be called getPromisesAndHandler because it used to return promise and message handler. if that is changed to only return handler, then can we rename the function to reflect that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed the name


let setupMessageListener = _ => {
Utils.messageParentWindow([("preMountLoaderIframeMountedCallback", true->JSON.Encode.bool)])
Window.addEventListener("message", messageHandler)
Utils.messageParentWindow([("preMountLoaderIframeMountedCallback", true->JSON.Encode.bool)])
}

let cleanupMessageListener = _ => {
Utils.messageParentWindow([("preMountLoaderIframeUnMount", true->JSON.Encode.bool)])
Window.removeEventListener("message", messageHandler)
Utils.messageParentWindow([("preMountLoaderIframeUnMount", true->JSON.Encode.bool)])
}

setupMessageListener()

let executeAllPromises = async () => {
try {
let _ = await Promise.all(promises)
} catch {
| error => Console.error2("Error in message handler:", error)
let handleCleanUpEventListener = (ev: Window.event) => {
open Utils
let dict = ev.data->safeParse->getDictFromJson
if dict->Dict.get("cleanUpPreMountLoaderIframe")->Option.isSome {
cleanupMessageListener()
}
cleanupMessageListener()
}
executeAllPromises()->ignore

Some(cleanupMessageListener)
Window.addEventListener("message", handleCleanUpEventListener)

setupMessageListener()

Some(
() => {
cleanupMessageListener()
Window.removeEventListener("message", handleCleanUpEventListener)
},
)
}, [])
}

Expand Down Expand Up @@ -87,8 +93,7 @@ module PreMountLoaderForElements = {
}
}

let promises = [paymentMethodsPromise, customerPaymentMethodsPromise, sessionTokensPromise]
(promises, messageHandler)
messageHandler
})

React.null
Expand All @@ -114,8 +119,7 @@ module PreMountLoaderForPMMElements = {
}
}

let promises = [savedPaymentMethodsPromise]
(promises, messageHandler)
messageHandler
})

React.null
Expand Down
37 changes: 31 additions & 6 deletions src/hyper-loader/Elements.res
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,13 @@ let make = (
}
}

let fetchPaymentsList = (mountedIframeRef, componentType) => {
let fetchPaymentsList = (mountedIframeRef, componentType, promiseResolve) => {
let handlePaymentMethodsLoaded = (event: Types.event) => {
let json = event.data->anyTypeToJson
let dict = json->getDictFromJson
let isPaymentMethodsData = dict->getString("data", "") === "payment_methods"
if isPaymentMethodsData {
promiseResolve()
isTaxCalculationEnabled.contents =
dict->getDictFromDict("response")->getBool("is_tax_calculation_enabled", false)
addSmartEventListener("message", onPlaidCallback(mountedIframeRef), "onPlaidCallback")
Expand Down Expand Up @@ -212,6 +213,7 @@ let make = (
mountedIframeRef,
disableSavedPaymentMethods,
componentType,
promiseResolve,
) => {
if !disableSavedPaymentMethods {
let handleCustomerPaymentMethodsLoaded = (event: Types.event) => {
Expand All @@ -223,13 +225,16 @@ let make = (
let json = dict->getJsonFromDict("response", JSON.Encode.null)
let msg = [("customerPaymentMethods", json)]->Dict.fromArray
mountedIframeRef->Window.iframePostMessage(msg)
promiseResolve()
}
}
addSmartEventListener(
"message",
handleCustomerPaymentMethodsLoaded,
`onCustomerPaymentMethodsLoaded-${componentType}`,
)
} else {
promiseResolve()
}
let msg =
[
Expand Down Expand Up @@ -809,13 +814,14 @@ let make = (
addSmartEventListener("message", handleGooglePayThirdPartyFlow, "onGooglePayThirdParty")
addSmartEventListener("message", handleApplePayThirdPartyFlow, "onApplePayThirdParty")

let fetchSessionTokens = mountedIframeRef => {
let fetchSessionTokens = (mountedIframeRef, promiseResolve) => {
let handleSessionTokensLoaded = (event: Types.event) => {
let json = event.data->anyTypeToJson
let dict = json->getDictFromJson
let sessionTokensData = dict->getString("data", "") === "session_tokens"
if sessionTokensData {
let json = dict->getJsonFromDict("response", JSON.Encode.null)
promiseResolve()

{
let sessionsArr =
Expand Down Expand Up @@ -1288,15 +1294,34 @@ let make = (
}
preMountLoaderMountedPromise
->then(_ => {
fetchPaymentsList(mountedIframeRef, componentType)
let paymentMethodsPromise = Promise.make((resolve, _) => {
fetchPaymentsList(mountedIframeRef, componentType, resolve)
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we have to wrap fetchPaymentsList by Promise.make((resolve, _) =. {} ? Why can't fetchPaymentsList return a promise on its own and resolve that promise when it wants

Wrapping with Promise.make and passing the resolve function seems unnecessary - adding a lot of responsibility on the function caller. It might be simpler if the function returns promise by itself

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved Promise inside the function

let disableSavedPaymentMethods =
newOptions
->getDictFromJson
->getBool("displaySavedPaymentMethods", true) &&
!(spmComponents->Array.includes(componentType))->not
fetchCustomerPaymentMethods(mountedIframeRef, disableSavedPaymentMethods, componentType)
fetchSessionTokens(mountedIframeRef)
resolve()
let customerPaymentMethodsPromise = Promise.make((resolve, _) => {
fetchCustomerPaymentMethods(
mountedIframeRef,
disableSavedPaymentMethods,
componentType,
resolve,
)
})
let sessionTokensPromise = Promise.make((resolve, _) => {
fetchSessionTokens(mountedIframeRef, resolve)
})
Promise.all([
paymentMethodsPromise,
customerPaymentMethodsPromise,
sessionTokensPromise,
])->then(_ => {
let msg = [("cleanUpPreMountLoaderIframe", true->JSON.Encode.bool)]->Dict.fromArray
preMountLoaderIframeDiv->Window.iframePostMessage(msg)
resolve()
})
})
->catch(_ => resolve())
->ignore
Expand Down
20 changes: 18 additions & 2 deletions src/hyper-loader/PaymentMethodsManagementElements.res
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,19 @@ let make = (
)
})

let fetchSavedPaymentMethods = (mountedIframeRef, disableSaveCards, componentType) => {
let fetchSavedPaymentMethods = (
mountedIframeRef,
disableSaveCards,
componentType,
resolvePromise,
) => {
if !disableSaveCards {
let handleSavedPaymentMethodsLoaded = (event: Types.event) => {
let json = event.data->Identity.anyTypeToJson
let dict = json->getDictFromJson
let isSavedPaymentMethodsData = dict->getString("data", "") === "saved_payment_methods"
if isSavedPaymentMethodsData {
resolvePromise()
let json = dict->getJsonFromDict("response", JSON.Encode.null)
let msg = [("savedPaymentMethods", json)]->Dict.fromArray
mountedIframeRef->Window.iframePostMessage(msg)
Expand All @@ -115,6 +121,8 @@ let make = (
handleSavedPaymentMethodsLoaded,
`onSavedPaymentMethodsLoaded-${componentType}`,
)
} else {
resolvePromise()
}
let msg =
[("sendSavedPaymentMethodsResponse", !disableSaveCards->JSON.Encode.bool)]->Dict.fromArray
Expand Down Expand Up @@ -208,7 +216,15 @@ let make = (
disableSavedPaymentMethods &&
!(expressCheckoutComponents->Array.includes(componentType))
) {
fetchSavedPaymentMethods(mountedIframeRef, false, componentType)
Promise.make((resolve, _) => {
fetchSavedPaymentMethods(mountedIframeRef, false, componentType, resolve)
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

same over here as well. Can fetchSavedPaymentMethods return promise instead of the caller having to do Promise.make and pass resolve function down as arg?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

moved the promise inside here as well

->then(_ => {
let msg = [("cleanUpPreMountLoaderIframe", true->JSON.Encode.bool)]->Dict.fromArray
preMountLoaderIframeDiv->Window.iframePostMessage(msg)
resolve()
})
->ignore
}
resolve()
})
Expand Down
Loading