search

snowplow / snowplow-javascript-tracker

Snowplow event tracker for client-side and server-side JavaScript. Add analytics to your websites, web apps and servers.
http://snowplowanalytics.com
BSD 3-Clause "New" or "Revised" License
555 stars 222 forks source link

ResizeObserver cannot be initialized if document.body doesn't exist yet #1311

Closed AngusMorton closed 4 months ago

AngusMorton commented 6 months ago

We stream our page in and initialize Snowplow in the <head>, so it's possible for the tracker to start before the <body> has been sent if the <body> doesn't exist when Snowplow initializes it will throw an exception because document.body doesn't exist when it tries to set up the ResizeObserver.

image

TypeError: Failed to execute 'observe' on 'ResizeObserver': parameter 1 is not of type 'Element'.

Specifically, the issue is here where we assume document.body is defined.

Possible Fix Maybe we should use the resize event instead of a ResizeObserver. It has better support on old browsers and is potentially closer to what we want to know (when the window innerHeight and innerWidth have changed), but we may need to implement some kind of debounce.

Alternatively, we could just guard the initializeResizeObserver code with an if (document.body).

I'm happy to contribute either of those changes.

Workaround Using <script defer src="..."> instead of <script async src="..."> works around the issue because the script gets evaluated after the DOM is loaded.

Snowplow Version 3.23.0

matus-tomlein commented 5 months ago

Hi @AngusMorton, thanks for the report and sorry for getting back to you late!

Using the resize event instead of ResizeObserver does make sense to me and could also fix issue #1306 that we haven't been able to reproduce so far. I can't think of a reason not to use the resize event over the ResizeObserver and as you mentioned it is compatible with more browsers.

Adding debouncing might be a good idea too to keep the performance too.

If you're happy to implement this change, we would really appreciate the contribution!

EricHeitmuller-Gusto commented 5 months ago

Sent a potential stopgap PR until such time as this can be changed to use the resize event.

EricHeitmuller-Gusto commented 4 months ago

Hey folks, would it be possible to do a patch release for the stopgap change I've contributed? We're currently blocked on it.

matus-tomlein commented 4 months ago

Sorry for the delay @EricHeitmuller-Gusto, we'll aim to release it tomorrow!

EricHeitmuller-Gusto commented 4 months ago

@matus-tomlein no apologies necessary - just needed something to tell my team 😅