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

fix: Change default storage type, migrate existing users #875

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 20 additions & 20 deletions src/__tests__/session-props.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { SessionPropsManager } from '../session-props'
import { SessionPropsManager, SessionSourceProps, StoredSessionSourceProps } from '../session-props'
import { SessionIdManager } from '../sessionid'
import { PostHogPersistence } from '../posthog-persistence'

Expand Down Expand Up @@ -36,7 +36,7 @@ describe('Session Props Manager', () => {
const utmSource = 'some-utm-source'
const sessionId = 'session-id'
const { onSessionId, generateProps, persistenceRegister } = createSessionPropsManager()
generateProps.mockReturnValue({ utm_source: utmSource })
generateProps.mockReturnValue({ s: utmSource } as SessionSourceProps)
const callback = onSessionId.mock.calls[0][0]

// act
Expand All @@ -46,12 +46,12 @@ describe('Session Props Manager', () => {
expect(generateProps).toHaveBeenCalledTimes(1)

expect(persistenceRegister).toBeCalledWith({
$client_session_props: {
props: {
utm_source: 'some-utm-source',
$cl_ses_p: {
p: {
s: 'some-utm-source',
},
sessionId: 'session-id',
},
s: 'session-id',
} as StoredSessionSourceProps,
})
})

Expand All @@ -60,10 +60,10 @@ describe('Session Props Manager', () => {
const sessionId1 = 'session-id-1'
const { onSessionId, persistence, generateProps, persistenceRegister } = createSessionPropsManager()
persistence.props = {
$client_session_props: {
props: {},
sessionId: sessionId1,
},
$cl_ses_p: {
p: {},
s: sessionId1,
} as StoredSessionSourceProps,
}
const callback = onSessionId.mock.calls[0][0]

Expand All @@ -82,10 +82,10 @@ describe('Session Props Manager', () => {

const { onSessionId, persistence, generateProps, persistenceRegister } = createSessionPropsManager()
persistence.props = {
$client_session_props: {
props: {},
sessionId: sessionId1,
},
$cl_ses_p: {
p: {},
s: sessionId1,
} as StoredSessionSourceProps,
}
const callback = onSessionId.mock.calls[0][0]

Expand All @@ -101,12 +101,12 @@ describe('Session Props Manager', () => {
// arrange
const { persistence, sessionPropsManager } = createSessionPropsManager()
persistence.props = {
$client_session_props: {
props: {
utm_source: 'some-utm-source',
$cl_ses_p: {
p: {
s: 'some-utm-source',
},
sessionId: 'session-id',
},
s: 'session-id',
} as StoredSessionSourceProps,
}

// act
Expand Down
2 changes: 1 addition & 1 deletion src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const SURVEYS = '$surveys'
export const FLAG_CALL_REPORTED = '$flag_call_reported'
export const USER_STATE = '$user_state'
export const POSTHOG_QUOTA_LIMITED = '$posthog_quota_limited'
export const CLIENT_SESSION_PROPS = '$client_session_props'
export const CLIENT_SESSION_PROPS = '$cl_ses_p'

// These are properties that are reserved and will not be automatically included in events
export const PERSISTENCE_RESERVED_PROPERTIES = [
Expand Down
2 changes: 1 addition & 1 deletion src/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export const defaultConfig = (): PostHogConfig => ({
autocapture: true,
rageclick: true,
cross_subdomain_cookie: isCrossDomainCookie(document?.location),
persistence: 'cookie',
persistence: 'default',
persistence_name: '',
cookie_name: '',
loaded: __NOOP,
Expand Down
79 changes: 73 additions & 6 deletions src/posthog-persistence.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
/* eslint camelcase: "off" */

import { _each, _extend, _include, _strip_empty_properties } from './utils'
import { cookieStore, localStore, localPlusCookieStore, memoryStore, sessionStore } from './storage'
import {
COOKIE_PERSISTED_PROPERTIES,
cookieStore,
localPlusCookieStore,
localStore,
memoryStore,
sessionStore,
} from './storage'
import { PersistentStore, PostHogConfig, Properties } from './types'
import {
PERSISTENCE_RESERVED_PROPERTIES,
EVENT_TIMERS_KEY,
ENABLED_FEATURE_FLAGS,
EVENT_TIMERS_KEY,
PERSISTENCE_RESERVED_PROPERTIES,
POSTHOG_QUOTA_LIMITED,
USER_STATE,
} from './constants'
Expand All @@ -21,6 +28,7 @@ const CASE_INSENSITIVE_PERSISTENCE_TYPES: readonly Lowercase<PostHogConfig['pers
'localstorage+cookie',
'sessionstorage',
'memory',
'default',
]

/**
Expand Down Expand Up @@ -64,8 +72,8 @@ export class PostHogPersistence {
config['persistence'].toLowerCase() as Lowercase<PostHogConfig['persistence']>
) === -1
) {
logger.critical('Unknown persistence type ' + config['persistence'] + '; falling back to cookie')
config['persistence'] = 'cookie'
logger.critical('Unknown persistence type ' + config['persistence'] + '; falling back to default')
config['persistence'] = 'default'
}
// We handle storage type in a case-insensitive way for backwards compatibility
const storage_type = config['persistence'].toLowerCase() as Lowercase<PostHogConfig['persistence']>
Expand All @@ -77,8 +85,11 @@ export class PostHogPersistence {
this.storage = sessionStore
} else if (storage_type === 'memory') {
this.storage = memoryStore
} else {
} else if (storage_type === 'cookie') {
this.storage = cookieStore
} else {
const { storage } = this._getAndMigrateToDefaultStore()
this.storage = storage
}

this.user_state = 'anonymous'
Expand Down Expand Up @@ -314,4 +325,60 @@ export class PostHogPersistence {
this.props[POSTHOG_QUOTA_LIMITED] = state
this.save()
}

_getAndMigrateToDefaultStore = (): { storage: PersistentStore; wasPreviouslyDifferentStore: boolean } => {
// Check if there's any data in local/cookie/session storage, and if needed,
// migrate to cookie+localstorage
const defaultStore = localPlusCookieStore
const currentStore = this._getCurrentStore()

if (currentStore && currentStore !== defaultStore) {
this._migrateStoreToDefaultStore(currentStore, defaultStore)
return { storage: defaultStore, wasPreviouslyDifferentStore: !!currentStore }
} else {
return { storage: defaultStore, wasPreviouslyDifferentStore: false }
}
}

_getCurrentStore = () => {
// Test by picking a property that is stored in the cookie for local+cookie
const testKey = COOKIE_PERSISTED_PROPERTIES[0]

// If that key exists in the local store, then we're definitely using
// local storage
const isLocalStore = localStore.is_supported() && localStore.get(this.name)?.[testKey]
if (isLocalStore) {
return localStore
}
// If not using pure localStore, we must be using localPlusCookie if
// there's data in both
const isLocalPlusCookie =
localPlusCookieStore.is_supported() &&
localStore.is_supported() &&
localStore.get(this.name) &&
cookieStore.is_supported() &&
cookieStore.get(this.name)
if (isLocalPlusCookie) {
return localPlusCookieStore
}
// If there's any data in cookieStore at this point then we're on cookieStore
const isCookie = cookieStore.is_supported() && cookieStore.get(this.name)
if (isCookie) {
return cookieStore
}

const isSession = sessionStore.is_supported() && sessionStore.get(this.name)
if (isSession) {
return sessionStore
}

// memory store, no existing store, or existing store no longer supported
return undefined
}

_migrateStoreToDefaultStore(store: PersistentStore, defaultStore: PersistentStore) {
const values = store.get(this.name)
store.remove(this.name, this.cross_subdomain)
defaultStore.set(this.name, values, this.expire_days, this.cross_subdomain, this.secure)
}
}
63 changes: 35 additions & 28 deletions src/session-props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,28 +11,35 @@ import { _info } from './utils/event-utils'
import { SessionIdManager } from './sessionid'
import { PostHogPersistence } from './posthog-persistence'
import { CLIENT_SESSION_PROPS } from './constants'
import { _strip_empty_properties } from './utils'

interface SessionSourceProps {
initialPathName: string
referringDomain: string // Is actually host, but named domain for internal consistency. Should contain a port if there is one.
utm_medium?: string
utm_source?: string
utm_campaign?: string
utm_content?: string
utm_term?: string
// this might be stored in a cookie with a hard 4096 byte limit, so save characters on key names
export interface SessionSourceProps {
p?: string // initial pathname
r?: string // referring domain
m?: string // utm medium
s?: string // utm source
c?: string // utm campaign
n?: string // utm content
t?: string // utm term
}

interface StoredSessionSourceProps {
sessionId: string
props: SessionSourceProps
export interface StoredSessionSourceProps {
s: string // session id
p: SessionSourceProps
}

const generateSessionSourceParams = (): SessionSourceProps => {
return {
initialPathName: window?.location.pathname || '',
referringDomain: _info.referringDomain(),
..._info.campaignParams(),
Copy link
Member Author

@robbie-c robbie-c Nov 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this also means we're not storing extra stuff beyond the keys I actually use - actually the biggest space saving comapred to reducing the key lengths

}
export const generateSessionSourceParams = (): SessionSourceProps => {
const campaignParams = _info.campaignParams()
return _strip_empty_properties({
p: window?.location.pathname || '',
r: _info.referringDomain(),
m: campaignParams.utm_medium,
s: campaignParams.utm_source,
c: campaignParams.utm_campaign,
n: campaignParams.utm_content,
t: campaignParams.utm_term,
})
}

export class SessionPropsManager {
Expand All @@ -58,31 +65,31 @@ export class SessionPropsManager {

_onSessionIdCallback = (sessionId: string) => {
const stored = this._getStoredProps()
if (stored && stored.sessionId === sessionId) {
if (stored && stored.s === sessionId) {
return
}

const newProps: StoredSessionSourceProps = {
sessionId,
props: this._sessionSourceParamGenerator(),
s: sessionId,
p: this._sessionSourceParamGenerator(),
}
this._persistence.register({ [CLIENT_SESSION_PROPS]: newProps })
}

getSessionProps() {
const p = this._getStoredProps()?.props
const p = this._getStoredProps()?.p
if (!p) {
return {}
}

return {
$client_session_initial_referring_host: p.referringDomain,
$client_session_initial_pathname: p.initialPathName,
$client_session_initial_utm_source: p.utm_source,
$client_session_initial_utm_campaign: p.utm_campaign,
$client_session_initial_utm_medium: p.utm_medium,
$client_session_initial_utm_content: p.utm_content,
$client_session_initial_utm_term: p.utm_term,
$client_session_initial_referring_host: p.r,
$client_session_initial_pathname: p.p,
$client_session_initial_utm_source: p.s,
$client_session_initial_utm_campaign: p.c,
$client_session_initial_utm_medium: p.m,
$client_session_initial_utm_content: p.n,
$client_session_initial_utm_term: p.t,
}
}
}
8 changes: 7 additions & 1 deletion src/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,12 @@ export const cookieStore: PersistentStore = {
'; SameSite=Lax; path=/' +
cdomain +
secure

// 4096 bytes is the size at which firefox will not store a cookie, warn slightly before that
if (new_cookie_val.length > 4096 * 0.9) {
logger.warn('cookieStore warning: large cookie, len=' + new_cookie_val.length)
}

document.cookie = new_cookie_val
return new_cookie_val
} catch (err) {
Expand Down Expand Up @@ -230,7 +236,7 @@ export const localStore: PersistentStore = {
// Use localstorage for most data but still use cookie for COOKIE_PERSISTED_PROPERTIES
// This solves issues with cookies having too much data in them causing headers too large
// Also makes sure we don't have to send a ton of data to the server
const COOKIE_PERSISTED_PROPERTIES = [DISTINCT_ID, SESSION_ID, SESSION_RECORDING_IS_SAMPLED]
export const COOKIE_PERSISTED_PROPERTIES = [DISTINCT_ID, SESSION_ID, SESSION_RECORDING_IS_SAMPLED]

export const localPlusCookieStore: PersistentStore = {
...localStore,
Expand Down
2 changes: 1 addition & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export interface PostHogConfig {
autocapture: boolean | AutocaptureConfig
rageclick: boolean
cross_subdomain_cookie: boolean
persistence: 'localStorage' | 'cookie' | 'memory' | 'localStorage+cookie' | 'sessionStorage'
persistence: 'localStorage' | 'cookie' | 'memory' | 'localStorage+cookie' | 'sessionStorage' | 'default'
persistence_name: string
cookie_name: string
loaded: (posthog_instance: PostHog) => void
Expand Down
7 changes: 5 additions & 2 deletions src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,11 @@ export const _safewrap_instance_methods = function (obj: Record<string, any>): v
}
}

export const _strip_empty_properties = function (p: Properties): Properties {
const ret: Properties = {}
export const _strip_empty_properties = function <T extends Record<string, any>>(
p: T
): // remove non-string properties from the type and all keys optional
{ [k in keyof T as T[k] extends string ? k : never]?: T[k] } {
const ret = {} as any
_each(p, function (v, k) {
if (_isString(v) && v.length > 0) {
ret[k] = v
Expand Down
Loading