Conversation
- Enhanced the `evaluateHeroDescriptionExperiment` function documentation to clarify the need for client-side exposure logging. - Implemented `onMount` in the hero component to log exposure for the `best_description` experiment after SSR, ensuring accurate enrollment tracking in Statsig. Made-with: Cursor
…support - Added support for passing `userAgent` during Statsig client initialization to avoid bootstrap user mismatch warnings. - Updated relevant functions and components to handle the new `statsigUserAgent` parameter, ensuring consistency across server and client initialization. Made-with: Cursor
Greptile SummaryThis PR threads the server request's Confidence Score: 4/5Safe to merge; changes are narrowly scoped to Statsig bootstrap alignment with no functional regressions. Only P2 findings: stale module-level pending state (pre-existing pattern, harmless) and a minor readability note in hero.svelte. No logic errors or security concerns. No files require special attention. Important Files Changed
Reviews (1): Last reviewed commit: "fix" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| pendingStatsigUserAgent = | ||
| typeof bootstrapUserAgent === 'string' && bootstrapUserAgent.length > 0 | ||
| ? bootstrapUserAgent | ||
| : undefined; |
There was a problem hiding this comment.
initStatsig calls
When initStatsig is called on a navigation where the client is already fully bootstrapped (client != null && appliedBootstrapPayload != null), it sets pendingStatsigUserAgent (and the other two pending variables) but the code falls through to the final return whenStatsigNetworkReady() without triggering a new runStatsigInit. The stale value then sits in the module variable until the next call that does trigger a re-init. The same pattern exists for pendingBootstrapJson and pendingServerStableUserId, so this is consistent, but worth noting that on non-/ navigations back to /, the pending UA value is left unconsumed.
| * exposure when the user actually sees the hero. Reading `description` logs the exposure and | ||
| * keeps the rendered line as the SSR value (default matches props). | ||
| * @see https://docs.statsig.com/pulse (exposures enroll units in the experiment) | ||
| */ | ||
| onMount(() => { | ||
| if (!browser || ENV.TEST) return; | ||
| void whenStatsigReady().then((client) => { |
There was a problem hiding this comment.
subtitle fallback
client.getExperiment(...).get('description', subtitle) uses the subtitle prop as the Statsig fallback. If the client bootstrap assigns a different variant than what the server evaluated (e.g., bootstrap mismatch that this PR is trying to fix), the return value of get will differ from the rendered text, but the return value is intentionally discarded so the DOM isn't updated. This is correct for the exposure-only pattern, but the discarded return makes the call's intent non-obvious; a brief inline comment clarifying that the return value is deliberately ignored would help future readers.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
What does this PR do?
(Provide a description of what this PR does.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)