Support Web Worker and ServiceWorker scope

[Tombarr]

Add support for Web Worker and ServiceWorker contexts with the following changes

• When navigator.sendBeacon isn't available, default to XMLHttpRequest (Worker) or fetch (ServiceWorker)
• When sessionStorage or localStorage isn't available (i.e. Worker/ ServiceWorker), use an in-memory Map and warn that clientId is not persisted
• Allow use of global self or window, depending on context
• Don't set window or document scoped events when not available
• Because localStorage isn't available in a Worker scope, per Unable to override default parameters #43, allow param overrides, i.e. to pull clientId from IndexedDB

[Tombarr]

----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
----------|---------|----------|---------|---------|-------------------
All files |   96.58 |    77.25 |     100 |   96.49 |                   
 index.ts |   96.46 |    77.25 |     100 |   96.39 | 283,305,393,403   
 model.ts |     100 |      100 |     100 |     100 |                   
----------|---------|----------|---------|---------|-------------------
Jest: "global" coverage threshold for branches (78%) not met: 77.25%

Looks like CI ran well this time, but failed due to test coverage. I'll take a look at covering additional branches within ga4

[jahilldev]

Good work! This looks great 👍

Just a few initial comments, and I believe you'll need to either run eslint . --fix or setup format on save for your IDE using ESLint & Prettier to get the formatting just right.

I'm also going to have a think about any other opportunities we might have to reduce the output file size a bit.

[jahilldev]

issue: this should be across multiple lines according to Prettier 👍

Suggested change

	function getEventMeta({ type = '', event }: Pick<IProps, 'type' | 'event'>): [string, string][] {
	function getEventMeta({
	type = '',
	event,
	}: Pick<IProps, 'type' | 'event'>): [string, string][] {

[Tombarr]

I can't seem to get these configs loaded into VSCode with the Prettier extension. I'll look for a CLI command soon. Might be worth adding as a pre-commit hook if it can be automated.

[jahilldev]

This might be obvious, but just to make sure, it's worth checking these:

Format on save:

Formatter:

[jahilldev]

question: Is there a reason we're not using fetch() as the primary fallback for navigator.sendBeacon()? Is there a need to use XMLHttpRequest here? Is there an opportunity to save some bytes here by going down that route?

[Tombarr]

Although fetch with the keepalive property is a fallback for navigator.sendBeacon, it's not available on any version of Firefox. Synchronous XHR is more broadly supported, so it's the preferred fallback. See https://developer.mozilla.org/en-US/docs/Web/API/Fetch#keepalive

[jahilldev]

suggestion: Although we're not making use of the Promise returned via fetch() here (yet), it's still probably worth returning one here for future sake. Can we rework this so we can return & resolve the Promise used within this functions scope?

[Tombarr]

Good work! This looks great 👍

Just a few initial comments, and I believe you'll need to either run eslint . --fix or setup format on save for your IDE using ESLint & Prettier to get the formatting just right.

I'm also going to have a think about any other opportunities we might have to reduce the output file size a bit.

Thanks for the suggestions, I (hopefully) brought test coverage high enough now that it'll pass CI. Returning and passing through Promise will be a larger refactor that I'll get to when time permits.

[jahilldev]

suggestion: Do we need this condition here? It seems like sendBeaconFetch() can handle an undefined value for data, resulting in an empty body value. Any thoughts?

The same goes for this condition and this one.

[Tombarr]

I think I was writing to the tests, because without this the toBeCalledWith need to explicitly include undefined. From what I can tell, jest-extended is needed for expect.anything() to include undefined or include some other hack.

https://stackoverflow.com/questions/47027011/using-expect-anyobject-or-expect-anything-does-not-work-to-match-undefi

[jahilldev]

Ah I see, if we know in the test case whether data will be undefined (or something else), couldn't we do one of the following:

// data is defined in test case
expect(sendBeaconFetch).toBeCalledWith(expect.any(String), stubOfData);

// data is undefined in test case
expect(sendBeaconFetch).toBeCalledWith(expect.any(String), undefined);

I may be missing something, but we should be able to do something similar I believe. Ideally we'd also want an explicit value for each, so rather than using expect.any() we pass in the stub or test value.

What do you think?

[jahilldev]

question: This seems to be very similar to the following type, can we re-use that instead?

minimal-analytics/packages/shared/src/values.ts

Line 10 in 1a32381

type EventParams = Record<string, ParamValue> | [string, ParamValue][];