IntersectionObserver is not defined dashboard error when accessing Analytics tab

[jamesozzie]

Bug Description

There have been reports of the below dashboard error, both occurring from Site Kit 1.31.0 users. This seems to occur only when clicking on the Analytics tab for one user while it only occurs on the primary dashboard tab for another.

One user reported that disconnecting Analytics results in no dashboard notice.

Awaiting further insights.

Can't find variable: IntersectionObserver

    in header
    in DashboardPageSpeed
    in div
    in Layout
    in div
    in LegacyDashboardSpeed
    in FilteredComponent
    in WithFilters(LegacyDashboardModule)
    in div
    in Row
    in div
    in Grid
    in div
    in DashboardApp
    in GoogleSitekitDashboard
    in RestoreSnapshots
    in ErrorHandler
    in Root

https://wordpress.org/support/topic/cant-find-variable-intersectionobserver/ | Open | Awaiting SH info
https://wordpress.org/support/topic/site-kit-encountered-an-error-32 | Resolved (by user, unknown cause) | SH info filled in although not complete
https://wordpress.org/support/topic/intersectionobserver-is-not-defined/#post-14401649 | Open | awaiting SH info
https://wordpress.org/support/topic/site-kit-encountered-an-error-34/ | Open | Awaiting SH info
https://wordpress.org/support/topic/fatal-error-3876/ | Open | SH info
https://wordpress.org/support/topic/fatal-error-3876/#post-14475607 | Resolved | SH info - Safari specific - MacOS Big Sur, version 11.4.

Additional Context

• SK 1.31.0
• From the impacted URL we have from one reporter only the Yoast plugin is evident, no obvious caching, optimization or other plugins

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

• Site Kit should provide a polyfill for the IntersectionObserver, which is loaded in case it isn't available. See these docs for reference.

Implementation Brief

• Add intersection-observer to our npm dependencies
• Add a new googlesitekit-polyfills.js entry
  • This module will only include feature tests and dynamically require polyfills as needed
  • Checks for intersection observer and load the polyfill asynchronously if the feature is not available
• Add this new entry as a dependency of our vendor asset so that it is automatically loaded with our dependencies
• Merge Add polyfill for intersection observer #3965

Test Coverage

• N/A

Visual Regression Changes

• N/A

QA Brief

Testing Site Kit

• Test intersection observer specific functionality – enable usage tracking, enable PSI and scroll down to view the widget – no error should occur

Browsers

• Test Site Kit using a modern browser (check for errors in the console related to polyfills or intersection observer)
• Test Site Kit using an older browser that does not support intersection observer (e.g. Safari 12.0 or below) – there should be no errors related to intersection observer

Changelog entry

• Fix potential error in older browsers that don't support IntersectionObserver.

[mxbclang]

This user reports that they're using Mac 10.11.6 OS X El Capitan and Safari version 11.1.2. I tried testing in Safari 11 in BrowserStack and am not able to replicate. Site Health is available.

[jamesozzie]

We've had another report of this today, troubleshooting ongoing. Added to the list of impacted users in original post.

[abdullah1908]

Couldn't find out any kind of conflict/error while testing in Safari.
cc: @bethanylang @jamesozzie

[abdullah1908]

As per this topic, user is not observing any issues in the incognito window. By checking SH info I can see there is a plugin NitroPack installed that might be the reason. Asking the user to disable the plugin & check again.

[jamesozzie]

This seems Safari specific, similar to #3255, as confirmed by 2/6 impacted users heighted in the issue description. Awaiting response from impacted users, follow up responses sent and testing to be performed today.

From checking the Intersection Observer API Safari doesn't provide full support yet, with many reporting similar issues in Stackoverflow.
https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API

One user stated that they enabled "prevent cross-site-tracking" within their Safari configuration.

[mxbclang]

@jamesozzie Checking in here – should this go over to L2 or are you and @abdullah1908 still working on it?

[jamesozzie]

