From 1833f0d9fe92e9861add82d25941150725d67e0b Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Wed, 8 Nov 2023 13:22:51 -0800 Subject: [PATCH 1/3] fix: call fetch on start by default --- packages/experiment-browser/src/config.ts | 5 ++-- .../src/experimentClient.ts | 28 +++---------------- .../experiment-browser/test/client.test.ts | 4 +-- 3 files changed, 8 insertions(+), 29 deletions(-) diff --git a/packages/experiment-browser/src/config.ts b/packages/experiment-browser/src/config.ts index 379911a0..b09ce5de 100644 --- a/packages/experiment-browser/src/config.ts +++ b/packages/experiment-browser/src/config.ts @@ -87,8 +87,7 @@ export interface ExperimentConfig { * * - `true`: fetch will always be called on start. * - `false`: fetch will never be called on start. - * - `undefined`: the SDK will determine whether to call fetch based on the - * flags returned in the result. + * - `undefined`: fetch will always be called on start. */ fetchOnStart?: boolean; @@ -175,7 +174,7 @@ export const Defaults: ExperimentConfig = { retryFetchOnFailure: true, automaticExposureTracking: true, pollOnStart: true, - fetchOnStart: undefined, + fetchOnStart: true, automaticFetchOnAmplitudeIdentityChange: false, userProvider: null, analyticsProvider: null, diff --git a/packages/experiment-browser/src/experimentClient.ts b/packages/experiment-browser/src/experimentClient.ts index 77be974b..23eba5f1 100644 --- a/packages/experiment-browser/src/experimentClient.ts +++ b/packages/experiment-browser/src/experimentClient.ts @@ -163,8 +163,8 @@ export class ExperimentClient implements Client { * the flag configurations cached locally or received in the initial flag * configuration response. * - * To explicitly force this request to fetch or not, set the - * {@link fetchOnStart} configuration option when initializing the SDK. + * To force this request not to fetch, set the {@link fetchOnStart} + * configuration option to `false` when initializing the SDK. * * Finally, this function will start polling for flag configurations at a * fixed interval. To disable polling, set the {@link pollOnStart} @@ -183,32 +183,12 @@ export class ExperimentClient implements Client { this.isRunning = true; } this.setUser(user); - // Get flag configurations, and simultaneously check the local cache for - // remote evaluation flags. const flagsReadyPromise = this.doFlags(); - let remoteFlags = - this.config.fetchOnStart ?? - Object.values(this.flags.getAll()).some((flag) => - isRemoteEvaluationMode(flag), - ); - if (remoteFlags) { - // We already have remote flags in our flag cache, so we know we need to - // evaluate remotely even before we've updated our flags. + const fetchOnStart = this.config.fetchOnStart ?? true; + if (fetchOnStart) { await Promise.all([this.fetch(user), flagsReadyPromise]); } else { - // We don't know if remote evaluation is required, await the flag promise, - // and recheck for remote flags. await flagsReadyPromise; - remoteFlags = - this.config.fetchOnStart ?? - Object.values(this.flags.getAll()).some((flag) => - isRemoteEvaluationMode(flag), - ); - if (remoteFlags) { - // We already have remote flags in our flag cache, so we know we need to - // evaluate remotely even before we've updated our flags. - await this.fetch(user); - } } if (this.config.pollOnStart) { this.poller.start(); diff --git a/packages/experiment-browser/test/client.test.ts b/packages/experiment-browser/test/client.test.ts index c28bb716..6f9a63c5 100644 --- a/packages/experiment-browser/test/client.test.ts +++ b/packages/experiment-browser/test/client.test.ts @@ -1010,7 +1010,7 @@ describe('start', () => { await client.start(); expect(fetchSpy).toBeCalledTimes(1); }, 10000); - test('with local evaluation only, does not call fetch', async () => { + test('with local evaluation only, calls fetch', async () => { const client = new ExperimentClient(API_KEY, {}); mockClientStorage(client); // eslint-disable-next-line @typescript-eslint/ban-ts-comment @@ -1020,7 +1020,7 @@ describe('start', () => { }; const fetchSpy = jest.spyOn(client, 'fetch'); await client.start(); - expect(fetchSpy).toBeCalledTimes(0); + expect(fetchSpy).toBeCalledTimes(1); }); test('with local evaluation only, fetchOnStart enabled, calls fetch', async () => { From bb128ce552370d36183d305303198bd934d2e138 Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Wed, 8 Nov 2023 14:30:16 -0800 Subject: [PATCH 2/3] fix: comment --- packages/experiment-browser/src/experimentClient.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/experiment-browser/src/experimentClient.ts b/packages/experiment-browser/src/experimentClient.ts index 23eba5f1..753addcf 100644 --- a/packages/experiment-browser/src/experimentClient.ts +++ b/packages/experiment-browser/src/experimentClient.ts @@ -159,11 +159,7 @@ export class ExperimentClient implements Client { * local flag configurations have been updated, and the {@link fetch()} * result has been received (if the request was made). * - * This function determines whether to {@link fetch()} based on the result of - * the flag configurations cached locally or received in the initial flag - * configuration response. - * - * To force this request not to fetch, set the {@link fetchOnStart} + * To force this function not to fetch variants, set the {@link fetchOnStart} * configuration option to `false` when initializing the SDK. * * Finally, this function will start polling for flag configurations at a From f295dc4baf24c51643d08df8d4ff285726b9ea5b Mon Sep 17 00:00:00 2001 From: Brian Giori Date: Wed, 8 Nov 2023 15:34:08 -0800 Subject: [PATCH 3/3] fix: defaults comment --- packages/experiment-browser/src/config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/experiment-browser/src/config.ts b/packages/experiment-browser/src/config.ts index b09ce5de..a9e3d2d2 100644 --- a/packages/experiment-browser/src/config.ts +++ b/packages/experiment-browser/src/config.ts @@ -152,7 +152,7 @@ export interface ExperimentConfig { | **retryFailedAssignment** | `true` | | **automaticExposureTracking** | `true` | | **pollOnStart** | `true` | - | **fetchOnStart** | `undefined` | + | **fetchOnStart** | `true` | | **automaticFetchOnAmplitudeIdentityChange** | `false` | | **userProvider** | `null` | | **analyticsProvider** | `null` |