fix: call fetch on start by default

amplitude · Nov 8, 2023 · 1833f0d · 1833f0d
1 parent febe36e
commit 1833f0d
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 29 deletions.
diff --git 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
@@ -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
@@ -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 () => {