Skip to content

Commit

Permalink
fix: improve survey bundle size (#1759)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lucasheriques
lucasheriques authored Feb 22, 2025
1 parent 48ae875 commit c3da349
Show file tree
Hide file tree
Showing 5 changed files with 143 additions and 173 deletions.
74 changes: 36 additions & 38 deletions src/__tests__/surveys.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/// <reference lib="dom" />

import { SURVEYS_REQUEST_TIMEOUT_MS } from '../constants'
import { generateSurveys } from '../extensions/surveys'
import { generateSurveys, getNextSurveyStep } from '../extensions/surveys'
import {
canActivateRepeatedly,
getDisplayOrderChoices,
Expand Down Expand Up @@ -927,8 +927,8 @@ describe('surveys', () => {
{ type: SurveyQuestionType.Open, question: 'Question A' },
{ type: SurveyQuestionType.Open, question: 'Question B' },
] as SurveyQuestion[]
expect(surveys.getNextSurveyStep(survey, 0, 'Some response')).toEqual(1)
expect(surveys.getNextSurveyStep(survey, 1, 'Some response')).toEqual(SurveyQuestionBranchingType.End)
expect(getNextSurveyStep(survey, 0, 'Some response')).toEqual(1)
expect(getNextSurveyStep(survey, 1, 'Some response')).toEqual(SurveyQuestionBranchingType.End)
})

it('should branch out to `end`', () => {
Expand All @@ -940,7 +940,7 @@ describe('surveys', () => {
},
{ type: SurveyQuestionType.Open, question: 'Question B' },
] as SurveyQuestion[]
expect(surveys.getNextSurveyStep(survey, 0, 'Some response')).toEqual(SurveyQuestionBranchingType.End)
expect(getNextSurveyStep(survey, 0, 'Some response')).toEqual(SurveyQuestionBranchingType.End)
})

it('should branch out to a specific question', () => {
Expand All @@ -953,7 +953,7 @@ describe('surveys', () => {
{ type: SurveyQuestionType.Open, question: 'Question B' },
{ type: SurveyQuestionType.Open, question: 'Question C' },
] as SurveyQuestion[]
expect(surveys.getNextSurveyStep(survey, 0, 'Some response')).toEqual(2)
expect(getNextSurveyStep(survey, 0, 'Some response')).toEqual(2)
})

// Single-choice
Expand All @@ -973,9 +973,9 @@ describe('surveys', () => {
{ type: SurveyQuestionType.Open, question: 'Why no?' },
{ type: SurveyQuestionType.Open, question: 'Why maybe?' },
] as unknown[] as SurveyQuestion[]
expect(surveys.getNextSurveyStep(survey, 0, 'Yes')).toEqual(1)
expect(surveys.getNextSurveyStep(survey, 0, 'No')).toEqual(2)
expect(surveys.getNextSurveyStep(survey, 0, 'Maybe')).toEqual(3)
expect(getNextSurveyStep(survey, 0, 'Yes')).toEqual(1)
expect(getNextSurveyStep(survey, 0, 'No')).toEqual(2)
expect(getNextSurveyStep(survey, 0, 'Maybe')).toEqual(3)
})

// Response-based branching, scale 1-3
Expand All @@ -995,9 +995,9 @@ describe('surveys', () => {
{ type: SurveyQuestionType.Open, question: 'Glad to hear that. Tell us more!' },
] as unknown[] as SurveyQuestion[]

expect(surveys.getNextSurveyStep(survey, 0, 1)).toEqual(1)
expect(surveys.getNextSurveyStep(survey, 0, 2)).toEqual(2)
expect(surveys.getNextSurveyStep(survey, 0, 3)).toEqual(3)
expect(getNextSurveyStep(survey, 0, 1)).toEqual(1)
expect(getNextSurveyStep(survey, 0, 2)).toEqual(2)
expect(getNextSurveyStep(survey, 0, 3)).toEqual(3)
})

// Response-based branching, scale 1-5
Expand All @@ -1017,9 +1017,9 @@ describe('surveys', () => {
{ type: SurveyQuestionType.Open, question: 'Glad to hear that. Tell us more!' },
] as unknown as SurveyQuestion[]

expect(surveys.getNextSurveyStep(survey, 0, 1)).toEqual(1)
expect(surveys.getNextSurveyStep(survey, 0, 3)).toEqual(2)
expect(surveys.getNextSurveyStep(survey, 0, 5)).toEqual(3)
expect(getNextSurveyStep(survey, 0, 1)).toEqual(1)
expect(getNextSurveyStep(survey, 0, 3)).toEqual(2)
expect(getNextSurveyStep(survey, 0, 5)).toEqual(3)
})

// Response-based branching, scale 1-7
Expand All @@ -1039,13 +1039,13 @@ describe('surveys', () => {
{ type: SurveyQuestionType.Open, question: 'Great! What did you enjoy the most?' },
] as unknown as SurveyQuestion[]

expect(surveys.getNextSurveyStep(survey, 0, 1)).toEqual(1)
expect(surveys.getNextSurveyStep(survey, 0, 2)).toEqual(1)
expect(surveys.getNextSurveyStep(survey, 0, 3)).toEqual(1)
expect(surveys.getNextSurveyStep(survey, 0, 4)).toEqual(2)
expect(surveys.getNextSurveyStep(survey, 0, 5)).toEqual(3)
expect(surveys.getNextSurveyStep(survey, 0, 6)).toEqual(3)
expect(surveys.getNextSurveyStep(survey, 0, 7)).toEqual(3)
expect(getNextSurveyStep(survey, 0, 1)).toEqual(1)
expect(getNextSurveyStep(survey, 0, 2)).toEqual(1)
expect(getNextSurveyStep(survey, 0, 3)).toEqual(1)
expect(getNextSurveyStep(survey, 0, 4)).toEqual(2)
expect(getNextSurveyStep(survey, 0, 5)).toEqual(3)
expect(getNextSurveyStep(survey, 0, 6)).toEqual(3)
expect(getNextSurveyStep(survey, 0, 7)).toEqual(3)
})

// Response-based branching, scale 0-10 (NPS)
Expand All @@ -1065,9 +1065,9 @@ describe('surveys', () => {
{ type: SurveyQuestionType.Open, question: 'Glad to hear that. Tell us more!' },
] as unknown as SurveyQuestion[]

expect(surveys.getNextSurveyStep(survey, 0, 1)).toEqual(1)
expect(surveys.getNextSurveyStep(survey, 0, 8)).toEqual(2)
expect(surveys.getNextSurveyStep(survey, 0, 10)).toEqual(3)
expect(getNextSurveyStep(survey, 0, 1)).toEqual(1)
expect(getNextSurveyStep(survey, 0, 8)).toEqual(2)
expect(getNextSurveyStep(survey, 0, 10)).toEqual(3)
})

it('should display questions in the order AGCEHDFB', () => {
Expand Down Expand Up @@ -1121,7 +1121,7 @@ describe('surveys', () => {
for (let i = 0; i < survey.questions.length; i++) {
const currentQuestion = survey.questions[currentStep]
actualOrder.push(currentQuestion.question)
currentStep = surveys.getNextSurveyStep(survey, currentStep, 'Some response')
currentStep = getNextSurveyStep(survey, currentStep, 'Some response')
}

expect(desiredOrder).toEqual(actualOrder)
Expand Down Expand Up @@ -1180,7 +1180,7 @@ describe('surveys', () => {
for (const answer of answers) {
const currentQuestion = survey.questions[currentStep]
actualOrder.push(currentQuestion.question)
currentStep = surveys.getNextSurveyStep(survey, currentStep, answer)
currentStep = getNextSurveyStep(survey, currentStep, answer)
}
expect(desiredOrder).toEqual(actualOrder)
expect(currentStep).toEqual(SurveyQuestionBranchingType.End)
Expand All @@ -1193,7 +1193,7 @@ describe('surveys', () => {
for (const answer of answers) {
const currentQuestion = survey.questions[currentStep]
actualOrder.push(currentQuestion.question)
currentStep = surveys.getNextSurveyStep(survey, currentStep, answer)
currentStep = getNextSurveyStep(survey, currentStep, answer)
}
expect(desiredOrder).toEqual(actualOrder)
expect(currentStep).toEqual(SurveyQuestionBranchingType.End)
Expand All @@ -1206,7 +1206,7 @@ describe('surveys', () => {
for (const answer of answers) {
const currentQuestion = survey.questions[currentStep]
actualOrder.push(currentQuestion.question)
currentStep = surveys.getNextSurveyStep(survey, currentStep, answer)
currentStep = getNextSurveyStep(survey, currentStep, answer)
}
expect(desiredOrder).toEqual(actualOrder)
expect(currentStep).toEqual(SurveyQuestionBranchingType.End)
Expand All @@ -1223,7 +1223,7 @@ describe('surveys', () => {
for (const answer of answers) {
const currentQuestion = survey.questions[currentStep]
actualOrder.push(currentQuestion.question)
currentStep = surveys.getNextSurveyStep(survey, currentStep, answer)
currentStep = getNextSurveyStep(survey, currentStep, answer)
}
expect(desiredOrder).toEqual(actualOrder)
expect(currentStep).toEqual(SurveyQuestionBranchingType.End)
Expand All @@ -1244,7 +1244,7 @@ describe('surveys', () => {
{ type: SurveyQuestionType.Open, question: 'Seems you are not completely happy. Tell us more!' },
{ type: SurveyQuestionType.Open, question: 'Glad to hear that. Tell us more!' },
] as unknown as SurveyQuestion[]
expect(() => surveys.getNextSurveyStep(survey, 0, 1)).toThrow('The scale must be one of: 3, 5, 7, 10')
expect(() => getNextSurveyStep(survey, 0, 1)).toThrow('The scale must be one of: 3, 5, 7, 10')
})

it('should throw an error for a response value out of the valid range', () => {
Expand All @@ -1262,13 +1262,13 @@ describe('surveys', () => {
{ type: SurveyQuestionType.Open, question: 'Seems you are not completely happy. Tell us more!' },
{ type: SurveyQuestionType.Open, question: 'Glad to hear that. Tell us more!' },
] as unknown as SurveyQuestion[]
expect(() => surveys.getNextSurveyStep(survey, 0, 20)).toThrow('The response must be in range 1-3')
expect(() => getNextSurveyStep(survey, 0, 20)).toThrow('The response must be in range 1-3')
;(survey.questions[0] as RatingSurveyQuestion).scale = 5
expect(() => surveys.getNextSurveyStep(survey, 0, 20)).toThrow('The response must be in range 1-5')
expect(() => getNextSurveyStep(survey, 0, 20)).toThrow('The response must be in range 1-5')
;(survey.questions[0] as RatingSurveyQuestion).scale = 7
expect(() => surveys.getNextSurveyStep(survey, 0, 20)).toThrow('The response must be in range 1-7')
expect(() => getNextSurveyStep(survey, 0, 20)).toThrow('The response must be in range 1-7')
;(survey.questions[0] as RatingSurveyQuestion).scale = 10
expect(() => surveys.getNextSurveyStep(survey, 0, 20)).toThrow('The response must be in range 0-10')
expect(() => getNextSurveyStep(survey, 0, 20)).toThrow('The response must be in range 0-10')
})

it('should throw an error for if a response value in a rating question is not an integer', () => {
Expand All @@ -1286,10 +1286,8 @@ describe('surveys', () => {
{ type: SurveyQuestionType.Open, question: 'Seems you are not completely happy. Tell us more!' },
{ type: SurveyQuestionType.Open, question: 'Glad to hear that. Tell us more!' },
] as unknown as SurveyQuestion[]
expect(() => surveys.getNextSurveyStep(survey, 0, '2')).toThrow('The response type must be an integer')
expect(() => surveys.getNextSurveyStep(survey, 0, 'some_string')).toThrow(
'The response type must be an integer'
)
expect(() => getNextSurveyStep(survey, 0, '2')).toThrow('The response type must be an integer')
expect(() => getNextSurveyStep(survey, 0, 'some_string')).toThrow('The response type must be an integer')
})
})

Expand Down
3 changes: 1 addition & 2 deletions src/entrypoints/surveys-preview.es.ts
View file
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
export { renderFeedbackWidgetPreview, renderSurveysPreview } from '../extensions/surveys'
export { getNextSurveyStep } from '../posthog-surveys'
export { getNextSurveyStep, renderFeedbackWidgetPreview, renderSurveysPreview } from '../extensions/surveys'
116 changes: 104 additions & 12 deletions src/extensions/surveys.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,109 @@ const logger = createLogger('[Surveys]')
const window = _window as Window & typeof globalThis
const document = _document as Document

function getRatingBucketForResponseValue(responseValue: number, scale: number) {
if (scale === 3) {
if (responseValue < 1 || responseValue > 3) {
throw new Error('The response must be in range 1-3')
}

return responseValue === 1 ? 'negative' : responseValue === 2 ? 'neutral' : 'positive'
} else if (scale === 5) {
if (responseValue < 1 || responseValue > 5) {
throw new Error('The response must be in range 1-5')
}

return responseValue <= 2 ? 'negative' : responseValue === 3 ? 'neutral' : 'positive'
} else if (scale === 7) {
if (responseValue < 1 || responseValue > 7) {
throw new Error('The response must be in range 1-7')
}

return responseValue <= 3 ? 'negative' : responseValue === 4 ? 'neutral' : 'positive'
} else if (scale === 10) {
if (responseValue < 0 || responseValue > 10) {
throw new Error('The response must be in range 0-10')
}

return responseValue <= 6 ? 'detractors' : responseValue <= 8 ? 'passives' : 'promoters'
}

throw new Error('The scale must be one of: 3, 5, 7, 10')
}

export function getNextSurveyStep(
survey: Survey,
currentQuestionIndex: number,
response: string | string[] | number | null
) {
const question = survey.questions[currentQuestionIndex]
const nextQuestionIndex = currentQuestionIndex + 1

if (!question.branching?.type) {
if (currentQuestionIndex === survey.questions.length - 1) {
return SurveyQuestionBranchingType.End
}

return nextQuestionIndex
}

if (question.branching.type === SurveyQuestionBranchingType.End) {
return SurveyQuestionBranchingType.End
} else if (question.branching.type === SurveyQuestionBranchingType.SpecificQuestion) {
if (Number.isInteger(question.branching.index)) {
return question.branching.index
}
} else if (question.branching.type === SurveyQuestionBranchingType.ResponseBased) {
// Single choice
if (question.type === SurveyQuestionType.SingleChoice) {
// :KLUDGE: for now, look up the choiceIndex based on the response
// TODO: once QuestionTypes.MultipleChoiceQuestion is refactored, pass the selected choiceIndex into this method
const selectedChoiceIndex = question.choices.indexOf(`${response}`)

if (question.branching?.responseValues?.hasOwnProperty(selectedChoiceIndex)) {
const nextStep = question.branching.responseValues[selectedChoiceIndex]

// Specific question
if (Number.isInteger(nextStep)) {
return nextStep
}

if (nextStep === SurveyQuestionBranchingType.End) {
return SurveyQuestionBranchingType.End
}

return nextQuestionIndex
}
} else if (question.type === SurveyQuestionType.Rating) {
if (typeof response !== 'number' || !Number.isInteger(response)) {
throw new Error('The response type must be an integer')
}

const ratingBucket = getRatingBucketForResponseValue(response, question.scale)

if (question.branching?.responseValues?.hasOwnProperty(ratingBucket)) {
const nextStep = question.branching.responseValues[ratingBucket]

// Specific question
if (Number.isInteger(nextStep)) {
return nextStep
}

if (nextStep === SurveyQuestionBranchingType.End) {
return SurveyQuestionBranchingType.End
}

return nextQuestionIndex
}
}

return nextQuestionIndex
}

logger.warn('Falling back to next question index due to unexpected branching type')
return nextQuestionIndex
}

export class SurveyManager {
private posthog: PostHog
private surveyInFocus: string | null
Expand Down Expand Up @@ -663,18 +766,7 @@ export function Questions({

setQuestionsResponses({ ...questionsResponses, [responseKey]: res })

// Old SDK, no branching
if (!posthog.getNextSurveyStep) {
const isLastDisplayedQuestion = displayQuestionIndex === survey.questions.length - 1
if (isLastDisplayedQuestion) {
sendSurveyEvent({ ...questionsResponses, [responseKey]: res }, survey, posthog)
} else {
setCurrentQuestionIndex(displayQuestionIndex + 1)
}
return
}

const nextStep = posthog.getNextSurveyStep(survey, displayQuestionIndex, res)
const nextStep = getNextSurveyStep(survey, displayQuestionIndex, res)
if (nextStep === SurveyQuestionBranchingType.End) {
sendSurveyEvent({ ...questionsResponses, [responseKey]: res }, survey, posthog)
} else {
Expand Down
11 changes: 1 addition & 10 deletions src/posthog-core.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { PostHogExceptions } from './posthog-exceptions'
import { PostHogFeatureFlags } from './posthog-featureflags'
import { PostHogPersistence } from './posthog-persistence'
import { PostHogSurveys } from './posthog-surveys'
import { Survey, SurveyCallback, SurveyQuestionBranchingType } from './posthog-surveys-types'
import { SurveyCallback } from './posthog-surveys-types'
import { RateLimiter } from './rate-limiter'
import { RemoteConfigLoader } from './remote-config'
import { extendURLParams, request, SUPPORTS_REQUEST } from './request'
Expand Down Expand Up @@ -1343,15 +1343,6 @@ export class PostHog {
this.surveys.canRenderSurvey(surveyId)
}

/** Get the next step of the survey: a question index or `end` */
getNextSurveyStep(
survey: Survey,
currentQuestionIndex: number,
response: string | string[] | number | null
): number | SurveyQuestionBranchingType.End {
return this.surveys.getNextSurveyStep(survey, currentQuestionIndex, response)
}

/**
* Identify a user with a unique ID instead of a PostHog
* randomly generated distinct_id. If the method is never called,
Expand Down
Loading

0 comments on commit c3da349

Please sign in to comment.