[BUG] Violation of Lighthouse Best Practice — 'Registers an unload listener'

[NoelAbrahams]

Description/Screenshot

Steps to Reproduce

• OS/Browser: Chrome
• SDK Version [e.g. 22]: The script being loaded is this one: https://az416426.vo.msecnd.net/scripts/b/ai.2.min.js
• How you initialized the SDK: Using custom code

1. Open up a web page that loads AI.
2. Open up Chrome Developer Tools (F12)
3. Switch over to the Lighthouse Tab.
4. Click 'Generate Report'. I ran this for device 'Mobile'.
5. After the report completes, click on 'Best Practices' and then the violation is reported under 'General'.

Expected behavior

No violation

Additional context

Clicking on 'Learn More' takes you to this page: https://developers.google.com/web/updates/2018/07/page-lifecycle-api?utm_source=lighthouse&utm_medium=devtools#the-unload-event

[MSNev]

We register and listen to "unload", "beforeunload", "pagehide" and "visibilitychange" (this last one as of last release).

We need to listen to all possible events to support the full browser matrix and not just more modern browser that support the other events. As such we NEED to continue listening to the unload event so that we can flush any unsent event in ALL browsers.

[NoelAbrahams]

Interesting argument. We also load Google Analytics and Google Analytics 4.

No violations there.

[MSNev]

Their supported browser matrix is a lot smaller than ours https://support.google.com/analytics/answer/3541880?hl=en

[NoelAbrahams]

@MSNev that refers to browsers one can use to access the analytics dashboard. Of course the script itself will work on all versions of all browsers.

We register and listen to "unload", "beforeunload", "pagehide" and "visibilitychange"

This sounds like an expensive way to do things. Should be based on feature detection.

[MSNev]

We effectively are performing a lightweight form of feature detection if the event is available we hook it, otherwise we don't.
This is much more lightweight than having a more complete feature detection with feature tables identifying what can and can't be done and then causing additional testing and deployed dead code (for some browsers) due to multiple branches etc.

So this is the entire attach code required and and it's effectively called this 4 times this also attempts to list to the event at the window, body and document level, again this is to support all of the legacy browsers and platforms as not all support the events at the window level

Some of this is more complex than we would like, but it is required as we are providing support for IE 8+ with active end-user customers still actively using IE8 (and in some cases IE7 😥 ).

[MSNev]

FYI - We have now supplied an optional configuration settings to allow you to "request" that you don't want the "unload" event to be hooked #1683.

The reason for calling it a "request", is that if the runtime environment fails to hook any "unload" event (taking the excluded set into account), then it will still hook the default set of events.

[MSNev]

This will be release in the next release which is currently planned to be v2.7.1

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.