amplitude / experiment-js-client

Amplitude Experiment client-side SDK for JavaScript
MIT License
8 stars 7 forks source link

InitialVariants from SSR & automaticFetchOnAmplitudeIdentityChange #85

Open michal-filip opened 1 year ago

michal-filip commented 1 year ago

It looks like the combination of initialVariants provided during server side rendering together with automaticFetchOnAmplitudeIdentityChange: true is not supported. My idea was that durring SSR we compute the flags and should the identity later change on the client (on login/logout), new variants would be fetched and preferred over initialVariants.

First issue I ran into was that at the moment of initialization of experiment SDK in the browser, analytics SDK is not yet ready - it takes a bit before it sets the initial identity provided to amplitude.init call (this results in an unnecessary call to vardata endpoint). So I was left with calling the experiment client's fetch method manually. But then, even after the new variants were resolved for the new identity and stored into experiment client's storage field, the initialVariants were still preferred. This seems to be the current behavior as per https://github.com/amplitude/experiment-js-client/blob/main/packages/experiment-browser/src/experimentClient.ts#L264

It seems like I can live with this behavior and create a new instance of experiment client if needed, just figured I'd raise this as something you might want to support.

bgiori commented 1 year ago

Hi @michal-filip, thanks for this feedback.

Heads up we're most likely going to remove the automaticFetchOnAmplitudeIdentityChange configuration in the next major version release for this reason: the analytics SDK often "staggers" updates the the user, (i.e. init, setUserId, setUserProperties) which triggers many calls to fetch where each call may have different responses. This is (a) spammy, and (b) often causes variant jumping.

For storage, I would suggest to use the LocalStorage source, however that would mean that exposure events would not be tracked for variants accessed from initialVariants.

It seems like a more comprehensive and configurable configuration is required. Again I appreciate this feedback!