@bethanylang Sounds good to me to escalate to L2. Alternatively we can check with the team whether the upcoming changes from #3255 would impact this issue.

[aaemnnosttv]

This seems to be related to our use of react-intersection-observer which was added in #3116.

I found a similar issue on the repo which also indicates that it isn't compatible out-of-the-box with Safari 11:
thebuilder/react-intersection-observer#469 (comment)

There is a section in the readme about how to add this:
https://github.com/thebuilder/react-intersection-observer#polyfill

I think this is sufficient to move the issue forward with – thanks all!

[aaemnnosttv]

@felixarntz we talked about conditionally relying on the intersection observer, but this may be more trouble than it's worth since we can't call hooks conditionally:

site-kit-wp/assets/js/modules/pagespeed-insights/components/dashboard/DashboardPageSpeed.js

Line 92 in 37e9cad

const [ trackingRef, inView ] = useInView( { triggerOnce: true, threshold: 0.25 } );

The library does offer an InView component instead which we could use conditionally but again, may be more problematic than it's worth.

There is the polyfill as documented on the repo – the cost of adding this to our bundle doesn't look too bad though. What do you think?

[eugene-manuilov]

@aaemnnosttv the problem with dynamic import is that webpack uses a static public path which is an empty string and tries to load the imported script from the current virtual directory. In our case, webpack loads dynamically imported scripts using http://{domain}/wp-admin/{imported-script} URLs instead of http://{domain}/wp-content/plugins/google-sitekit/dist/assets/js/{imported-script}. Unfortunately, I don't see how we can override it.

[aaemnnosttv]

@eugene-manuilov ah yes – I believe that's why in the past we had to set the public path on-the-fly before using a dynamic import. The base path should be already available via Site Kit's base data global:

[eugene-manuilov]

@aaemnnosttv unfortunately, the public path on the fly doesn't seem to work. Tried it on my local machine and I still see the empty public path string in the generated runtime.js file:

// runtime.js
...
/******/
/******/ 	// __webpack_public_path__
/******/ 	__webpack_require__.p = "";
/******/
...

[aaemnnosttv]

unfortunately, the public path on the fly doesn't seem to work. Tried it on my local machine and I still see the empty public path string in the generated runtime.js file

@eugene-manuilov I don't think setting it on the fly would cause it to be updated in the built runtime.js, instead it's changing it at runtime. We used to use this in a few places back when we were using chunks for a few things so it does work 🙂

I've created a quick PR to do this as a proof of concept but I also think it's good to go as-is. In development the added googlesitekit-polyfills.js was 3kb (850b in prod) and only does the dynamic require for the feature polyfill if needed so it's very lightweight.

I've updated the IB to use this, let me know what you think.

[eugene-manuilov]

I don't think setting it on the fly would cause it to be updated in the built runtime.js, instead it's changing it at runtime. We used to use this in a few places back when we were using chunks for a few things so it does work slightly_smiling_face

It works 🎉! IB ✔️

[eugene-manuilov]

@aaemnnosttv could you please add QAB?

[wpdarren]

QA Update: ⚠️

@aaemnnosttv can I have a sanity check on what I tested: the ticket mentions 'Analytics tab' and when I read a few of the support tickets they mention when disconnecting Analytics or PSI. I am a little confused that the QAB does not mention this. Anyway, what I did in a modern browser and Safari 11.1 is connected PSI and scrolled down to the widget on the dashboard and no errors appear in the FE or console. Just as a precaution, I also disconnected PSI and no errors appeared.

Is this all I have to do?

[aaemnnosttv]

@wpdarren on the current release, if you activate PSI in Safari 11.1 you should see this error

On the develop branch, no such error occurs

Feel free to try other pages too, but you can test it right on the main dashboard as long as PSI is active.

[wpdarren]

QA Update: ✅

When on Safari 11.1 and connecting PSI, I was able to recreate the issue on the latest release. When I switched to develop, the error no longer appears when the module is connecting or displaying the insights. While on the branch, I disconnected and connected a few other modules and no error message appeared.