diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/processing-step/index.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/processing-step/index.tsx index 2a1637d726d57..94c3aed5bf272 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/processing-step/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/processing-step/index.tsx @@ -13,7 +13,7 @@ import { } from '@automattic/onboarding'; import { useSelect } from '@wordpress/data'; import { useI18n } from '@wordpress/react-i18n'; -import { useEffect, useState } from 'react'; +import { useEffect, useState, useRef } from 'react'; import DocumentHead from 'calypso/components/data/document-head'; import { LoadingBar } from 'calypso/components/loading-bar'; import { LoadingEllipsis } from 'calypso/components/loading-ellipsis'; @@ -49,6 +49,28 @@ const ProcessingStep: React.FC< ProcessingStepProps > = function ( props ) { const [ hasEmptyActionRun, setHasEmptyActionRun ] = useState( false ); const [ destinationState, setDestinationState ] = useState( {} ); + /** + * There is a long-term bug here that the `submit` function will be called multiple times if we + * call `resetOnboardStoreWithSkipFlags` after the submit function (e.g.: exitFlow) to reset states + * that are listed as the dependencies of the `recordSignupComplete`, e.g.: goals, selectedDesign, etc. + * + * Here is a possible flow: + * 1. The Design Picker step submits, sets a pending action, and goes to this step + * 2. Run the pending action, and then submit first + * 3. The `submit` may trigger the `exitFlow` function that is defined by each flow + * 4. The `exitFlow` function may set another pending action, and call `resetOnboardStoreWithSkipFlags` function + * 5. The effect to call the `submit` runs again since the `recordSignupComplete` function changes + * + * It's also a reason why we have a hacky to set a pending action to return a Promise that is never resolved. + * + * To resolve this issue, we define a flag to avoid calling the submit function multiple times. + * + * Another way is to remove the recordSignupComplete function from the dependencies of the effect that + * is called when the hasActionSuccessfullyRun flag turns on. But it seems to be better to use an explicit + * flag to avoid the issue and describe it here. + */ + const isSubmittedRef = useRef( false ); + const recordSignupComplete = useRecordSignupComplete( flow ); useInterval( @@ -104,7 +126,10 @@ const ProcessingStep: React.FC< ProcessingStepProps > = function ( props ) { // As for hasActionSuccessfullyRun, in this case we submit the no action result. useEffect( () => { - if ( hasEmptyActionRun ) { + if ( hasEmptyActionRun && ! isSubmittedRef.current ) { + // Let's ensure the submit function is called only once, + // but only for the onboarding flow to mitigate risks. + isSubmittedRef.current = flow === 'site-setup' ? true : false; submit?.( {}, ProcessingResult.NO_ACTION ); } // eslint-disable-next-line react-hooks/exhaustive-deps @@ -112,7 +137,7 @@ const ProcessingStep: React.FC< ProcessingStepProps > = function ( props ) { // When the hasActionSuccessfullyRun flag turns on, run submit() and fire the sign-up completion event. useEffect( () => { - if ( hasActionSuccessfullyRun ) { + if ( hasActionSuccessfullyRun && ! isSubmittedRef.current ) { // We should only trigger signup completion for signup flows, so check if we have one. if ( availableFlows[ flow ] ) { availableFlows[ flow ]().then( ( flowExport ) => { @@ -134,6 +159,10 @@ const ProcessingStep: React.FC< ProcessingStepProps > = function ( props ) { wccom_from: getWccomFrom( destinationState ), } ); + // Let's ensure the submit function is called only once, + // but only for the onboarding flow to mitigate risks. + isSubmittedRef.current = flow === 'site-setup' ? true : false; + // Default processing handler. submit?.( destinationState, ProcessingResult.SUCCESS ); }