Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shipping Countly In a WebWorker Context #382

Closed
whizzzkid opened this issue Mar 8, 2023 · 11 comments
Closed

Shipping Countly In a WebWorker Context #382

whizzzkid opened this issue Mar 8, 2023 · 11 comments

Comments

@whizzzkid
Copy link

Hello Team

We are using countly (wrapped inside [ignite-metrics](https://github.com/ipfs-shipyard/ignite-metrics/)) in [ipfs-companion](https://github.com/ipfs/ipfs-companion). We are in process of migrating to Chrome manifest v3 and are facing issues with bundling countly-sdk-web with respective shims and browserfied counterparts in a webworker context.

Manifest V3 expects the background page to be a webworker which means no access to DOM/window objects and using only fetch api to request resources. However, countly-sdk-web does not ship with tree-shakable modules, which would remove direct dependency on window object. As we're not using all the features of the SDK, and it gets bundled in it's entirety.

I see #75 wanted to tackle something along those lines but has not seen much traction. Any guidance on this would be appreciated. In the meantime, we've resolved to including the entire sdk on pages with window context.

Thanks

@ArtursKadikis
Copy link
Member

Hello. Which features are you using in your integration?
The window object is used for many things in the SDK so it might not be removed even if the SDK was tree-shakable.
If I understand correctly, the main problem in your case is that there is no window object in the context that you want to run the SDK in? Would your problem be solved if there was some kind of "switch" in the SDK that would make the SDK not use the window object?

@whizzzkid
Copy link
Author

Thanks @ArtursKadikis, if there was a switch that would be a nice short-term fix, whether it meets the expectation can only be validated in the webworker context. WebWorkers also don't allow XMLHttpRequest i.e. only fetch would work.

The reason this will be short-term fix is because it will still bloat the bundles with all the unreachable code. So the ideal solution would be some sort of modularization that would allow for tree-shaking all unreachable code.

@BigLep
Copy link

BigLep commented Mar 8, 2023

Also @ArtursKadikis, just so it's clear, any Countly users who use Countly within browser extensions are going to hit this same issue with the MV3 migration that is underway in the Chromium ecosystem. Addressing this issue will be a benefit beyond @whizzzkid and the ipfs-companion project. Thanks a lot.

@BigLep
Copy link

BigLep commented Mar 10, 2023

@ArtursKadikis : is there any sense of timeline for addressing this? Some info (even if rough) is better than none. For example would this happen in a month, in 2023Q2, by end of the year?

I'm asking so we can plan our work accordingly. I'd ideally like to be staying on countly SDKs but given the last shared dates in March of Chrome webstore rejecting extensions that aren't MV3 compatible, we may have to roll our own client SDK.

@SgtPooki
Copy link

@ArtursKadikis the main thing I think many users would like to see is a version of the countly sdk that works in both node and the web. i.e. an ESM module, that doesn't include side-effects (not kicking off the IIFE).

However, for the sake of getting this resolved; Simply removing the IIFE, and using dependency injection should resolve many of our current problems as well as future issues without being too much of a burden on your team.

@ArtursKadikis
Copy link
Member

@SgtPooki I'm not sure if that helps your specific usecase, but we already have a SDK that targets specifically NodeJS:
https://github.com/Countly/countly-sdk-nodejs

@BigLep on the timelines this currently seems more of a Q2 - Q3 kind of thing. This would include taking care of the window object and XMLHttpRequest that we currently use. This probably would not include any kind of tree-shaking type of functionality. That would probably require a more involved rework of the SDK which would break backwards compatibility in major ways.

@SgtPooki
Copy link

@ArtursKadikis any update on the removal of the window object and making the web SDK more webworker friendly?

@turtledreams
Copy link
Contributor

@SgtPooki Unfortunately, other priorities took over, and we did not have time to focus on web SDK for a while. Currently, we are looking into creating a new JS SDK that would be able to work in broader contexts.
Which features are you currently using?

@whizzzkid
Copy link
Author

@turtledreams thanks for your feedback, I patched the sdk like so: https://github.com/ipfs/ipfs-companion/pull/1232/files#diff-a05913488f8fc1e2aa8f1c8f051438b7f05e9dca1ad8d185b793d48dfa6099cb

This seems to be able to ship metrics to countly. What doesn't work:

  • anything related to window/document
  • auto-track
  • instrumentation logic.

And, that's fine, because what we need is building blocks for shipping metrics, the other functionality is nice to have or not needed in the webworker context.

As @SgtPooki mentioned before: Simply removing the IIFE, and using dependency injection should resolve many of our current problems would be a good start, but if you want to plan for the future, you can publish this as an ESM and add the features as additional plugins.

@turtledreams
Copy link
Contributor

Hi,
We have modified the SDK to be Web Worker friendly with the latest release (23.12.0). Currently, it seems to be working with non-Module Workers. However, as you also mentioned in your implementation, automatic tracking is not working (like for sessions, views, clicks, etc) as it depends on the window object.

Also, we extended the storage methods so you can provide your own. If you encounter any issues, please let us know.

Current documentation on the topic: https://support.count.ly/hc/en-us/articles/360037441932-Web-analytics-JavaScript-#h_01HH1BABFQ48FKWGJCX9HFMKW8

@ArtursKadikis
Copy link
Member

Hello,

Looks like it should be working in a web worker context. Will be closing this for now.
We can reopen in case there are any issues related to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants