Skip to content

Commit

Permalink
fix: don't load preact until you have to (#1311)
Browse files Browse the repository at this point in the history
  • Loading branch information
pauldambra authored Jul 22, 2024
1 parent 306ed7d commit f2581a4
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 36 deletions.
53 changes: 24 additions & 29 deletions src/__tests__/surveys.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,18 @@ import {
SurveyQuestionBranchingType,
SurveyQuestion,
} from '../posthog-surveys-types'
import { getDisplayOrderChoices, getDisplayOrderQuestions } from '../extensions/surveys/surveys-utils'
import {
canActivateRepeatedly,
getDisplayOrderChoices,
getDisplayOrderQuestions,
} from '../extensions/surveys/surveys-utils'
import { PostHogPersistence } from '../posthog-persistence'
import { PostHog } from '../posthog-core'
import { DecideResponse, PostHogConfig, Properties } from '../types'
import { window } from '../utils/globals'
import { RequestRouter } from '../utils/request-router'
import { assignableWindow } from '../utils/globals'
import { expectScriptToExist, expectScriptToNotExist } from './helpers/script-utils'
import { generateSurveys } from '../extensions/surveys'

describe('surveys', () => {
let config: PostHogConfig
Expand All @@ -34,6 +38,7 @@ describe('surveys', () => {
'enabled-internal-targeting-flag-key': true,
'disabled-internal-targeting-flag-key': false,
},
surveys: true,
} as unknown as DecideResponse

const firstSurveys: Survey[] = [
Expand Down Expand Up @@ -119,6 +124,16 @@ describe('surveys', () => {
beforeEach(() => {
surveysResponse = { surveys: firstSurveys }

const loadScriptMock = jest.fn()

loadScriptMock.mockImplementation((_path, callback) => {
assignableWindow.__PosthogExtensions__ = assignableWindow.__Posthog__ || {}
assignableWindow.extendPostHogWithSurveys = generateSurveys
assignableWindow.__PosthogExtensions__.canActivateRepeatedly = canActivateRepeatedly

callback()
})

config = {
token: 'testtoken',
api_host: 'https://app.posthog.com',
Expand Down Expand Up @@ -147,7 +162,12 @@ describe('surveys', () => {
},
} as unknown as PostHog

instance.requestRouter.loadScript = loadScriptMock

surveys = new PostHogSurveys(instance)
instance.surveys = surveys
// all being squashed into a mock posthog so...
instance.getActiveMatchingSurveys = instance.surveys.getActiveMatchingSurveys.bind(instance.surveys)

Object.defineProperty(window, 'location', {
configurable: true,
Expand All @@ -156,6 +176,8 @@ describe('surveys', () => {
// eslint-disable-next-line compat/compat
value: new URL('https://example.com'),
})

surveys.afterDecideResponse(decideResponse)
})

afterEach(() => {
Expand Down Expand Up @@ -757,33 +779,6 @@ describe('surveys', () => {
})
})

describe('decide response', () => {
beforeEach(() => {
// clean the JSDOM to prevent interdependencies between tests
document.body.innerHTML = ''
document.head.innerHTML = ''
})

it('should not load when decide response says no', () => {
surveys.afterDecideResponse({ surveys: false } as DecideResponse)
// Make sure the script is not loaded
expectScriptToNotExist('https://us-assets.i.posthog.com/static/surveys.js')
})

it('should load when decide response says so', () => {
surveys.afterDecideResponse({ surveys: true } as DecideResponse)
// Make sure the script is loaded
expectScriptToExist('https://us-assets.i.posthog.com/static/surveys.js')
})

it('should not load when config says no', () => {
config.disable_surveys = true
surveys.afterDecideResponse({ surveys: true } as DecideResponse)
// Make sure the script is not loaded
expectScriptToNotExist('https://us-assets.i.posthog.com/static/surveys.js')
})
})

describe('branching logic', () => {
const survey: Survey = {
name: 'My survey',
Expand Down
3 changes: 3 additions & 0 deletions src/entrypoints/surveys.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { generateSurveys } from '../extensions/surveys'

import { window } from '../utils/globals'
import { canActivateRepeatedly } from '../extensions/surveys/surveys-utils'

if (window) {
;(window as any).__PosthogExtensions__ = (window as any).__PosthogExtensions__ || {}
;(window as any).__PosthogExtensions__.canActivateRepeatedly = canActivateRepeatedly
;(window as any).extendPostHogWithSurveys = generateSurveys
}

Expand Down
22 changes: 15 additions & 7 deletions src/posthog-surveys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import { SurveyEventReceiver } from './utils/survey-event-receiver'
import { assignableWindow, document, window } from './utils/globals'
import { DecideResponse } from './types'
import { logger } from './utils/logger'
import { canActivateRepeatedly } from './extensions/surveys/surveys-utils'
import { isNullish } from './utils/type-utils'

const LOGGER_PREFIX = '[Surveys]'

export const surveyUrlValidationMap: Record<SurveyUrlMatchType, (conditionsUrl: string) => boolean> = {
icontains: (conditionsUrl) =>
Expand Down Expand Up @@ -50,12 +52,10 @@ function getRatingBucketForResponseValue(responseValue: number, scale: number) {
}

export class PostHogSurveys {
instance: PostHog
private _decideServerResponse?: boolean
public _surveyEventReceiver: SurveyEventReceiver | null

constructor(instance: PostHog) {
this.instance = instance
constructor(private readonly instance: PostHog) {
// we set this to undefined here because we need the persistence storage for this type
// but that's not initialized until loadIfEnabled is called.
this._surveyEventReceiver = null
Expand All @@ -75,7 +75,7 @@ export class PostHogSurveys {
}
this.instance.requestRouter.loadScript('/static/surveys.js', (err) => {
if (err) {
return logger.error(`Could not load surveys script`, err)
return logger.error(LOGGER_PREFIX, 'Could not load surveys script', err)
}

assignableWindow.extendPostHogWithSurveys(this.instance)
Expand Down Expand Up @@ -178,7 +178,7 @@ export class PostHogSurveys {
const eventBasedTargetingFlagCheck =
hasEvents || hasActions ? activatedSurveys?.includes(survey.id) : true

const overrideInternalTargetingFlagCheck = canActivateRepeatedly(survey)
const overrideInternalTargetingFlagCheck = this._canActivateRepeatedly(survey)
const internalTargetingFlagCheck =
survey.internal_targeting_flag_key && !overrideInternalTargetingFlagCheck
? this.instance.featureFlags.isFeatureEnabled(survey.internal_targeting_flag_key)
Expand Down Expand Up @@ -258,7 +258,15 @@ export class PostHogSurveys {
return nextQuestionIndex
}

console.warn('Falling back to next question index due to unexpected branching type') // eslint-disable-line no-console
logger.warn(LOGGER_PREFIX, 'Falling back to next question index due to unexpected branching type')
return nextQuestionIndex
}

// this method is lazily loaded onto the window to avoid loading preact and other dependencies if surveys is not enabled
private _canActivateRepeatedly(survey: Survey) {
if (isNullish(assignableWindow.__PosthogExtensions__.canActivateRepeatedly)) {
logger.warn(LOGGER_PREFIX, 'canActivateRepeatedly is not defined, must init before calling')
}
return assignableWindow.__PosthogExtensions__.canActivateRepeatedly(survey)
}
}

0 comments on commit f2581a4

Please sign in to comment.