Skip to content

Commit

Permalink
fix: Reacting to config changes (#1138)
Browse files Browse the repository at this point in the history
  • Loading branch information
benjackwhite authored Apr 17, 2024
1 parent e3ca684 commit 4998648
Show file tree
Hide file tree
Showing 19 changed files with 254 additions and 249 deletions.
2 changes: 1 addition & 1 deletion cypress/e2e/capture.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ describe('Event capture', () => {

describe('advanced_disable_decide config', () => {
it('does not autocapture anything when /decide is disabled', () => {
start({ options: { advanced_disable_decide: true }, waitForDecide: false })
start({ options: { autocapture: false, advanced_disable_decide: true }, waitForDecide: false })

cy.get('body').click(100, 100).click(98, 102).click(101, 103)
cy.get('[data-cy-custom-event-button]').click()
Expand Down
14 changes: 3 additions & 11 deletions playground/nextjs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,12 @@ NEXT_PUBLIC_POSTHOG_KEY='<your-local-api-key>' NEXT_PUBLIC_POSTHOG_HOST='http://
```

### Testing local changes to posthog-js
Follow the instructions to set up YALC in the [root README](../../README.md).

After you have run `yalc publish` in the root directory, run `yalc add posthog-js` in this directory, and then you can
run `pnpm dev` to see your changes reflected in the demo project.
Running `pnpm dev` will run an additional script that uses pnpm to link `posthog-js` locally to this package.

There is a shorthand script for running these 3 commands
If you need to provide environment variables, you can do so:

```bash
pnpm yalc-dev
```

If you need to provide environment variables, you can do so, like

```bash
NEXT_PUBLIC_POSTHOG_KEY='<your-local-api-key>' NEXT_PUBLIC_POSTHOG_HOST='http://localhost:8000' pnpm yalc-dev
NEXT_PUBLIC_POSTHOG_KEY='<your-local-api-key>' NEXT_PUBLIC_POSTHOG_HOST='http://localhost:8000' pnpm dev
```

4 changes: 2 additions & 2 deletions playground/nextjs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
"private": true,
"scripts": {
"clean-react": "cd ../../react && rm -rf ./node_modules/",
"dev": "pnpm run clean-react && next dev",
"dev": "pnpm run link-posthog-js && pnpm run clean-react && next dev",
"build": "next build",
"start": "next start",
"lint": "next lint",
"yalc-dev": "pnpm run -C ../.. yalc-publish && rm -r .next/cache .yalc/posthog-js && yalc add posthog-js && pnpm dev"
"link-posthog-js": "cd ../../ && pnpm link --global && cd playground/nextjs && pnpm link --global posthog-js"
},
"dependencies": {
"@lottiefiles/react-lottie-player": "^3.5.3",
Expand Down
17 changes: 2 additions & 15 deletions playground/nextjs/pages/_app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,8 @@ import { useRouter } from 'next/router'

import posthog from 'posthog-js'
import { PostHogProvider } from 'posthog-js/react'
import { CookieBanner, cookieConsentGiven } from '@/src/CookieBanner'

if (typeof window !== 'undefined') {
posthog.init(process.env.NEXT_PUBLIC_POSTHOG_KEY || '', {
api_host: process.env.NEXT_PUBLIC_POSTHOG_HOST || 'https://us.i.posthog.com',
session_recording: {
recordCrossOriginIframes: true,
},
debug: true,
scroll_root_selector: ['#scroll_element', 'html'],
persistence: cookieConsentGiven() ? 'localStorage+cookie' : 'memory',
__preview_process_person: 'identified_only',
})
;(window as any).posthog = posthog
}
import { CookieBanner } from '@/src/CookieBanner'
import '@/src/posthog'

export default function App({ Component, pageProps }: AppProps) {
const router = useRouter()
Expand Down
8 changes: 8 additions & 0 deletions playground/nextjs/pages/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ import Head from 'next/head'
import { useFeatureFlagEnabled, usePostHog } from 'posthog-js/react'
import React, { useEffect, useState } from 'react'
import Link from 'next/link'
import { cookieConsentGiven } from '@/src/posthog'

export default function Home() {
const posthog = usePostHog()
const [isClient, setIsClient] = useState(false)
const result = useFeatureFlagEnabled('test')

const [time, setTime] = useState('')
const consentGiven = cookieConsentGiven()

useEffect(() => {
setIsClient(true)
Expand Down Expand Up @@ -71,6 +73,12 @@ export default function Home() {

{isClient && (
<>
{!consentGiven && (
<p className="border border-red-900 bg-red-200 rounded p-2">
<b>Consent not given!</b> Session recording, surveys, and autocapture are disabled.
</p>
)}

<h2 className="mt-4">PostHog info</h2>
<ul className="text-xs bg-gray-100 rounded border-2 border-gray-800 p-4">
<li className="font-mono">
Expand Down
21 changes: 11 additions & 10 deletions playground/nextjs/src/CookieBanner.tsx
Original file line number Diff line number Diff line change
@@ -1,23 +1,26 @@
/* eslint-disable posthog-js/no-direct-null-check */
import posthog from 'posthog-js'
import { useEffect, useState } from 'react'
import { cookieConsentGiven, updatePostHogConsent } from './posthog'

export function cookieConsentGiven() {
return localStorage.getItem('cookie_consent') === 'true'
}

export function CookieBanner() {
const [consentGiven, setConsentGiven] = useState<null | boolean>(null)
export function useCookieConsent(): [boolean | null, (consentGiven: boolean) => void] {
const [consentGiven, setConsentGiven] = useState<boolean | null>(null)

useEffect(() => {
setConsentGiven(cookieConsentGiven())
}, [])

useEffect(() => {
if (consentGiven === null) return
posthog.set_config({ persistence: consentGiven ? 'localStorage+cookie' : 'memory' })

updatePostHogConsent(consentGiven)
}, [consentGiven])

return [consentGiven, setConsentGiven]
}

export function CookieBanner() {
const [consentGiven, setConsentGiven] = useCookieConsent()

if (consentGiven === null) return null

return (
Expand All @@ -27,7 +30,6 @@ export function CookieBanner() {
<p>I am a cookie banner - you wouldn't like me when I'm hangry.</p>
<button
onClick={() => {
localStorage.setItem('cookie_consent', 'true')
setConsentGiven(true)
}}
>
Expand All @@ -38,7 +40,6 @@ export function CookieBanner() {
<>
<button
onClick={() => {
localStorage.removeItem('cookie_consent')
setConsentGiven(false)
}}
>
Expand Down
49 changes: 49 additions & 0 deletions playground/nextjs/src/posthog.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import posthog, { PostHogConfig } from 'posthog-js'

/**
* Below is an example of a consent-driven config for PostHog
* Lots of things start in a disabled state and posthog will not use cookies without consent
*
* Once given, we enable autocapture, session recording, and use localStorage+cookie for persistence via set_config
* This is only an example - data privacy requirements are different for every project
*/
export function cookieConsentGiven(): null | boolean {
if (typeof window === 'undefined') return null
return localStorage.getItem('cookie_consent') === 'true'
}

export const configForConsent = (): Partial<PostHogConfig> => {
const consentGiven = localStorage.getItem('cookie_consent') === 'true'

return {
persistence: consentGiven ? 'localStorage+cookie' : 'memory',
disable_surveys: !consentGiven,
autocapture: consentGiven,
disable_session_recording: !consentGiven,
}
}

export const updatePostHogConsent = (consentGiven: boolean) => {
if (consentGiven) {
localStorage.setItem('cookie_consent', 'true')
} else {
localStorage.removeItem('cookie_consent')
}

posthog.set_config(configForConsent())
}

if (typeof window !== 'undefined') {
posthog.init(process.env.NEXT_PUBLIC_POSTHOG_KEY || '', {
api_host: process.env.NEXT_PUBLIC_POSTHOG_HOST || 'https://us.i.posthog.com',
session_recording: {
recordCrossOriginIframes: true,
},
debug: true,
scroll_root_selector: ['#scroll_element', 'html'],
// persistence: cookieConsentGiven() ? 'localStorage+cookie' : 'memory',
__preview_process_person: 'identified_only',
...configForConsent(),
})
;(window as any).posthog = posthog
}
11 changes: 9 additions & 2 deletions src/__tests__/autocapture.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,8 @@ describe('Autocapture system', () => {
describe('_captureEvent', () => {
beforeEach(() => {
posthog.config.rageclick = true
// Trigger proper enabling
autocapture.afterDecideResponse({} as DecideResponse)
})

it('should capture rageclick', () => {
Expand Down Expand Up @@ -920,10 +922,10 @@ describe('Autocapture system', () => {
beforeEach(() => {
document.title = 'test page'
posthog.config.mask_all_element_attributes = false
autocapture.afterDecideResponse({} as DecideResponse)
})

it('should capture click events', () => {
autocapture['_addDomEventHandlers']()
const button = document.createElement('button')
document.body.appendChild(button)
simulateClick(button)
Expand All @@ -943,7 +945,12 @@ describe('Autocapture system', () => {
jest.spyOn(autocapture, '_addDomEventHandlers')
})

it('should be enabled before the decide response', () => {
it('should not be enabled before the decide response', () => {
expect(autocapture.isEnabled).toBe(false)
})

it('should be enabled before the decide response if decide is disabled', () => {
posthog.config.advanced_disable_decide = true
expect(autocapture.isEnabled).toBe(true)
})

Expand Down
96 changes: 5 additions & 91 deletions src/__tests__/decide.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Decide } from '../decide'
import { PostHogPersistence } from '../posthog-persistence'
import { RequestRouter } from '../utils/request-router'
import { expectScriptToExist, expectScriptToNotExist } from './helpers/script-utils'

const expectDecodedSendRequest = (send_request, data, noCompression) => {
const lastCall = send_request.mock.calls[send_request.mock.calls.length - 1]
Expand All @@ -19,25 +20,6 @@ const expectDecodedSendRequest = (send_request, data, noCompression) => {
})
}

const checkScriptsForSrc = (src, negate = false) => {
const scripts = document.querySelectorAll('body > script')
let foundScript = false
for (let i = 0; i < scripts.length; i++) {
if (scripts[i].src === src) {
foundScript = true
break
}
}

if (foundScript && negate) {
throw new Error(`Script with src ${src} was found when it should not have been.`)
} else if (!foundScript && !negate) {
throw new Error(`Script with src ${src} was not found when it should have been.`)
} else {
return true
}
}

describe('Decide', () => {
given('decide', () => new Decide(given.posthog))
given('posthog', () => ({
Expand All @@ -51,16 +33,6 @@ describe('Decide', () => {
_afterDecideResponse: jest.fn(),
get_distinct_id: jest.fn().mockImplementation(() => 'distinctid'),
_send_request: jest.fn().mockImplementation(({ callback }) => callback?.({ config: given.decideResponse })),
toolbar: {
maybeLoadToolbar: jest.fn(),
afterDecideResponse: jest.fn(),
},
sessionRecording: {
afterDecideResponse: jest.fn(),
},
autocapture: {
afterDecideResponse: jest.fn(),
},
featureFlags: {
receivedFeatureFlags: jest.fn(),
setReloadingPaused: jest.fn(),
Expand Down Expand Up @@ -197,11 +169,8 @@ describe('Decide', () => {
given('decideResponse', () => ({}))
given.subject()

expect(given.posthog.sessionRecording.afterDecideResponse).toHaveBeenCalledWith(given.decideResponse)
expect(given.posthog.toolbar.afterDecideResponse).toHaveBeenCalledWith(given.decideResponse)
expect(given.posthog.featureFlags.receivedFeatureFlags).toHaveBeenCalledWith(given.decideResponse, false)
expect(given.posthog._afterDecideResponse).toHaveBeenCalledWith(given.decideResponse)
expect(given.posthog.autocapture.afterDecideResponse).toHaveBeenCalledWith(given.decideResponse)
})

it('Make sure receivedFeatureFlags is called with errors if the decide response fails', () => {
Expand All @@ -228,10 +197,7 @@ describe('Decide', () => {

given.subject()

expect(given.posthog.autocapture.afterDecideResponse).toHaveBeenCalledWith(given.decideResponse)
expect(given.posthog.sessionRecording.afterDecideResponse).toHaveBeenCalledWith(given.decideResponse)
expect(given.posthog.toolbar.afterDecideResponse).toHaveBeenCalledWith(given.decideResponse)

expect(given.posthog._afterDecideResponse).toHaveBeenCalledWith(given.decideResponse)
expect(given.posthog.featureFlags.receivedFeatureFlags).not.toHaveBeenCalled()
})

Expand All @@ -248,18 +214,15 @@ describe('Decide', () => {

given.subject()

expect(given.posthog.autocapture.afterDecideResponse).toHaveBeenCalledWith(given.decideResponse)
expect(given.posthog.sessionRecording.afterDecideResponse).toHaveBeenCalledWith(given.decideResponse)
expect(given.posthog.toolbar.afterDecideResponse).toHaveBeenCalledWith(given.decideResponse)

expect(given.posthog._afterDecideResponse).toHaveBeenCalledWith(given.decideResponse)
expect(given.posthog.featureFlags.receivedFeatureFlags).not.toHaveBeenCalled()
})

it('runs site apps if opted in', () => {
given('config', () => ({ api_host: 'https://test.com', opt_in_site_apps: true, persistence: 'memory' }))
given('decideResponse', () => ({ siteApps: [{ id: 1, url: '/site_app/1/tokentoken/hash/' }] }))
given.subject()
expect(checkScriptsForSrc('https://test.com/site_app/1/tokentoken/hash/')).toBe(true)
expectScriptToExist('https://test.com/site_app/1/tokentoken/hash/')
})

it('does not run site apps code if not opted in', () => {
Expand All @@ -272,56 +235,7 @@ describe('Decide', () => {
// throwing only in tests, just an error in production
'Unexpected console.error: [PostHog.js],PostHog site apps are disabled. Enable the "opt_in_site_apps" config to proceed.'
)
expect(checkScriptsForSrc('https://test.com/site_app/1/tokentoken/hash/', true)).toBe(true)
})

it('Make sure surveys are not loaded when decide response says no', () => {
given('decideResponse', () => ({
featureFlags: { 'test-flag': true },
surveys: false,
}))
given('config', () => ({
api_host: 'https://test.com',
token: 'testtoken',
persistence: 'memory',
}))

given.subject()
// Make sure the script is not loaded
expect(checkScriptsForSrc('https://test.com/static/surveys.js', true)).toBe(true)
})

it('Make sure surveys are loaded when decide response says so', () => {
given('decideResponse', () => ({
featureFlags: { 'test-flag': true },
surveys: true,
}))
given('config', () => ({
api_host: 'https://test.com',
token: 'testtoken',
persistence: 'memory',
}))

given.subject()
// Make sure the script is loaded
expect(checkScriptsForSrc('https://test.com/static/surveys.js')).toBe(true)
})

it('Make sure surveys are not loaded when config says no', () => {
given('decideResponse', () => ({
featureFlags: { 'test-flag': true },
surveys: true,
}))
given('config', () => ({
api_host: 'https://test.com',
token: 'testtoken',
persistence: 'memory',
disable_surveys: true,
}))

given.subject()
// Make sure the script is not loaded
expect(checkScriptsForSrc('https://test.com/static/surveys.js', true)).toBe(true)
expectScriptToNotExist('https://test.com/site_app/1/tokentoken/hash/')
})
})
})
Loading

0 comments on commit 4998648

Please sign in to comment.