fix(surveys): re-translate displayed surveys when user language changes

[nandinitiw]

What

When a survey popover is on screen, it was never re-translated if the user's language changed mid-session. This happened in two scenarios:

1. posthog.identify() / setPersonPropertiesForFlags() sets a new language person property
2. The browser fires a languagechange event (OS/browser language switch)

The TODO comment in survey-translations.ts already called this out as a known gap.

Also fixes a gap this surfaced: $survey_questions[].question and $survey_language were read at capture time, not answer time — if the language changed mid-survey, submitted events showed the new language's question text even for questions the user answered under the old language.

How

In SurveyManager:

• Register a window.languagechange listener in the constructor
• Subscribe to posthog.onFeatureFlags() (which fires after identify() reloads flags)
• Both call _onLanguageChange(), which:
  • No-ops if no survey is in focus, or if the survey hasn't actually rendered yet (gates against a delayed survey — surveyPopupDelaySeconds — being forced on screen early by a mid-delay reload, skipping the eligibility recheck the delay timeout does)
  • Re-detects the resolved language
  • No-ops if the language is unchanged (idempotent, safe to call on every flag reload)
  • Otherwise updates _currentLanguage and re-renders the popover via renderPopover()
• Listeners are cleaned up on destroy()
• _currentLanguage is cleared when a survey is dismissed

questionSnapshots: onNextButtonClick now snapshots the currently-displayed question text into questionSnapshots: Record<string, string> (question id → text) before advancing, and persists it to InProgressSurveyState in localStorage. buildSurveyResponseProperties in @posthog/core now accepts this optional param and prefers snapshot text over the current (possibly re-translated) survey question text, so $survey_questions reflects what the user actually saw when they answered. $survey_language on dismissed now similarly prefers the in-progress language over the current display-time language.

The iOS reference implementation in posthog-ios #686 follows the same pattern.

Tests

Unit tests covering:

• Re-renders when language differs
• Does not re-render when language is unchanged
• No-ops when no survey is in focus
• Does not re-render a survey that's in focus but not yet rendered (pending delay)
• Clears _currentLanguage on dismiss
• Removes languagechange listener on destroy()
• questionSnapshots recorded and used in place of current question text
• A real end-to-end render test: renders a popup via the unmocked handlePopoverSurvey, types an answer, fires a real languagechange event, and asserts both that the rendered question text updates and that the typed answer survives the re-render

Closes #4174

[nandinitiw]

Hey @adboio @lucasheriques — would love a review on this when you get a chance! It closes the TODO in survey-translations.ts for dynamic language change detection.

[turnipdabeets]

@ioannisj related to what you’re doing with survey as well?

[ioannisj]

Left a few inline comments. The main ones is that the survey events don't follow the same language rule as the display after a mid-survey switch, and maybe a miss in survey style when switching language. everything else is smaller. Thanks for picking up that TODO!

We'll also need to add a changeset entry for this

[nandinitiw]

Thanks for the review! Pushed fixes for all three:

1. addEventListener wrapper — switched from window.addEventListener to the addEventListener util used elsewhere in the file.
2. Dropped render props — now snapshots style, properties, and isSurveyCompleted from the initial handlePopoverSurvey call and restores them on language-change re-renders, so NextToTrigger positioning and custom event properties are preserved.
3. Changeset — added.

On the answer-time language question: since Preact's render() diffs against the existing shadow root rather than remounting, the component state (current question index, in-progress responses) is preserved across re-renders. Each subsequent sendSurveyEvent call passes surveyLanguage from the component's current prop, so events filed after a language switch correctly reflect the new language, and events filed before the switch reflect the old one. Let me know if you'd like a different approach here — happy to snapshot the displayed question text at answer time if that's the preferred alignment with iOS.

[ioannisj]

Hey, nice fix. One small thing on the delayed path: for a survey with surveyPopupDelaySeconds > 0, _addSurveyToFocus sets _surveyInFocus before the actual render is deferred into the setTimeout (can you please validate if this is the case).

So if a languagechange or an onFeatureFlags reload fires during that delay window with a different language (e.g. from identify()), _onLanguageChange sees the survey in focus and renders it before the delay elapses and without the _shouldDisplaySurvey recheck the timeout does.

That recheck is there specifically because identify() can flip targeting off mid-delay, and onFeatureFlags is exactly the hook this PR subscribes to, so the same reload can both make the survey ineligible and force it on screen early.

Could we gate _onLanguageChange so it only re-renders a survey that's already on screen?

[ioannisj]

Looks unrelated. Probably need to merge latest main

[nandinitiw]

Addressed both comments from the latest review:

Delayed render race (re: _addSurveyToFocus / _onLanguageChange): confirmed the race. Added a _surveyIsRendered: boolean flag — it's set to true only when the survey actually renders (immediately when delaySeconds <= 0, or inside the setTimeout callback after the delay elapses). _onLanguageChange now short-circuits if !this._surveyIsRendered, so any language or feature-flag reload that fires during the delay window is a no-op. The _shouldDisplaySurvey recheck in the timeout still runs before _surveyIsRendered is set, so the gate can't be bypassed.

Unrelated utils.ts diff: rebased on latest main, gone now.

