Support loading in useTracksEventProps (#98219)

Co-authored-by: vykes-mac <knrymcleish@gmail.com>

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;

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 ] );
 };

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 ] );
 };

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,
 			} );

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(

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 = {

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() {

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 = [