-
Notifications
You must be signed in to change notification settings - Fork 2k
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.
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
Stepper: Record calypso_signup_start
unconditionally => multiple logs
#94743
Stepper: Record calypso_signup_start
unconditionally => multiple logs
#94743
Conversation
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~93 bytes removed 📉 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. 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. Generated by performance advisor bot at iscalypsofastyet.com. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested it and the event is triggered with every refresh 👍
This is working fine! I got confused, is this the same as this? If so, I think I can close my PR then? |
@chriskmnds just noting that this PR will also fix this comment, since @renancarvalho yes, as pointed out in your PR 👍 |
@renancarvalho It's something like part of a long story we've been drilling around for a few weeks #94175 (and related PR #94248 ) It is an alternative to your PR (and any other approach that ignores dependencies like "extra props" from retriggering the event), so if we ship this then we can close yours. Shipping this means also closing @gabrielcaires' PR: #94248 I want to get everyone involved/aware around #94175 so nobody's surprised if multiple events triggering show up in discussions. I intend to follow up similarly with other events :-)
Thanks @escapemanuele! Cool :D |
Thank you all for reviewing! and for buying into this idea of "unconditional tracking" 🙈 |
Related to #94175
Fixes #94717
Proposed Changes
Records
calypso_signup_start
event unconditionally. This means it can get logged not only on the first step but on any of the dependencies changing.extraProps
coming fromuseSignupStartEventProps
are guaranteed now to be logged as they change through flow progression.Why are these changes being made?
Addressed #94175 (comment)
Testing Instructions
/setup/onboarding
calypso_signup_start
being recorded when logging in and on refreshing the pagePre-merge Checklist