fix: survey preview better bundle size #1665
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Size Change: -10.3 kB (-0.32%) Total Size: 3.25 MB
ℹ️ View Unchanged
|
|
hey @pauldambra appreciate if you could take a look!! from the previous PR, #1661, which increased size by 9.2kb, this one decreased it by 10.3kb. I tested it locally both on the nextjs playground, and also on the posthog/posthog ponting to the file, looks like everything worked. can you double check to see if I did anything weird? |
|
very quick and tired brain scan but that looks great to me! |
| if (isNullish(assignableWindow.__PosthogExtensions__?.getNextSurveyStep)) { | ||
| logger.warn('init was not called') | ||
| return 0 | ||
| } |
There was a problem hiding this comment.
Lets do better logging here similar to #1663
so we know if the problem is the PosthogExtensions not loaded, or the getNextSurveyStep somehow wasn't available
| @@ -844,6 +843,9 @@ describe('surveys', () => { | |||
| }) | |||
|
|
|||
| describe('branching logic', () => { | |||
| beforeEach(() => { | |||
| surveys.getNextSurveyStep = getNextSurveyStep | |||
There was a problem hiding this comment.
i tend to mock the external dependency loader so that more of the flow is being exercised...
so e.g. in session replay
loadScriptMock.mockImplementation((_ph, _path, callback) => {
addRRwebToWindow()
callback()
})
assignableWindow.__PosthogExtensions__.loadExternalDependency = loadScriptMock
```
otherwise that startup can be broken and tests carry on passing
There was a problem hiding this comment.
ofc explicit tests somewhere else for it is totally fine too
| @@ -78,6 +78,7 @@ interface PostHogExtensions { | |||
| rrwebPlugins?: { getRecordConsolePlugin: any; getRecordNetworkPlugin?: any } | |||
| canActivateRepeatedly?: (survey: any) => boolean | |||
| generateSurveys?: (posthog: PostHog) => any | undefined | |||
| getNextSurveyStep?: (survey: any, currentQuestionIndex: number, response: string | string[] | number | null) => any | |||
There was a problem hiding this comment.
we can't move generateSurveys because older clients are already reading from that location...
but it might be nice to put a surveys on here so you'd have
// deprecated: still in place for older clients but you should use `surveys.generateSurveys`
generateSurveys?: (posthog: PostHog) => any | undefined
surveys: {
generateSurveys?: (posthog: PostHog) => any | undefined,
getNextSurveyStep?: (survey: any, currentQuestionIndex: number, response: string | string[] | number | null) =>
any
}
so that over time it stays obvious what surveys has put on the window
not a hard requirement but i think tidier in the long run (and i wish we'd started doing this earlier)
Changes
no functional changes
changes some imports to optimize for bundle size
Checklist