Direct localStorage/ sessionStorage access prevents usage from Worker context or Safari Private tab

[Tombarr]

WorkerGlobalScope (which ServiceWorkerGlobalScope inherits from) does not include localStorage or sessionStorage, but using @minimal-analytics@gav4 in a ServiceWorker context throws a ReferenceError because of this hard dependency. It also throws a QuotaExceededError in a Safari Private tab when attempting to call setItem.

Instead, using a wrapper like storage-factory would prevent these runtime errors. Developers in a Worker context would then need to manually set the Client ID cid value, i.e. by copying the clientId value on the main thread between localStorage and indexedDB.

[jahilldev]

Hi @Tombarr,

Thanks for reaching out! Interesting use case, you have here.

Adjusting the ga4 script to support use in non main thread contexts should be fairly trivial.

Feel free to open a PR, all contributions welcome 👍

[Tombarr]

Thanks @jahilldev, working on changes here: https://github.com/jahilldev/minimal-analytics/compare/main...Tombarr:minimal-analytics:main

Currently getting stuck on issues with yarn dependencies & sub-packages. I'll keep trying, but open to tips. I don't want to pollute your repo, maybe it's easier to keep package versions locked since you'll still have to build & publish on npm anyway?

lerna ERR! yarn exited 1 in '@minimal-analytics/shared'
lerna WARN complete Waiting for 2 child processes to exit. CTRL-C to exit immediately.
npm ERR! code 1
npm ERR! path /Documents/GitHub/minimal-analytics
npm ERR! command failed
npm ERR! command sh -c lerna exec -- yarn

npm ERR! 404 Not Found - GET https://registry.npmjs.org/@minimal-analytics%2fshared - Not found
npm ERR! 404 
npm ERR! 404  '@minimal-analytics/[email protected]' is not in this registry.
npm ERR! 404 
npm ERR! 404 Note that you can also install from a
npm ERR! 404 tarball, folder, http url, or git url.

[jahilldev]

Hey @Tombarr,

Ahh yeh, so I should really create a contributors doc..

The package @minimal-analytics/shared isn't published to the registry, it's only used during development.

You normally come across this issue when lerna isn't be run at any point during install. To fix this (I think), you'll need to ensure you're using yarn (for the workspaces) and do the following:

• Run yarn in the root (this should use lerna exec -- yarn)
• Run yarn build in the root (this should run lerna bootstrap --no-ci in the pre-build step)

If you're still having issues, let me know, and I'll checkout a fresh copy and put together a CONTRIBUTING.md doc.

Thanks for helping out! 👍

[Tombarr]

Thanks for the tip @jahilldev, I'm able to run tests on the shared package, Now I'm scratching my head at why the local @minimal-analytics@shared package isn't getting imported while running tests.

Once I get past this, I'll finalize my changes and put up a PR. I'll leave the package number updates up to you, then I'll get to work on #43 which should be a much smaller change.

tests/index.test.ts:1:51 - error TS2307: Cannot find module '@minimal-analytics/shared' or its corresponding type declarations.

1 import { clientKey, counterKey, sessionKey } from '@minimal-analytics/shared';

[jahilldev]

Hey @Tombarr,

This is great work! Let me know if you're still having issues. I'll Pull your branch and take a look if so 👌

[Tombarr]

Hey @jahilldev, almost done! I got the local package setup working and I'm able to build & test the ga4 package. I added a bunch more tests and an additional navigator.sendBeacon fallback to use fetch when XHR isn't available (i.e. ServiceWorker scope). The only issue now seems to be with the first visit flag. My hunch is it's an issue with mocks, but I'm working to figure out and fix.

BTW, any concerns with the package size? With the fallbacks, index.js not GZipped is up from 6,610 to 9,264 bytes (+40%), which might change the topline number in the README from ~2kb to ~3kb GZipped. On the plus side, once complete it should work in pretty much any browser scope (main thread, Web Worker & ServiceWorker).

[Tombarr]

@jahilldev Should be all set on PR #45. The first attempt failed CI due to unrelated build issues, looks like the second hasn't run yet but it should be good to go. I'm doing some final UAT with the build in my app and not seeing any issues, but understand if you want to run more tests since the changes are significant.

[jahilldev]

Hey @Tombarr,

Great! I'm travelling for work over the next few days but I'll try and have a thorough look when I get time / when I'm back. I've had a brief look at your PR (good work!), and I think we have a few opportunities to reduce the distributable size.

On a side note, I'd be really interested to know how you found contributing, any specific issues that you feel would've been useful in a document somewhere, etc.

Thanks again for the contribution!