fathom-client

fathom-client copied to clipboard

This PR adds some warnings in each of the methods available from the client, to let the user know when Fathom was not detected and that the methods will fire when Fathom loads.

Thanks @EnMod! I'm a little hesitant to create console noise, since this could potentially happen frequently (depending on the order that scripts load on the page and whether they are async/deferred). Can you share a bit about the motivation for adding these?

Sure, @derrickreimer, thanks for your reply! I have some ideas/more questions here as well:

Motivation

I was motivated to add these after noticing that a failure to load Fathom wouldn't be known to a dev when the fathom-client's methods (e.g.: trackGoal()) were called. With these added warnings, a dev can see if their methods are not submitting data to the Fathom backend due to being queued for execution, rather than due to some error in the network or elsewhere.

About frequency of warnings

Wouldn't the warnings only fire if the method containing them is explicitly called without Fathom being loaded? For example, if I have trackGoal('ID', 0) set to execute on click or scroll, the warning wouldn't fire until that specific click or scroll, right?

I can definitely see issues with on-load tracking though, where it's the most likely that not all scripts have loaded yet and could block Fathom. However, in those cases I imagine there would be few calls (if not just one call) for each method, so I'm thinking the noise would be somewhat low 🤔

Got it, makes sense. Seems like there are two scenarios here where the warning would get triggered:

1. A load call and trackPageview call are made back-to-back and the page view is queued until the fathom script finishes loading.
2. The load call is forgotten and trackPageview (or trackGoal) are called, commands are queued, and ultimately never executed since the script will never actually load.

I don't think we want to warn under scenario (1). But, I do think we should warn if a call (trackPageview, etc) is made before load is called. We could store this in a loadWasCalled variable and only warn if it's false. What do you think?

Oh! I like that idea (storing the "was called" state in a variable), I think it makes sense to use to avoid unnecessary calls.