Stepper: Support loading in useTracksEventProps

[alshakero]

Fixes: p1736504786087909/1736448746.488719-slack-C085HCWCEDN

Blocked by: #98097

Proposed Changes

The new part of Stepper API, useTracksEventProps, is young and needs some honing. Because of the way React works, an event is fired every time the return value of this hook changes. As opposed to picking up the most recent value when firing the event.

To fix that, I added an isLoading flag to the hook. When this flag is true, the hook return value will not be considered and the event will not fire until it flips to false. This is a tradeoff because if the loading takes too long, the event order may be messed up. There is one more possible solution, which to remove the hook and turn it into an unchangeable property of the flow to avoid these cases, but that would miss a key feature (loading things then altering events).

Another risk is if all of these conditions are met:

1. A flow customizes calypso_signup_start using the hook.
2. The customization needs time load.
3. They do not customize calypso_signup_step_start as well.

The calypso_signup_step_start will fire before calypso_signup_start. This is the only risky combo.

Why are these changes being made?

To avoid duplicated events.

Testing Instructions

1. Go to 22138-explat-experiment
2. Save the treatment bookmarklet.
3. While incognito, go to Calypso My Home / (don't go to the flow URL, the experiment assignment will be cached).
4. Open DevTools > Network and filter by calypso_signup_start.
5. Click the bookmarklet.
6. Go to the flow /setup/onboarding.
7. You should see a single instance of calypso_signup_start.
8. Go on to sign up.
9. After auth, you shouldn't see one more instance of calypso_signup_start.

[github-actions]

Calypso Live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-129522

Jetpack Cloud live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-129522&env=jetpack

Automattic for Agencies live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-129522&env=a8c-for-agencies

[matticbot]

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~25 bytes added 📈 [gzipped])

name           parsed_size           gzip_size
entry-stepper        +81 B  (+0.0%)      +25 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~62 bytes added 📈 [gzipped])

name                     parsed_size           gzip_size
connect-domain                 +46 B  (+0.0%)      +23 B  (+0.0%)
onboarding-flow                +27 B  (+0.1%)      +29 B  (+0.4%)
tailored-ecommerce-flow        +19 B  (+0.3%)      +10 B  (+0.4%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

[chriskmnds]

Posting here so we can keep a discussion.

Cool thinking! I've nearly completely lost hope in this API, at least in its current form, and primarily from the way that we pick the events to guide analyses. But having an async isLoading prop that propagates this way can indeed create misalignment. My feeling is this is not insignificant risk and it also creates ambiguity/trust issues with the API:

Another risk is if all of these conditions are met:

1. A flow customizes calypso_signup_start using the hook.
2. The customization needs time load.
3. They do not customize calypso_signup_step_start as well.

The calypso_signup_step_start will fire before calypso_signup_start. This is the only risky combo.

So a usage that for whatever reason (common reason apparently, like an experiment running) will need to customise all the events handled by the API to ensure there is correct order. Is this correct, from what you are saying?

Just thinking about the above, assuming correct, it's almost like this prop doesn't accomplish anything more than not passing custom props until isLoading is true:

...( ! isLoading && {
    [ STEPPER_TRACKS_EVENT_SIGNUP_START ]: {
        is_goals_first: isGoalsAtFrontExperiment.toString(),
        ...( isGoalsAtFrontExperiment && { step: 'goals' } ),
        is_logged_out: ( ! userIsLoggedIn ).toString(),
    }, 
    ...
} )

If the two are equivalent, then I'd consider this instead and not have it as part of the API. It feels more ambiguous when something's part of the API but with conditions of use.

I wonder if other caveats may exist too, but I've been away from this for quite a while to have perspective 🤔

[alshakero]

Hi Christos! Long time no see.

I've nearly completely lost hope in this API, at least in its current form, and primarily from the way that we pick the events to guide analyses

Totally on the same page here. I'll reconsider this once I'm done with pdDR7T-22t-p2.

it's almost like this prop doesn't accomplish anything more than not passing custom props until isLoading is true:

That's not entirely correct. When the prop is true, the event does not fire at all. In the snippet you share, the event will fire but without the customizations passed by the flow..