-
Notifications
You must be signed in to change notification settings - Fork 68
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
Refactor tracker into importable TS module #143
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #143 +/- ##
==========================================
+ Coverage 84.15% 84.31% +0.15%
==========================================
Files 34 39 +5
Lines 2386 2512 +126
Branches 274 302 +28
==========================================
+ Hits 2008 2118 +110
- Misses 370 386 +16
Partials 8 8 ☔ View full report in Codecov by Sentry. |
Here by request from Bluesky 🫶🏻 I think it’s fine, but Counterscale looks like a singleton; you can probably discard the returns from the init function in favor of an import, ie
This then lets someone easily get to counterscale functions even when using a bundler and deep within their app. I’d also make autotrack the default and use a “disableAutotrack” option. This removes one more piece of the setup for most users. I wouldn't worry too much about DSN v URL naming. Suggested result:
|
It isn't. You can do this, for example: let trackPageviewSiteA = Counterscale({
siteId: "siteA"
reporterUrl: "..."
}).trackPageview;
let trackPageviewSiteB = Counterscale({
siteId: "siteB",
reporterUrl: "..."
}).trackPageview;
trackPageviewSiteA();
trackPageviewSiteB(); Or you could similarly report the event to multiple Counterscale servers.
Okay, I understand what you're saying. I think though, if I do that, we basically are committing to making it a singleton (the state has to be global for that to work) which I don't love. But you're right about ease of use, etc. |
Eh it'll be copy-paste for most people so not too worried about that. |
Refs #83
This patch does a few things:
new Image
counterscale.q
global vars) into a slim compatibility layerdist/module
dist/loader/tracker.js
Proposed API
Install via your package manager:
To automatically track pageviews:
To manually track pageviews:
Open Questions
Instead of
reporterUrl
, should this just beDSN
like Sentry uses? Seems redundant to have to specify/collect
.Plausible uses a method approach for auto-tracking pageviews, something like:
I'm of the mind to simplify as much as possible and avoid people having to write multiple statements for what will be the 90% default case, but I'm probably missing something. Thoughts?