MWPW-142878 add events for milo post lcp & milo delayed

[vhargrave]

Resolves: MWPW-142878

Test URLs:

• Before: https://main--milo--adobecom.hlx.page/?martech=off
• After: https://vhargrave-MWPW-142878-events--milo--adobecom.hlx.page/?martech=off

This branch shows how we'll listen to the two events to load both additional analytics and then get the primary cta url which we redirect to.

• https://github.com/adobecom/express-milo/pull/56/files
• https://mwpw-158182-google-yolo--express-milo--adobecom.hlx.page/express/colors/gunmetal?milolibs=vhargrave-MWPW-142878-events

[aem-code-sync]

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

• Re-run PSI checks
• Re-sync branch

Commits

• ab52103 ✅ (latest)
• 87f8e46 ✅
• 229a056 ✅
• c15b8ce ✅
• 6ad905e ✅
• a52896b ✅
• 528f8dd ✅
• 08f57aa ✅
• 90ba467 ✅
• 092c97a ✅

[aem-code-sync]

	Page	Scores	Audits	Google
📱	/?martech=off
🖥️	/?martech=off

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.24%. Comparing base (264c998) to head (ab52103).
Report is 51 commits behind head on stage.

Additional details and impacted files

@@            Coverage Diff             @@
##            stage    #2881      +/-   ##
==========================================
+ Coverage   95.89%   96.24%   +0.34%     
==========================================
  Files         173      236      +63     
  Lines       46316    54255    +7939     
==========================================
+ Hits        44415    52218    +7803     
- Misses       1901     2037     +136     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mokimo]

I'm not particularly fond of the idea of consumers hooking into the milo rendering pipeline.
This event previously has been removed https://github.com/adobecom/milo/pull/1282/files#diff-28218f239d8d9c4d41e1924854440b50964619e56058fdd0ae243ff5ff6c616dL126 I can imagine with a tight coupling, this would have broken consumer code.

Can you not run your code after loadArea? https://github.com/adobecom/bacom/blob/stage/scripts/scripts.js#L189

Also to add to this postLCP might not really be postLCP. This method might go away or get refactored - from a performance standpoint, we also have the 2nd section popping up as LCP or other elements, currently we have a very late LCP due to congestion shortly after loading the first section. We can migrate the event, but it makes change harder if you have consuming code relying on something that comes from milo core.

[vhargrave]

I'm not particularly fond of the idea of consumers hooking into the milo rendering pipeline. This event previously has been removed https://github.com/adobecom/milo/pull/1282/files#diff-28218f239d8d9c4d41e1924854440b50964619e56058fdd0ae243ff5ff6c616dL126 I can imagine with a tight coupling, this would have broken consumer code.

Can you not run your code after loadArea? https://github.com/adobecom/bacom/blob/stage/scripts/scripts.js#L189

Also to add to this postLCP might not really be postLCP. This method might go away or get refactored - from a performance standpoint, we also have the 2nd section popping up as LCP or other elements, currently we have a very late LCP due to congestion shortly after loading the first section. We can migrate the event, but it makes change harder if you have consuming code relying on something that comes from milo core.

Hey @mokimo ,
Especially regarding the load delayed event, no we cannot. We have to execute the code before the google-yolo code on your end runs, and this is the only way to hook ourselves in.
To be honest I also disagree with what you're saying about clients being able to hook themselves in. I think this is actually very useful and allows important things to be done at the right time. Otherwise you're forcing clients as you said to run their code before or after loadArea is called. There are times though, when code must be executed during because it's a long wait in between.
Yes, we can't remove without breaking things, but that's kind of the case with most things in utils no?
I'd conclude by saying that we discussed this with the previous milo core team at the beginning of this migration. -https://wiki.corp.adobe.com/pages/viewpage.action?pageId=3091611394

[npeltier]

@vhargrave @mokimo i think i stand in the middle :) i agree with Victor that giving hooks to consumer is great, but with Okan that running code before some pretty structural milo processes are run might lead to surprises.
another thing: is it something we could do in loadPostLCP to cut event dispatch from main thread with timeout?

[vhargrave]

For fresh eyes looking at the PR, I've talked with with Okah & Rares and agreed to remove the events and instead add a callback to the google login function due to the concern of keeping these events maintainable.
Maybe we can can come back to the topic of hooks for consumers at some later point in time though.
cc: @mokimo @narcis-radu @npeltier @overmyheadandbody @fullcolorcoder

[overmyheadandbody]

I've removed the Ready for Stage label, as I didn't see any QA performed for this. We usually get either @SilviuLCF or @NadiiaSokolova involved to test and apply the label. The story is on a different board, so if you have QEs of your own, they can definitely help out, but I want to avoid pushing untested code.

[github-actions]

This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR.