feat: Load surveys via RemoteConfig #1652

benjackwhite · 2025-01-15T07:56:57Z

Changes

We have RemoteConfig loading that includes surveys so time to start using it!

Follow up

Remove draft surveys from the RemoteConfig
Do we need to persist surveys? Seems to be a speed thing but the RemoteConfig loads so fast now that it feels weird. Should we instead persist the remote config?

Checklist

Tests for new code (see advice on the tests we use)
Accounted for the impact of any changes across different browsers
Accounted for backwards compatibility of any changes (no breaking changes in posthog-js!)

vercel · 2025-01-15T07:57:02Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
posthog-js	✅ Ready (Inspect)	Visit Preview	Jan 15, 2025 9:31am

github-actions · 2025-01-15T08:01:26Z

Size Change: -1.27 kB (-0.04%)

Total Size: 3.23 MB

Filename	Size	Change
`dist/all-external-dependencies.js`	207 kB	-60 B (-0.03%)
`dist/array.full.es5.js`	264 kB	-95 B (-0.04%)
`dist/array.full.js`	366 kB	-150 B (-0.04%)
`dist/array.full.no-external.js`	365 kB	-150 B (-0.04%)
`dist/array.js`	180 kB	-91 B (-0.05%)
`dist/array.no-external.js`	178 kB	-91 B (-0.05%)
`dist/main.js`	181 kB	-91 B (-0.05%)
`dist/module.full.js`	366 kB	-150 B (-0.04%)
`dist/module.full.no-external.js`	365 kB	-150 B (-0.04%)
`dist/module.js`	180 kB	-91 B (-0.05%)
`dist/module.no-external.js`	179 kB	-91 B (-0.05%)
`dist/surveys.js`	63.7 kB	-60 B (-0.09%)

ℹ️ View Unchanged

Filename	Size
`dist/customizations.full.js`	12.6 kB
`dist/dead-clicks-autocapture.js`	14.4 kB
`dist/exception-autocapture.js`	9.48 kB
`dist/external-scripts-loader.js`	2.64 kB
`dist/recorder-v2.js`	115 kB
`dist/recorder.js`	115 kB
`dist/surveys-preview.js`	58.1 kB
`dist/tracing-headers.js`	1.76 kB
`dist/web-vitals.js`	10.4 kB

_{compressed-size-action}

marandaneto · 2025-01-15T09:33:48Z

src/posthog-surveys.ts

+
+    private onSurveys(surveys: Survey[]) {
+        this._loadedSurveys = surveys
+        this._surveysEnabledRemotely = surveys.length > 0


I think _surveysEnabledRemotely should be true if surveys isn't undefined, and empty is a valid state (no surveys but the API call has been made), otherwise it'd be semantically wrong, at least the naming.

The thing is I actually think we should just ditch that property altogether. Server side that property is simply "are there surveys to load" but if we now include them in remtoe config I actually think that property is pointless...

marandaneto · 2025-01-15T09:36:23Z

src/posthog-surveys.ts

+                (survey.conditions?.events &&
+                    survey.conditions?.events?.values &&
+                    survey.conditions?.events?.values?.length > 0) ||


Suggested change

(survey.conditions?.events &&

survey.conditions?.events?.values &&

survey.conditions?.events?.values?.length > 0) ||

(survey.conditions?.events?.values?.length > 0) ||

not needed the 2 checks since the last one is already using the safe operator ?.

True. Just moved code but happy to improve it whilst im here

marandaneto · 2025-01-15T09:36:46Z

src/posthog-surveys.ts

+                (survey.conditions?.actions &&
+                    survey.conditions?.actions?.values &&
+                    survey.conditions?.actions?.values?.length > 0)


Suggested change

(survey.conditions?.actions &&

survey.conditions?.actions?.values &&

survey.conditions?.actions?.values?.length > 0)

(survey.conditions?.actions?.values?.length > 0)

marandaneto · 2025-01-15T09:42:11Z

Do we need to persist surveys? Seems to be a speed thing but the RemoteConfig loads so fast now that it feels weird. Should we instead persist the remote config?

Neil created this ticket which aims to improve the API perf., in case things are slow.
Surveys have to be cached, or at least they currently are, see the docs:

Surveys are requested on first load and then cached by default by the JavaScript SDK. If you want to force an API call to get an updated list of surveys, pass true to the forceReload parameter. You only need to do this if you want changed surveys to appear mid-session for users.

So I'd avoid breaking changes if possible.

marandaneto · 2025-01-15T09:48:10Z

src/posthog-core.ts

@@ -1285,13 +1285,13 @@ export class PostHog {
    }

    /** Get list of all surveys. */
-    getSurveys(callback: SurveyCallback, forceReload = false): void {
-        this.surveys.getSurveys(callback, forceReload)
+    getSurveys(callback: SurveyCallback): void {


Removing forceReload from public APIs will break customers that are using the methods directly.
Is there a reason to remove the forceReload?

It'll contradict whats written here:

Surveys are requested on first load and then cached by default by the JavaScript SDK. If you want to force an API call to get an updated list of surveys, pass true to the forceReload parameter. You only need to do this if you want changed surveys to appear mid-session for users.

Ewww - I wish we hadn't allowed people to do that tbh. But I can add it back in. We have a bug currently that causes multiple loads anyways

I mean it's useful that people do not have to call the API directly, since the "API" mode is one of the supported presentation mode, so yeah I'd add it back, fixing the bug is the extra sauce :D

Fix survey loading

d1f2514

vercel bot deployed to Preview January 15, 2025 08:00 View deployment

benjackwhite added 3 commits January 15, 2025 09:46

Fixes

5a196bf

Revert

10dc861

revert

5357c3b

vercel bot deployed to Preview January 15, 2025 08:51 View deployment

Fixes

8807aa8

benjackwhite changed the title ~~Load surveys via RemoteConfig~~ feat: Load surveys via RemoteConfig Jan 15, 2025

vercel bot deployed to Preview January 15, 2025 08:55 View deployment

benjackwhite added 2 commits January 15, 2025 10:24

Fix some things

5d7d4d4

Fixes

9ff477d

benjackwhite marked this pull request as ready for review January 15, 2025 09:26

benjackwhite added the bump minor Bump minor version when this PR gets merged label Jan 15, 2025

marandaneto requested review from lucasheriques, ioannisj and marandaneto January 15, 2025 09:29

vercel bot deployed to Preview January 15, 2025 09:31 View deployment

marandaneto reviewed Jan 15, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Load surveys via RemoteConfig #1652

feat: Load surveys via RemoteConfig #1652

benjackwhite commented Jan 15, 2025 •

edited

Loading

vercel bot commented Jan 15, 2025 •

edited

Loading

github-actions bot commented Jan 15, 2025 •

edited

Loading

marandaneto Jan 15, 2025

benjackwhite Jan 15, 2025

marandaneto Jan 15, 2025

benjackwhite Jan 15, 2025

marandaneto Jan 15, 2025

marandaneto commented Jan 15, 2025 •

edited

Loading

marandaneto Jan 15, 2025 •

edited

Loading

benjackwhite Jan 15, 2025

marandaneto Jan 15, 2025

feat: Load surveys via RemoteConfig #1652

Are you sure you want to change the base?

feat: Load surveys via RemoteConfig #1652

Conversation

benjackwhite commented Jan 15, 2025 • edited Loading

Changes

Follow up

Checklist

vercel bot commented Jan 15, 2025 • edited Loading

github-actions bot commented Jan 15, 2025 • edited Loading

marandaneto Jan 15, 2025

Choose a reason for hiding this comment

benjackwhite Jan 15, 2025

Choose a reason for hiding this comment

marandaneto Jan 15, 2025

Choose a reason for hiding this comment

benjackwhite Jan 15, 2025

Choose a reason for hiding this comment

marandaneto Jan 15, 2025

Choose a reason for hiding this comment

marandaneto commented Jan 15, 2025 • edited Loading

marandaneto Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

benjackwhite Jan 15, 2025

Choose a reason for hiding this comment

marandaneto Jan 15, 2025

Choose a reason for hiding this comment

benjackwhite commented Jan 15, 2025 •

edited

Loading

vercel bot commented Jan 15, 2025 •

edited

Loading

github-actions bot commented Jan 15, 2025 •

edited

Loading

marandaneto commented Jan 15, 2025 •

edited

Loading

marandaneto Jan 15, 2025 •

edited

Loading