-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: use localStorage to cache consent in browsers #64
Changes from 2 commits
a82c0f6
0524e74
d8d6326
2cd942e
20920d6
d14d52a
1e68898
cc19b56
0263567
84ee0e2
8334d47
3e9c2a7
57636b2
f01a86b
77a95c5
af2b67d
ef9d4cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
import type { StorageProvider } from './types/index.js' | ||
|
||
export const BrowserStorageProvider: StorageProvider = { | ||
SgtPooki marked this conversation as resolved.
Show resolved
Hide resolved
SgtPooki marked this conversation as resolved.
Show resolved
Hide resolved
|
||
setStore: (consentArray) => { | ||
try { | ||
const jsonString = JSON.stringify(consentArray) | ||
window.localStorage.setItem('@ipfs-shipyard/ignite-metrics:consent', jsonString) | ||
SgtPooki marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} catch (err) { | ||
// eslint-disable-next-line no-console | ||
console.error(err) | ||
} | ||
}, | ||
getStore: () => { | ||
try { | ||
const jsonString = window.localStorage.getItem('@ipfs-shipyard/ignite-metrics:consent') | ||
if (jsonString != null) { | ||
return JSON.parse(jsonString) | ||
} | ||
} catch (err) { | ||
// eslint-disable-next-line no-console | ||
console.error(err) | ||
} | ||
/** | ||
* Return minimal consent if there is nothing in the store. | ||
*/ | ||
return ['minimal'] | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ import { COUNTLY_SETUP_DEFAULTS } from './config.js' | |
|
||
import type { metricFeatures, CountlyWebSdk } from 'countly-sdk-web' | ||
import type { CountlyNodeSdk } from 'countly-sdk-nodejs' | ||
import type { consentTypes, consentTypesExceptAll } from './types/index.js' | ||
import type { consentTypes, consentTypesExceptAll, StorageProvider } from './types/index.js' | ||
|
||
export interface MetricsProviderConstructorOptions<T> { | ||
appKey: string | ||
|
@@ -13,9 +13,10 @@ export interface MetricsProviderConstructorOptions<T> { | |
session_update?: number | ||
url?: string | ||
metricsService: T | ||
storageProvider?: StorageProvider | null | ||
} | ||
|
||
export default class MetricsProvider<T extends CountlyWebSdk & CountlyNodeSdk> { | ||
export default class MetricsProvider<T extends CountlyWebSdk | CountlyNodeSdk> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. either or, not both. |
||
private readonly groupedFeatures: Record<consentTypes, metricFeatures[]> = this.mapAllEvents({ | ||
minimal: ['sessions', 'views', 'events'], | ||
performance: ['crashes', 'apm'], | ||
|
@@ -27,14 +28,16 @@ export default class MetricsProvider<T extends CountlyWebSdk & CountlyNodeSdk> { | |
private sessionStarted: boolean = false | ||
private readonly _consentGranted: Set<consentTypes> = new Set() | ||
private readonly metricsService: T | ||
private readonly storageProvider: StorageProvider | null | ||
|
||
constructor (config: MetricsProviderConstructorOptions<T>) { | ||
const serviceConfig = { | ||
...COUNTLY_SETUP_DEFAULTS, | ||
...config | ||
} | ||
const { appKey, autoTrack, metricsService, url } = serviceConfig | ||
const { appKey, autoTrack, metricsService, url, storageProvider } = serviceConfig | ||
this.metricsService = metricsService | ||
this.storageProvider = storageProvider ?? null | ||
this.metricsService.init({ | ||
app_key: appKey, | ||
url, | ||
|
@@ -44,9 +47,11 @@ export default class MetricsProvider<T extends CountlyWebSdk & CountlyNodeSdk> { | |
this.metricsService.init(serviceConfig) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a fan of passing a bunch of "junk" to metricsService.init(), but I spent enough time fighting stuff today There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well there needs to be defaults in place in order to ship better defaults. What's the alternative you're thinking about? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The alternative is strict types for the inputs to .init(). They could easily update their code to throw if an invalid config is passed in :/
esbuild yelled at me when I did this earlier, but I think that was when I was messing with building different targets trying to get browser testing to work |
||
this.metricsService.group_features(this.groupedFeatures) | ||
|
||
if (autoTrack) { | ||
if (autoTrack === true) { | ||
SgtPooki marked this conversation as resolved.
Show resolved
Hide resolved
|
||
this.setupAutoTrack() | ||
} | ||
|
||
this.getConsentStore().forEach(this.addConsent.bind(this)) | ||
SgtPooki marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
mapAllEvents (eventMap: Record<consentTypesExceptAll, metricFeatures[]>): Record<consentTypes, metricFeatures[]> { | ||
|
@@ -77,6 +82,7 @@ export default class MetricsProvider<T extends CountlyWebSdk & CountlyNodeSdk> { | |
} | ||
consent.forEach(c => this._consentGranted.add(c)) | ||
this.metricsService.add_consent(consent) | ||
this.setConsentStore() | ||
} | ||
|
||
removeConsent (consent: consentTypes | consentTypes[]): void { | ||
|
@@ -85,6 +91,20 @@ export default class MetricsProvider<T extends CountlyWebSdk & CountlyNodeSdk> { | |
} | ||
consent.forEach(c => this._consentGranted.delete(c)) | ||
this.metricsService.remove_consent(consent, true) | ||
this.setConsentStore() | ||
} | ||
|
||
private setConsentStore (): void { | ||
if (this.storageProvider != null) { | ||
this.storageProvider.setStore(Array.from(this._consentGranted)) | ||
} | ||
SgtPooki marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
private getConsentStore (): consentTypes[] { | ||
if (this.storageProvider != null) { | ||
return this.storageProvider.getStore() | ||
} | ||
return [] | ||
SgtPooki marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
checkConsent (consent: consentTypes | metricFeatures): boolean { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,8 @@ | ||
|
||
export type consentTypesExceptAll = 'minimal' | 'performance' | 'ux' | 'feedback' | 'location' | ||
export type consentTypes = 'all' | consentTypesExceptAll | ||
|
||
export interface StorageProvider { | ||
setStore: (values: consentTypes[]) => void | ||
getStore: () => consentTypes[] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are only providing storage provider for non-node because that's the only one we currently need.