Skip to content

feat(statsig): enhance Statsig client initialization and configuration#2933

Merged
eldadfux merged 1 commit intomainfrom
feat/statsig-marketing-ssr
Apr 28, 2026
Merged

feat(statsig): enhance Statsig client initialization and configuration#2933
eldadfux merged 1 commit intomainfrom
feat/statsig-marketing-ssr

Conversation

@eldadfux
Copy link
Copy Markdown
Member

  • Added STATSIG_ENVIRONMENT variable to .env.example for environment configuration.
  • Introduced shutdown method in StatsigBrowserClient for graceful client termination.
  • Improved Statsig client initialization to include environment tier and stable user ID handling.
  • Updated server-side Statsig client to utilize environment options for better configuration.
  • Modified layout and marketing page to pass stable user ID to client initialization, ensuring consistency.

Made-with: Cursor

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.)

- Added STATSIG_ENVIRONMENT variable to .env.example for environment configuration.
- Introduced shutdown method in StatsigBrowserClient for graceful client termination.
- Improved Statsig client initialization to include environment tier and stable user ID handling.
- Updated server-side Statsig client to utilize environment options for better configuration.
- Modified layout and marketing page to pass stable user ID to client initialization, ensuring consistency.

Made-with: Cursor
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 28, 2026

Greptile Summary

This PR enhances the Statsig integration by adding environment-tier configuration (STATSIG_ENVIRONMENT), a graceful shutdown() method, and a stable-user-ID handoff between SSR and the browser client. It also moves initStatsig from onMount to afterNavigate so SPA navigations back to / can still bootstrap from server data.

The core logic in client.ts is sound, but the shared module-level pendingBootstrapJson state introduces two edge cases: rapid navigation can silently overwrite a pending bootstrap before the in-flight runStatsigInit consumes it, and the reinit guard (appliedBootstrapPayload == null) has no in-progress sentinel, leaving a theoretical window for double-reinit on concurrent afterNavigate firings.

Confidence Score: 4/5

Safe to merge — both findings are edge-case race conditions in P2 territory that self-correct on the next navigation, with no data loss or security impact.

All findings are P2. The bootstrap-overwrite race requires very rapid navigation and self-heals on the next afterNavigate. The double-reinit window requires two near-simultaneous afterNavigate events, which SvelteKit's sequential navigation model makes unlikely. No P0/P1 issues found.

src/lib/statsig/client.ts — the pendingBootstrapJson overwrite logic and reinit guard in initStatsig.

Important Files Changed

Filename Overview
src/lib/statsig/client.ts Refactored Statsig browser init into runStatsigInit + wrapInitFailure helpers; added environment tier, stable-ID override, and bootstrap/reinit tracking — two edge-case race conditions around pendingBootstrapJson overwrite and double-reinit triggering.
src/lib/statsig/hero-statsig.server.ts Adds buildStatsigServerOptions() to supply environment tier (string, as per @statsig/statsig-node-core API) from STATSIG_ENVIRONMENT env var, falling back to NODE_ENV. Straightforward and correct.
src/routes/(marketing)/+page.server.ts Adds statsigStableUserId to the load return value so the client can match the SSR stable ID; simple and correct.
src/routes/+layout.svelte Moves initStatsig from onMount to afterNavigate and passes both statsigBootstrap and statsigStableUserId — correctly handles SPA navigations; afterNavigate fires on initial load in SvelteKit so no regression.
.env.example Adds STATSIG_ENVIRONMENT variable with a clear documentation comment explaining defaults and expected values.

Reviews (1): Last reviewed commit: "feat(statsig): enhance Statsig client in..." | Re-trigger Greptile

Comment thread src/lib/statsig/client.ts
Comment on lines +226 to +229
pendingBootstrapJson =
typeof clientBootstrapJson === 'string' && clientBootstrapJson.length > 0
? clientBootstrapJson
: undefined;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Bootstrap payload silently cleared on rapid navigation

initStatsig unconditionally overwrites pendingBootstrapJson with undefined when called with a null/empty bootstrap (e.g., navigating away from / before the in-flight runStatsigInit has consumed the value). If the user navigates //docs quickly, the second afterNavigate call clears the bootstrap before runStatsigInit reaches line 123. The in-flight init then sees bootstrap = undefined and falls through to initializeAsync without the SSR data. The reinit-on-next-navigate path self-corrects, but the initial homepage render loses the bootstrap alignment.

A safer pattern would be to only update the pending values when the init hasn't yet consumed them (e.g., guard with if (!initPromise || client) before overwriting).

Comment thread src/lib/statsig/client.ts
Comment on lines +234 to +248
if (client && hasBootstrap && appliedBootstrapPayload == null) {
const previous = client as unknown as StatsigBrowserClient;
client = null;
initPromise = wrapInitFailure(
(async (): Promise<StatsigBrowserClient | null> => {
try {
await previous.shutdown();
} catch {
/* ignore */
}
return runStatsigInit();
})()
);
return whenStatsigNetworkReady();
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Reinit triggered on every navigation to / after non-bootstrap start

The condition client && hasBootstrap && appliedBootstrapPayload == null triggers a full shutdown() + reinit every time a navigation brings bootstrap data if appliedBootstrapPayload is still null. This is intentional for the /docs → / case, but if afterNavigate fires twice in quick succession (e.g., hash change then real navigation) for the homepage, two concurrent reinit chains could run — the second one shutting down the client the first one is still initializing. appliedBootstrapPayload is only set after runStatsigInit returns successfully, so a slow init leaves the window open.

Consider setting appliedBootstrapPayload to a sentinel value (e.g., 'pending') at the top of the reinit path to prevent double-triggering while the shutdown/reinit is in-flight.

@eldadfux eldadfux merged commit 41c05fc into main Apr 28, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant