Merge pull request #354 from splitio/init_function_for_side_effects

EmilianoSanchez · web-flow · commit e81bd2f2f365 · 2024-10-18T12:04:23.000-03:00
Add internal `init` function in SdkFactory to wrap initialization side effects
diff --git a/CHANGES.txt b/CHANGES.txt
@@ -1,6 +1,7 @@
-2.0.0 (September XX, 2024)
+2.0.0 (October XX, 2024)
  - Added support for targeting rules based on large segments.
  - Added `factory.destroy()` method, which invokes the `destroy` method on all SDK clients created by the factory.
+ - Updated the handling of timers and async operations by moving them into an `init` factory method to enable lazy initialization of the SDK. This update is intended for the React SDK.
  - Bugfixing - Fixed an issue with the server-side polling manager that caused dangling timers when the SDK was destroyed before it was ready.
  - BREAKING CHANGES:
    - Updated default flag spec version to 1.2.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@splitsoftware/splitio-commons",
-  "version": "1.17.1-rc.1",
+  "version": "1.17.1-rc.3",
   "description": "Split JavaScript SDK common components",
   "main": "cjs/index.js",
   "module": "esm/index.js",
diff --git a/src/readiness/__tests__/readinessManager.spec.ts b/src/readiness/__tests__/readinessManager.spec.ts
@@ -157,13 +157,14 @@ test('READINESS MANAGER / Segment updates should not be propagated', (done) => {
   });
 });
 
-describe('READINESS MANAGER / Timeout ready event', () => {
+describe('READINESS MANAGER / Timeout event', () => {
   let readinessManager: IReadinessManager;
   let timeoutCounter: number;
 
   beforeEach(() => {
     // Schedule timeout to be fired before SDK_READY
     readinessManager = readinessManagerFactory(EventEmitter, settingsWithTimeout);
+    readinessManager.init(); // Start the timeout
     timeoutCounter = 0;
 
     readinessManager.gate.on(SDK_READY_TIMED_OUT, () => {
@@ -212,6 +213,7 @@ test('READINESS MANAGER / Cancel timeout if ready fired', (done) => {
   let sdkReadyTimedoutCalled = false;
 
   const readinessManager = readinessManagerFactory(EventEmitter, settingsWithTimeout);
+  readinessManager.init(); // Start the timeout
 
   readinessManager.gate.on(SDK_READY_TIMED_OUT, () => { sdkReadyTimedoutCalled = true; });
   readinessManager.gate.once(SDK_READY, () => { sdkReadyCalled = true; });
diff --git a/src/readiness/readinessManager.ts b/src/readiness/readinessManager.ts
@@ -7,6 +7,8 @@ function splitsEventEmitterFactory(EventEmitter: new () => IEventEmitter): ISpli
   const splitsEventEmitter = objectAssign(new EventEmitter(), {
     splitsArrived: false,
     splitsCacheLoaded: false,
+    initialized: false,
+    initCallbacks: []
   });
 
   // `isSplitKill` condition avoids an edge-case of wrongly emitting SDK_READY if:
@@ -56,16 +58,17 @@ export function readinessManagerFactory(
   // emit SDK_READY_TIMED_OUT
   let hasTimedout = false;
 
-  function timeout() {
-    if (hasTimedout) return;
+  function timeout() { // eslint-disable-next-line no-use-before-define
+    if (hasTimedout || isReady) return;
     hasTimedout = true;
     syncLastUpdate();
     gate.emit(SDK_READY_TIMED_OUT, 'Split SDK emitted SDK_READY_TIMED_OUT event.');
   }
 
   let readyTimeoutId: ReturnType<typeof setTimeout>;
   if (readyTimeout > 0) {
-    readyTimeoutId = setTimeout(timeout, readyTimeout);
+    if (splits.initialized) readyTimeoutId = setTimeout(timeout, readyTimeout);
+    else splits.initCallbacks.push(() => { readyTimeoutId = setTimeout(timeout, readyTimeout); });
   }
 
   // emit SDK_READY and SDK_UPDATE
@@ -132,6 +135,12 @@ export function readinessManagerFactory(
     // tracking and evaluations, while keeping event listeners to emit SDK_READY_TIMED_OUT event
     setDestroyed() { isDestroyed = true; },
 
+    init() {
+      if (splits.initialized) return;
+      splits.initialized = true;
+      splits.initCallbacks.forEach(cb => cb());
+    },
+
     destroy() {
       isDestroyed = true;
       syncLastUpdate();
diff --git a/src/readiness/types.ts b/src/readiness/types.ts
@@ -12,6 +12,8 @@ export interface ISplitsEventEmitter extends IEventEmitter {
   once(event: ISplitsEvent, listener: (...args: any[]) => void): this;
   splitsArrived: boolean
   splitsCacheLoaded: boolean
+  initialized: boolean,
+  initCallbacks: (() => void)[]
 }
 
 /** Segments data emitter */
@@ -59,6 +61,7 @@ export interface IReadinessManager {
   timeout(): void,
   setDestroyed(): void,
   destroy(): void,
+  init(): void,
 
   /** for client-side */
   shared(): IReadinessManager,
diff --git a/src/sdkClient/__tests__/sdkClientMethodCS.spec.ts b/src/sdkClient/__tests__/sdkClientMethodCS.spec.ts
@@ -14,15 +14,15 @@ const storageMock = {
   })
 };
 
-const partialSdkReadinessManagers: { sdkStatus: jest.Mock, readinessManager: { destroy: jest.Mock } }[] = [];
+const partialSdkReadinessManagers: { sdkStatus: jest.Mock, readinessManager: { init: jest.Mock, destroy: jest.Mock } }[] = [];
 
 const sdkReadinessManagerMock = {
   sdkStatus: jest.fn(),
-  readinessManager: { destroy: jest.fn() },
+  readinessManager: { init: jest.fn(), destroy: jest.fn() },
   shared: jest.fn(() => {
     partialSdkReadinessManagers.push({
       sdkStatus: jest.fn(),
-      readinessManager: { destroy: jest.fn() },
+      readinessManager: { init: jest.fn(), destroy: jest.fn() },
     });
     return partialSdkReadinessManagers[partialSdkReadinessManagers.length - 1];
   })
@@ -46,7 +46,7 @@ const params = {
   signalListener: { stop: jest.fn() },
   settings: settingsWithKey,
   telemetryTracker: telemetryTrackerFactory(),
-  clients: {}
+  clients: {},
 };
 
 const invalidAttributes = [
diff --git a/src/sdkClient/sdkClientMethodCS.ts b/src/sdkClient/sdkClientMethodCS.ts
@@ -75,8 +75,6 @@ export function sdkClientMethodCSFactory(params: ISdkFactoryContext): (key?: Spl
         validKey
       );
 
-      sharedSyncManager && sharedSyncManager.start();
-
       log.info(NEW_SHARED_CLIENT);
     } else {
       log.debug(RETRIEVE_CLIENT_EXISTING);
diff --git a/src/sdkClient/sdkClientMethodCSWithTT.ts b/src/sdkClient/sdkClientMethodCSWithTT.ts
@@ -86,8 +86,6 @@ export function sdkClientMethodCSFactory(params: ISdkFactoryContext): (key?: Spl
         validTrafficType
       );
 
-      sharedSyncManager && sharedSyncManager.start();
-
       log.info(NEW_SHARED_CLIENT);
     } else {
       log.debug(RETRIEVE_CLIENT_EXISTING);
diff --git a/src/sdkFactory/index.ts b/src/sdkFactory/index.ts
@@ -23,14 +23,20 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO.
   const { settings, platform, storageFactory, splitApiFactory, extraProps,
     syncManagerFactory, SignalListener, impressionsObserverFactory,
     integrationsManagerFactory, sdkManagerFactory, sdkClientMethodFactory,
-    filterAdapterFactory } = params;
+    filterAdapterFactory, lazyInit } = params;
   const { log, sync: { impressionsMode } } = settings;
 
   // @TODO handle non-recoverable errors, such as, global `fetch` not available, invalid SDK Key, etc.
   // On non-recoverable errors, we should mark the SDK as destroyed and not start synchronization.
 
-  // We will just log and allow for the SDK to end up throwing an SDK_TIMEOUT event for devs to handle.
-  validateAndTrackApiKey(log, settings.core.authorizationKey);
+  // initialization
+  let hasInit = false;
+  const initCallbacks: (() => void)[] = [];
+
+  function whenInit(cb: () => void) {
+    if (hasInit) cb();
+    else initCallbacks.push(cb);
+  }
 
   const sdkReadinessManager = sdkReadinessManagerFactory(platform.EventEmitter, settings);
   const readiness = sdkReadinessManager.readinessManager;
@@ -67,8 +73,8 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO.
       strategy = strategyDebugFactory(observer);
   }
 
-  const impressionsTracker = impressionsTrackerFactory(settings, storage.impressions, strategy, integrationsManager, storage.telemetry);
-  const eventTracker = eventTrackerFactory(settings, storage.events, integrationsManager, storage.telemetry);
+  const impressionsTracker = impressionsTrackerFactory(settings, storage.impressions, strategy, whenInit, integrationsManager, storage.telemetry);
+  const eventTracker = eventTrackerFactory(settings, storage.events, whenInit, integrationsManager, storage.telemetry);
 
   // splitApi is used by SyncManager and Browser signal listener
   const splitApi = splitApiFactory && splitApiFactory(settings, platform, telemetryTracker);
@@ -85,8 +91,21 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO.
   const clientMethod = sdkClientMethodFactory(ctx);
   const managerInstance = sdkManagerFactory(settings, storage.splits, sdkReadinessManager);
 
-  syncManager && syncManager.start();
-  signalListener && signalListener.start();
+
+  function init() {
+    if (hasInit) return;
+    hasInit = true;
+
+    // We will just log and allow for the SDK to end up throwing an SDK_TIMEOUT event for devs to handle.
+    validateAndTrackApiKey(log, settings.core.authorizationKey);
+    readiness.init();
+    uniqueKeysTracker && uniqueKeysTracker.start();
+    syncManager && syncManager.start();
+    signalListener && signalListener.start();
+
+    initCallbacks.forEach((cb) => cb());
+    initCallbacks.length = 0;
+  }
 
   log.info(NEW_FACTORY);
 
@@ -107,7 +126,7 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO.
     settings,
 
     destroy() {
-      return Promise.all(Object.keys(clients).map(key => clients[key].destroy())).then(() => {});
+      return Promise.all(Object.keys(clients).map(key => clients[key].destroy())).then(() => { });
     }
-  }, extraProps && extraProps(ctx));
+  }, extraProps && extraProps(ctx), lazyInit ? { init } : init());
 }
diff --git a/src/sdkFactory/types.ts b/src/sdkFactory/types.ts
@@ -68,6 +68,8 @@ export interface ISdkFactoryContextAsync extends ISdkFactoryContext {
  * Object parameter with the modules required to create an SDK factory instance
  */
 export interface ISdkFactoryParams {
+  // If true, the `sdkFactory` is pure (no side effects), and the SDK instance includes a `init` method to run initialization side effects
+  lazyInit?: boolean,
 
   // The settings must be already validated
   settings: ISettings,
diff --git a/src/storages/inMemory/InMemoryStorageCS.ts b/src/storages/inMemory/InMemoryStorageCS.ts
@@ -63,7 +63,7 @@ export function InMemoryStorageCSFactory(params: IStorageFactoryParams): IStorag
   };
 
   // @TODO revisit storage logic in localhost mode
-  // No tracking data in localhost mode to avoid memory leaks
+  // No tracking in localhost mode to avoid memory leaks: https://github.com/splitio/javascript-commons/issues/181
   if (params.settings.mode === LOCALHOST_MODE) {
     const noopTrack = () => true;
     storage.impressions.track = noopTrack;
diff --git a/src/sync/__tests__/syncManagerOnline.spec.ts b/src/sync/__tests__/syncManagerOnline.spec.ts
@@ -125,12 +125,9 @@ test('syncManagerOnline should syncAll a single time when sync is disabled', ()
 
   if (!pollingSyncManagerShared) throw new Error('pollingSyncManagerShared should exist');
 
-  pollingSyncManagerShared.start();
-
   expect(pollingManagerMock.start).not.toBeCalled();
 
   pollingSyncManagerShared.stop();
-  pollingSyncManagerShared.start();
 
   expect(pollingManagerMock.start).not.toBeCalled();
 
@@ -153,12 +150,9 @@ test('syncManagerOnline should syncAll a single time when sync is disabled', ()
 
   if (!pushingSyncManagerShared) throw new Error('pushingSyncManagerShared should exist');
 
-  pushingSyncManagerShared.start();
-
   expect(pollingManagerMock.start).not.toBeCalled();
 
   pushingSyncManagerShared.stop();
-  pushingSyncManagerShared.start();
 
   expect(pollingManagerMock.start).not.toBeCalled();
 
diff --git a/src/sync/syncManagerOnline.ts b/src/sync/syncManagerOnline.ts
@@ -143,27 +143,29 @@ export function syncManagerOnlineFactory(
 
         const mySegmentsSyncTask = (pollingManager as IPollingManagerCS).add(matchingKey, readinessManager, storage);
 
-        return {
-          isRunning: mySegmentsSyncTask.isRunning,
-          start() {
-            if (syncEnabled) {
-              if (pushManager) {
-                if (pollingManager!.isRunning()) {
-                  // if doing polling, we must start the periodic fetch of data
-                  if (storage.splits.usesSegments()) mySegmentsSyncTask.start();
-                } else {
-                  // if not polling, we must execute the sync task for the initial fetch
-                  // of segments since `syncAll` was already executed when starting the main client
-                  mySegmentsSyncTask.execute();
-                }
-                pushManager.add(matchingKey, mySegmentsSyncTask);
-              } else {
+        if (running) {
+          if (syncEnabled) {
+            if (pushManager) {
+              if (pollingManager!.isRunning()) {
+                // if doing polling, we must start the periodic fetch of data
                 if (storage.splits.usesSegments()) mySegmentsSyncTask.start();
+              } else {
+                // if not polling, we must execute the sync task for the initial fetch
+                // of segments since `syncAll` was already executed when starting the main client
+                mySegmentsSyncTask.execute();
               }
+              pushManager.add(matchingKey, mySegmentsSyncTask);
             } else {
-              if (!readinessManager.isReady()) mySegmentsSyncTask.execute();
+              if (storage.splits.usesSegments()) mySegmentsSyncTask.start();
             }
-          },
+          } else {
+            if (!readinessManager.isReady()) mySegmentsSyncTask.execute();
+          }
+        }
+
+        return {
+          isRunning: mySegmentsSyncTask.isRunning,
+
           stop() {
             // check in case `client.destroy()` has been invoked more than once for the same client
             const mySegmentsSyncTask = (pollingManager as IPollingManagerCS).get(matchingKey);
diff --git a/src/sync/types.ts b/src/sync/types.ts
@@ -44,5 +44,5 @@ export interface ISyncManager extends ITask {
 }
 
 export interface ISyncManagerCS extends ISyncManager {
-  shared(matchingKey: string, readinessManager: IReadinessManager, storage: IStorageSync): ISyncManager | undefined
+  shared(matchingKey: string, readinessManager: IReadinessManager, storage: IStorageSync): Pick<ISyncManager, 'stop' | 'flush'> | undefined
 }
diff --git a/src/trackers/__tests__/eventTracker.spec.ts b/src/trackers/__tests__/eventTracker.spec.ts
@@ -29,13 +29,15 @@ const fakeEvent = {
   }
 };
 
+const fakeWhenInit = (cb: () => void) => cb();
+
 /* Tests */
 describe('Event Tracker', () => {
 
   test('Tracker API', () => {
     expect(typeof eventTrackerFactory).toBe('function'); // The module should return a function which acts as a factory.
 
-    const instance = eventTrackerFactory(fullSettings, fakeEventsCache, fakeIntegrationsManager);
+    const instance = eventTrackerFactory(fullSettings, fakeEventsCache, fakeWhenInit, fakeIntegrationsManager);
 
     expect(typeof instance.track).toBe('function'); // The instance should implement the track method.
   });
@@ -51,7 +53,7 @@ describe('Event Tracker', () => {
       }
     });
     // @ts-ignore
-    const tracker = eventTrackerFactory(fullSettings, fakeEventsCache, fakeIntegrationsManager, fakeTelemetryCache);
+    const tracker = eventTrackerFactory(fullSettings, fakeEventsCache, fakeWhenInit, fakeIntegrationsManager, fakeTelemetryCache);
     const result1 = tracker.track(fakeEvent, 1);
 
     expect(fakeEventsCache.track.mock.calls[0]).toEqual([fakeEvent, 1]); // Should be present in the event cache.
@@ -92,7 +94,7 @@ describe('Event Tracker', () => {
     const settings = { ...fullSettings };
     const fakeEventsCache = { track: jest.fn(() => true) };
 
-    const tracker = eventTrackerFactory(settings, fakeEventsCache);
+    const tracker = eventTrackerFactory(settings, fakeEventsCache, fakeWhenInit);
 
     expect(tracker.track(fakeEvent)).toBe(true);
     expect(fakeEventsCache.track).toBeCalledTimes(1); // event should be tracked if userConsent is undefined
diff --git a/src/trackers/__tests__/impressionsTracker.spec.ts b/src/trackers/__tests__/impressionsTracker.spec.ts
diff --git a/src/trackers/__tests__/uniqueKeysTracker.spec.ts b/src/trackers/__tests__/uniqueKeysTracker.spec.ts
diff --git a/src/trackers/eventTracker.ts b/src/trackers/eventTracker.ts
diff --git a/src/trackers/impressionsTracker.ts b/src/trackers/impressionsTracker.ts
diff --git a/src/trackers/types.ts b/src/trackers/types.ts
diff --git a/src/trackers/uniqueKeysTracker.ts b/src/trackers/uniqueKeysTracker.ts

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "@splitsoftware/splitio-commons",`
`3`		`- "version": "1.17.1-rc.1",`
	`3`	`+ "version": "1.17.1-rc.3",`
`4`	`4`	`"description": "Split JavaScript SDK common components",`
`5`	`5`	`"main": "cjs/index.js",`
`6`	`6`	`"module": "esm/index.js",`