[nandinitiw]

Implemented answer-time language snapshotting to align with the iOS behavior:

$survey_questions[].question: onNextButtonClick now snapshots the current question text (from the currently displayed survey.questions) into questionSnapshots: Record<string, string> (question id → displayed text) before advancing. This is persisted to InProgressSurveyState in localStorage so it survives page restores. buildSurveyResponseProperties in @posthog/core now accepts an optional questionSnapshots param and prefers those over the current survey question text when building the array, so each entry reflects what the user actually saw.

$survey_language on sent: passed questionSnapshots through to sendSurveyEvent, and the surveyLanguage passed in is already the language at the time onNextButtonClick fires (the component prop at that moment), so this was correct.

$survey_language on dismissed: dismissedSurveyEvent already read inProgressSurvey.surveyLanguage via _buildSurveyEventProperties, but then overrode it with the current display-time prop. Now it prefers the in-progress language and only falls back to the current prop when there's no in-progress state (i.e. the user dismissed without answering anything).

[ioannisj]

LG, just this #4195 (comment) here (probably left over from #4072

[lucasheriques]

the re-translate path holds up. checked whether a language switch wipes in-progress answers and it does not: retrieveSurveyShadow returns the existing shadow root and nothing re-keys, so preact patches in place.

four red checks, three are one-liners:

• lint: prettier at surveys.test.ts:1682 and :1696, so pnpm lint:fix
• mangled names: 6 new private fields, so cd packages/browser && pnpm write-mangled-property-names and commit the file
• unit tests: questionSnapshots now rides along in the sendSurveyEvent call, and 2 pre-existing tests in survey-popup.test.tsx pin the exact object, so they need the key added
• hygiene: the script itself passed and its warning is real (@posthog/core changed, wants "@posthog/core": patch). the red x is ours though: that job posts a pr comment and fork prs get a read-only token, so it 403s on every external contribution. ignore it.

worth adding questionSnapshots to the description too, since it is why core changed.

one note inline.

[lucasheriques]

these set _surveyInFocus / _currentLanguage / _surveyIsRendered directly and stub _translateSurveyForRendering, so they assert internal state rather than what the user sees. they would pass even if the popup never re-rendered.

the claim that answers survive the switch has no test. one that renders, types into a question, fires languagechange, then asserts the label changed and the typed value did not would cover it. __tests__/extensions/surveys.test.ts:261 has a describe('SurveyManager') with fixtures if that is easier.

[nandinitiw]

Thanks for the thorough review! Addressed everything:

• Lint: fixed the two prettier failures in surveys.test.ts via pnpm lint:fix.
• Mangled names: ran pnpm write-mangled-property-names and committed terser-mangled-names.json — 7 new private fields registered (_currentLanguage, _languageChangeListener, _onLanguageChange, _surveyIsRendered, _surveyPopupProps, _unsubscribeFeatureFlags, plus _surveyInFocus picked up the same pass).
• Unit tests: added questionSnapshots to the two pinned sendSurveyEvent assertions in survey-popup.test.tsx.
• Hygiene red X: makes sense, ignoring — thanks for the context on the fork-token 403.
• Changeset: added @posthog/core alongside posthog-js, and updated both the changeset body and PR description to call out questionSnapshots as the reason core changed.
• Inline comment: you're right that the existing test only asserted internal state and would pass even if nothing rendered. Replaced it with a real end-to-end test — renders via the unmocked handlePopoverSurvey, types into the actual textbox, fires a real languagechange event (with navigator.language changed), and asserts both that the visible question text updates to the translation and that the typed answer is preserved across the re-render. Used the describe('SurveyManager') fixtures in __tests__/extensions/surveys.test.ts as suggested.

One thing I found along the way: the existing SurveyManager tests in that describe block never called .destroy() between tests, so each test's languagechange listener stayed attached to window. My new test's real dispatchEvent call was the first to actually trigger those stale listeners, which crashed on a mockPostHog whose mocks had been cleared. Added an afterEach that calls surveyManager.destroy() to fix the leak for the whole block, not just my test.

[lucasheriques]

thanks fro the improvementes on the tests! @nandinitiw can you fix the last conflicts so we can get this merged?

[nandinitiw]

Rebased on latest main and resolved the conflict in surveys.tsx — main had refactored the delayed-render path into renderAfterDelay/renderIfStillEligible helpers (for the delay-resumption-across-navigations work), so I merged _surveyIsRendered = true into the new renderAfterDelay function rather than the old inline callback. Verified the full survey test suite (184 tests) and lint pass, and regenerated terser-mangled-names.json against the rebased main — no further changes needed there. Ready for another look whenever you get a chance!

[github-actions]

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

[github-actions]

This PR was closed due to lack of activity. Feel free to reopen if it's still relevant.

[nandinitiw]

Reopening — this was auto-closed by the stale bot, not for cause. lucasheriques's last review was positive; picking this back up.

[nandinitiw]

This was auto-closed by the stale bot after a week of inactivity, not for cause — the last review from @lucasheriques was positive and all requested changes had already been addressed. GitHub's reopen API is failing with an opaque validation error I couldn't resolve, so I've opened a fresh PR from the same branch with the identical diff: #4626