diff --git a/client/landing/stepper/declarative-flow/connect-domain.ts b/client/landing/stepper/declarative-flow/connect-domain.ts index 228b46649e107..3935f9508fe16 100644 --- a/client/landing/stepper/declarative-flow/connect-domain.ts +++ b/client/landing/stepper/declarative-flow/connect-domain.ts @@ -1,7 +1,7 @@ import { CONNECT_DOMAIN_FLOW } from '@automattic/onboarding'; import { useSelect, useDispatch } from '@wordpress/data'; import { translate } from 'i18n-calypso'; -import { useEffect } from 'react'; +import { useEffect, useMemo } from 'react'; import { useFlowLocale } from 'calypso/landing/stepper/hooks/use-flow-locale'; import { domainMapping } from 'calypso/lib/cart-values/cart-items'; import { triggerGuidesForStep } from 'calypso/lib/guides/trigger-guides-for-step'; @@ -109,12 +109,17 @@ const connectDomain: Flow = { useTracksEventProps() { const { domain, provider } = useDomainParams(); - return { - [ STEPPER_TRACKS_EVENT_STEP_NAV_SUBMIT ]: { - domain, - provider, - }, - }; + return useMemo( + () => ( { + eventsProperties: { + [ STEPPER_TRACKS_EVENT_STEP_NAV_SUBMIT ]: { + domain, + provider, + }, + }, + } ), + [ domain, provider ] + ); }, useStepNavigation( _currentStepSlug, navigate ) { const flowName = this.name; diff --git a/client/landing/stepper/declarative-flow/internals/components/step-route/hooks/use-step-route-tracking/index.tsx b/client/landing/stepper/declarative-flow/internals/components/step-route/hooks/use-step-route-tracking/index.tsx index 27d70c6e5daf0..2456832d14e44 100644 --- a/client/landing/stepper/declarative-flow/internals/components/step-route/hooks/use-step-route-tracking/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/components/step-route/hooks/use-step-route-tracking/index.tsx @@ -51,8 +51,10 @@ export const useStepRouteTracking = ( { flow, stepSlug, skipStepRender }: Props const pathname = window.location.pathname; const flowVariantSlug = flow.variantSlug; const flowName = flow.name; + const customProperties = flow.useTracksEventProps?.(); + const isLoading = customProperties?.isLoading; const signupStepStartProps = useSnakeCasedKeys( { - input: flow.useTracksEventProps?.()?.[ STEPPER_TRACKS_EVENT_SIGNUP_STEP_START ], + input: customProperties?.eventsProperties[ STEPPER_TRACKS_EVENT_SIGNUP_STEP_START ], } ); /** @@ -71,7 +73,8 @@ export const useStepRouteTracking = ( { flow, stepSlug, skipStepRender }: Props useEffect( () => { // We wait for the site to be fetched before tracking the step route. - if ( ! hasRequestedSelectedSite ) { + // And if `isLoading` is true, it means the flow is still loading custom properties. + if ( ! hasRequestedSelectedSite || isLoading ) { return; } @@ -134,5 +137,5 @@ export const useStepRouteTracking = ( { flow, stepSlug, skipStepRender }: Props // We also leave out pathname. The respective event (calypso_page_view) is recorded behind a timeout and we don't want to trigger it again. // - window.location.pathname called inside the effect keeps referring to the previous path on a redirect. So we moved it outside. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [ flowName, hasRequestedSelectedSite, stepSlug, skipStepRender ] ); + }, [ flowName, hasRequestedSelectedSite, stepSlug, skipStepRender, isLoading ] ); }; diff --git a/client/landing/stepper/declarative-flow/internals/hooks/use-sign-up-start-tracking/index.tsx b/client/landing/stepper/declarative-flow/internals/hooks/use-sign-up-start-tracking/index.tsx index ded48696fdd71..f9cb2b411d875 100644 --- a/client/landing/stepper/declarative-flow/internals/hooks/use-sign-up-start-tracking/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/hooks/use-sign-up-start-tracking/index.tsx @@ -21,8 +21,10 @@ export const useSignUpStartTracking = ( { flow }: Props ) => { const flowName = flow.name; const flowVariant = flow.variantSlug; const isSignupFlow = flow.isSignupFlow; + const customPropsConfig = flow.useTracksEventProps?.(); + const isLoading = customPropsConfig?.isLoading; const signupStartEventProps = useSnakeCasedKeys( { - input: flow.useTracksEventProps?.()[ STEPPER_TRACKS_EVENT_SIGNUP_START ], + input: customPropsConfig?.eventsProperties[ STEPPER_TRACKS_EVENT_SIGNUP_START ], } ); /** @@ -44,7 +46,7 @@ export const useSignUpStartTracking = ( { flow }: Props ) => { }, [ isSignupFlow, flowName ] ); useEffect( () => { - if ( ! isSignupFlow ) { + if ( ! isSignupFlow || isLoading ) { return; } @@ -56,5 +58,5 @@ export const useSignUpStartTracking = ( { flow }: Props ) => { ...( flowVariant && { flow_variant: flowVariant } ), }, } ); - }, [ isSignupFlow, flowName, ref, signupStartEventProps, flowVariant ] ); + }, [ isSignupFlow, flowName, ref, signupStartEventProps, isLoading, flowVariant ] ); }; diff --git a/client/landing/stepper/declarative-flow/internals/hooks/use-sign-up-start-tracking/test/index.tsx b/client/landing/stepper/declarative-flow/internals/hooks/use-sign-up-start-tracking/test/index.tsx index 7554fd9656e98..d3ed2a5de96b5 100644 --- a/client/landing/stepper/declarative-flow/internals/hooks/use-sign-up-start-tracking/test/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/hooks/use-sign-up-start-tracking/test/index.tsx @@ -88,7 +88,9 @@ describe( 'useSignUpTracking', () => { flow: { ...signUpFlow, useTracksEventProps: () => ( { - [ STEPPER_TRACKS_EVENT_SIGNUP_START ]: { extra: 'props' }, + eventsProperties: { + [ STEPPER_TRACKS_EVENT_SIGNUP_START ]: { extra: 'props' }, + }, } ), } satisfies Flow, queryParams: { ref: 'another-flow-or-cta' }, @@ -136,7 +138,9 @@ describe( 'useSignUpTracking', () => { flow: { ...signUpFlow, useTracksEventProps: () => ( { - [ STEPPER_TRACKS_EVENT_SIGNUP_START ]: { extra: 'props' }, + eventsProperties: { + [ STEPPER_TRACKS_EVENT_SIGNUP_START ]: { extra: 'props' }, + }, } ), } satisfies Flow, } ); diff --git a/client/landing/stepper/declarative-flow/internals/hooks/use-step-navigation-with-tracking/index.ts b/client/landing/stepper/declarative-flow/internals/hooks/use-step-navigation-with-tracking/index.ts index 8386604c23b76..c761a7cc0790b 100644 --- a/client/landing/stepper/declarative-flow/internals/hooks/use-step-navigation-with-tracking/index.ts +++ b/client/landing/stepper/declarative-flow/internals/hooks/use-step-navigation-with-tracking/index.ts @@ -1,6 +1,6 @@ import { OnboardSelect } from '@automattic/data-stores'; import { useSelect } from '@wordpress/data'; -import { useCallback, useMemo, useRef } from '@wordpress/element'; +import { useCallback, useMemo } from '@wordpress/element'; import { STEPPER_TRACKS_EVENT_STEP_NAV_EXIT_FLOW, STEPPER_TRACKS_EVENT_STEP_NAV_GO_BACK, @@ -36,8 +36,6 @@ export const useStepNavigationWithTracking = ( { }, [] ); const tracksEventPropsFromFlow = flow.useTracksEventProps?.(); - const tracksEventPropsFromFlowRef = useRef( tracksEventPropsFromFlow ); - tracksEventPropsFromFlowRef.current = tracksEventPropsFromFlow; const handleRecordStepNavigation = useCallback( ( { @@ -56,11 +54,16 @@ export const useStepNavigationWithTracking = ( { providedDependencies: dependencies, additionalProps: { ...( eventProps ?? {} ), - ...( tracksEventPropsFromFlowRef.current?.[ event ] ?? {} ), + // Don't add eventProps if `useTracksEventProps` is still loading. + // It's not tight, but it's a trade-off to avoid firing events with incorrect props. + // It's a tiny edge case where the use navigates before this hook is ready. + ...( tracksEventPropsFromFlow?.isLoading + ? undefined + : tracksEventPropsFromFlow?.eventsProperties?.[ event ] ?? {} ), }, } ); }, - [ intent, goals, currentStepRoute, flow ] + [ intent, tracksEventPropsFromFlow, goals, currentStepRoute, flow ] ); return useMemo( diff --git a/client/landing/stepper/declarative-flow/internals/types.ts b/client/landing/stepper/declarative-flow/internals/types.ts index 5608f989c2e16..3c39e23d6126e 100644 --- a/client/landing/stepper/declarative-flow/internals/types.ts +++ b/client/landing/stepper/declarative-flow/internals/types.ts @@ -118,7 +118,13 @@ export type UseSideEffectHook< FlowSteps extends StepperStep[] > = ( * Can pass any properties that should be recorded for the respective events. */ export type UseTracksEventPropsHook = () => { - [ key in ( typeof STEPPER_TRACKS_EVENTS )[ number ] ]?: Record< string, string | number | null >; + /** + * This flag is needed to indicate that the custom props are still loading. And the return value will be ignored until it's false. + */ + isLoading?: boolean; + eventsProperties: Partial< + Record< ( typeof STEPPER_TRACKS_EVENTS )[ number ], Record< string, string | number | null > > + >; }; export type Flow = { diff --git a/client/landing/stepper/declarative-flow/onboarding.ts b/client/landing/stepper/declarative-flow/onboarding.ts index dea3126890ae4..a8d3f35f19c6c 100644 --- a/client/landing/stepper/declarative-flow/onboarding.ts +++ b/client/landing/stepper/declarative-flow/onboarding.ts @@ -50,7 +50,7 @@ const onboarding: Flow = { isSignupFlow: true, __experimentalUseBuiltinAuth: true, useTracksEventProps() { - const isGoalsAtFrontExperiment = useGoalsFirstExperiment()[ 1 ]; + const [ isLoading, isGoalsAtFrontExperiment ] = useGoalsFirstExperiment(); const userIsLoggedIn = useSelector( isUserLoggedIn ); const goals = useSelect( ( select ) => ( select( ONBOARD_STORE ) as OnboardSelect ).getGoals(), @@ -63,21 +63,25 @@ const onboarding: Flow = { return useMemo( () => ( { - [ STEPPER_TRACKS_EVENT_SIGNUP_START ]: { - is_goals_first: isGoalsAtFrontExperiment.toString(), - ...( isGoalsAtFrontExperiment && { step: 'goals' } ), - is_logged_out: initialLoggedOut.current.toString(), - }, - [ STEPPER_TRACKS_EVENT_SIGNUP_STEP_START ]: { - ...( isGoalsAtFrontExperiment && { + isLoading, + eventsProperties: { + [ STEPPER_TRACKS_EVENT_SIGNUP_START ]: { is_goals_first: isGoalsAtFrontExperiment.toString(), - } ), - ...( initialGoals.current.length && { - goals: initialGoals.current.join( ',' ), - } ), + ...( isGoalsAtFrontExperiment && { step: 'goals' } ), + is_logged_out: initialLoggedOut.current.toString(), + }, + + [ STEPPER_TRACKS_EVENT_SIGNUP_STEP_START ]: { + ...( isGoalsAtFrontExperiment && { + is_goals_first: 'true', + } ), + ...( initialGoals.current.length && { + goals: initialGoals.current.join( ',' ), + } ), + }, }, } ), - [ isGoalsAtFrontExperiment, initialGoals, initialLoggedOut ] + [ isGoalsAtFrontExperiment, initialLoggedOut, initialGoals, isLoading ] ); }, useSteps() { diff --git a/client/landing/stepper/declarative-flow/tailored-ecommerce-flow.ts b/client/landing/stepper/declarative-flow/tailored-ecommerce-flow.ts index e91a647c464df..4ab5704f5b290 100644 --- a/client/landing/stepper/declarative-flow/tailored-ecommerce-flow.ts +++ b/client/landing/stepper/declarative-flow/tailored-ecommerce-flow.ts @@ -54,7 +54,10 @@ const ecommerceFlow: Flow = { [] ); - return useMemo( () => ( { [ STEPPER_TRACKS_EVENT_SIGNUP_START ]: { recur } } ), [ recur ] ); + return useMemo( + () => ( { eventsProperties: { [ STEPPER_TRACKS_EVENT_SIGNUP_START ]: { recur } } } ), + [ recur ] + ); }, useSteps() { const steps = [