Merge pull request #478 from splitio/refactor-init

Refactor internal `init` and `destroy` methods

Author: EmilianoSanchez

.gitignore

@@ -12,3 +12,11 @@
 
 ## coverage info
 /coverage
+
+## worktrees
+/.worktrees
+
+## agents files
+/AGENTS.md
+/CLAUDE.md
+/.claude

package-lock.json

@@ -60,10 +60,10 @@
     "@types/ioredis": "^4.28.0",
     "@types/jest": "^27.0.0",
     "@types/lodash": "^4.14.162",
-    "@typescript-eslint/eslint-plugin": "^6.6.0",
-    "@typescript-eslint/parser": "^6.6.0",
+    "@typescript-eslint/eslint-plugin": "^7.18.0",
+    "@typescript-eslint/parser": "^7.18.0",
     "cross-env": "^7.0.2",
-    "eslint": "^8.48.0",
+    "eslint": "^8.56.0",
     "eslint-plugin-compat": "^6.0.1",
     "eslint-plugin-import": "^2.25.3",
     "eslint-plugin-tsdoc": "^0.3.0",
@@ -76,7 +76,7 @@
     "redis-server": "1.2.2",
     "rimraf": "^3.0.2",
     "ts-jest": "^27.0.5",
-    "typescript": "4.4.4"
+    "typescript": "4.7.4"
   },
   "sideEffects": false
 }

package.json

@@ -21,7 +21,7 @@ export function pluggableIntegrationsManagerFactory(
   // No need to check if `settings.integrations` is an array of functions. It was already validated
   integrations.forEach(integrationFactory => {
     let integration = integrationFactory(params);
-    if (integration && integration.queue) listeners.push(integration);
+    if (integration) listeners.push(integration);
   });
 
   // If `listeners` is empty, not return a integration manager

src/integrations/pluggable.ts

@@ -1,18 +1,19 @@
 import { objectAssign } from '../utils/lang/objectAssign';
 import SplitIO from '../../types/splitio';
-import { releaseApiKey } from '../utils/inputValidation/apiKey';
+import { releaseApiKey, validateAndTrackApiKey } from '../utils/inputValidation/apiKey';
 import { clientFactory } from './client';
 import { clientInputValidationDecorator } from './clientInputValidation';
 import { ISdkFactoryContext } from '../sdkFactory/types';
 
 const COOLDOWN_TIME_IN_MILLIS = 1000;
 
 /**
- * Creates an Sdk client, i.e., a base client with status and destroy interface
+ * Creates an Sdk client, i.e., a base client with status, init, flush and destroy interface
  */
 export function sdkClientFactory(params: ISdkFactoryContext, isSharedClient?: boolean): SplitIO.IClient | SplitIO.IAsyncClient {
   const { sdkReadinessManager, syncManager, storage, signalListener, settings, telemetryTracker, uniqueKeysTracker } = params;
 
+  let hasInit = false;
   let lastActionTime = 0;
 
   function __cooldown(func: Function, time: number) {
@@ -47,13 +48,27 @@ export function sdkClientFactory(params: ISdkFactoryContext, isSharedClient?: bo
       params.fallbackTreatmentsCalculator
     ),
 
-    // Sdk destroy
     {
+      init() {
+        if (hasInit) return;
+        hasInit = true;
+
+        if (!isSharedClient) {
+          validateAndTrackApiKey(settings.log, settings.core.authorizationKey);
+          sdkReadinessManager.readinessManager.init();
+          uniqueKeysTracker.start();
+          syncManager && syncManager.start();
+          signalListener && signalListener.start();
+        }
+      },
+
       flush() {
         // @TODO define cooldown time
         return __cooldown(__flush, COOLDOWN_TIME_IN_MILLIS);
       },
+
       destroy() {
+        hasInit = false;
         // Mark the SDK as destroyed immediately
         sdkReadinessManager.readinessManager.destroy();
 

src/sdkClient/sdkClient.ts

@@ -7,7 +7,7 @@ import { FallbackTreatmentsCalculator } from '../../evaluator/fallbackTreatments
 
 /** Mocks */
 
-const clientInstance = { destroy: jest.fn() };
+const clientInstance = { init: jest.fn(), destroy: jest.fn() };
 const managerInstance = 'manager';
 const mockStorage = {
   splits: jest.fn(),
@@ -40,13 +40,10 @@ const paramsForAsyncSDK = {
   fallbackTreatmentsCalculator: new FallbackTreatmentsCalculator()
 };
 
-const SignalListenerInstanceMock = { start: jest.fn() };
-
 // IBrowserSDK, full params
 const fullParamsForSyncSDK = {
   ...paramsForAsyncSDK,
   syncManagerFactory: jest.fn(),
-  SignalListener: jest.fn(() => SignalListenerInstanceMock),
   impressionsObserverFactory: jest.fn(),
   platform: {
     getEventSource: jest.fn(),
@@ -79,10 +76,6 @@ function assertModulesCalled(params: any) {
   if (params.impressionsObserverFactory) {
     expect(params.impressionsObserverFactory).toBeCalledTimes(1);
   }
-  if (params.SignalListener) {
-    expect(params.SignalListener).toBeCalledTimes(1);
-    expect(SignalListenerInstanceMock.start).toBeCalledTimes(1);
-  }
   if (params.splitApiFactory) {
     expect(params.splitApiFactory.mock.calls).toEqual([[params.settings, params.platform, telemetryTrackerMock]]);
   }

src/sdkFactory/__tests__/index.spec.ts

@@ -4,7 +4,6 @@ import { impressionsTrackerFactory } from '../trackers/impressionsTracker';
 import { eventTrackerFactory } from '../trackers/eventTracker';
 import { telemetryTrackerFactory } from '../trackers/telemetryTracker';
 import SplitIO from '../../types/splitio';
-import { validateAndTrackApiKey } from '../utils/inputValidation/apiKey';
 import { createLoggerAPI } from '../logger/sdkLogger';
 import { NEW_FACTORY, RETRIEVE_MANAGER } from '../logger/constants';
 import { SDK_SPLITS_ARRIVED, SDK_SEGMENTS_ARRIVED, SDK_SPLITS_CACHE_LOADED } from '../readiness/constants';
@@ -33,15 +32,6 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ISDK | SplitIO.IA
   // @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.
 
-  // 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;
 
@@ -68,7 +58,7 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ISDK | SplitIO.IA
     if ((storage as IStorageSync).splits.getChangeNumber() > -1) readiness.splits.emit(SDK_SPLITS_CACHE_LOADED, { initialCacheLoad: false /* Not an initial load, cache exists */ });
   }
 
-  const clients: Record<string, SplitIO.IBasicClient> = {};
+  const clients: Record<string, SplitIO.IBasicClient & { init: () => void }> = {};
   const telemetryTracker = telemetryTrackerFactory(storage.telemetry, platform.now);
   const integrationsManager = integrationsManagerFactory && integrationsManagerFactory({ settings, storage, telemetryTracker });
 
@@ -82,8 +72,8 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ISDK | SplitIO.IA
       strategyDebugFactory(observer) :
       noneStrategy;
 
-  const impressionsTracker = impressionsTrackerFactory(settings, storage.impressions, noneStrategy, strategy, whenInit, integrationsManager, storage.telemetry);
-  const eventTracker = eventTrackerFactory(settings, storage.events, whenInit, integrationsManager, storage.telemetry);
+  const impressionsTracker = impressionsTrackerFactory(settings, storage.impressions, noneStrategy, strategy, integrationsManager, storage.telemetry);
+  const eventTracker = eventTrackerFactory(settings, storage.events, integrationsManager, storage.telemetry);
 
   // splitApi is used by SyncManager and Browser signal listener
   const splitApi = splitApiFactory && splitApiFactory(settings, platform, telemetryTracker);
@@ -93,6 +83,7 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ISDK | SplitIO.IA
   const syncManager = syncManagerFactory && syncManagerFactory(ctx as ISdkFactoryContextSync);
   ctx.syncManager = syncManager;
 
+  // @TODO: move into platform, and call inside sdkClientFactory (if it's used only there)
   const signalListener = SignalListener && new SignalListener(syncManager, settings, storage, splitApi);
   ctx.signalListener = signalListener;
 
@@ -102,18 +93,7 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ISDK | SplitIO.IA
 
 
   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.start();
-    syncManager && syncManager.start();
-    signalListener && signalListener.start();
-
-    initCallbacks.forEach((cb) => cb());
-    initCallbacks.length = 0;
+    Object.keys(clients).map(key => clients[key].init());
   }
 
   log.info(NEW_FACTORY, [settings.version]);
@@ -135,7 +115,6 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ISDK | SplitIO.IA
     settings,
 
     destroy() {
-      hasInit = false;
       return Promise.all(Object.keys(clients).map(key => clients[key].destroy())).then(() => { });
     }
   }, extraProps && extraProps(ctx), lazyInit ? { init } : init());

src/sdkFactory/index.ts

@@ -98,7 +98,11 @@ export interface ISdkFactoryParams {
 
   // Sdk client method factory.
   // It Allows to distinguish SDK clients with the client-side API (`IBrowserSDK` and `IBrowserAsyncSDK`) or server-side API (`ISDK` and `IAsyncSDK`).
-  sdkClientMethodFactory: (params: ISdkFactoryContext) => ({ (): SplitIO.IBrowserClient; (key: SplitIO.SplitKey): SplitIO.IBrowserClient; } | (() => SplitIO.IClient) | (() => SplitIO.IAsyncClient))
+  sdkClientMethodFactory: (params: ISdkFactoryContext) => (
+    { (): SplitIO.IBrowserClient & { init(): void }; (key: SplitIO.SplitKey): SplitIO.IBrowserClient & { init(): void }; } |
+    (() => SplitIO.IClient & { init(): void }) |
+    (() => SplitIO.IAsyncClient & { init(): void })
+  )
 
   // Impression observer factory.
   impressionsObserverFactory: () => IImpressionObserver

src/sdkFactory/types.ts

@@ -29,15 +29,13 @@ 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, fakeWhenInit, fakeIntegrationsManager);
+    const instance = eventTrackerFactory(fullSettings, fakeEventsCache, fakeIntegrationsManager);
 
     expect(typeof instance.track).toBe('function'); // The instance should implement the track method.
   });
@@ -53,7 +51,7 @@ describe('Event Tracker', () => {
       }
     });
     // @ts-ignore
-    const tracker = eventTrackerFactory(fullSettings, fakeEventsCache, fakeWhenInit, fakeIntegrationsManager, fakeTelemetryCache);
+    const tracker = eventTrackerFactory(fullSettings, fakeEventsCache, fakeIntegrationsManager, fakeTelemetryCache);
     const result1 = tracker.track(fakeEvent, 1);
 
     expect(fakeEventsCache.track.mock.calls[0]).toEqual([fakeEvent, 1]); // Should be present in the event cache.
@@ -94,7 +92,7 @@ describe('Event Tracker', () => {
     const settings = { ...fullSettings };
     const fakeEventsCache = { track: jest.fn(() => true) };
 
-    const tracker = eventTrackerFactory(settings, fakeEventsCache, fakeWhenInit);
+    const tracker = eventTrackerFactory(settings, fakeEventsCache);
 
     expect(tracker.track(fakeEvent)).toBe(true);
     expect(fakeEventsCache.track).toBeCalledTimes(1); // event should be tracked if userConsent is undefined

src/trackers/__tests__/eventTracker.spec.ts

@@ -34,8 +34,6 @@ const fakeSettingsWithListener = {
   ...fakeSettings,
   impressionListener: fakeListener
 };
-const fakeWhenInit = (cb: () => void) => cb();
-
 const fakeNoneStrategy = {
   process: jest.fn(() => false)
 };
@@ -53,7 +51,7 @@ describe('Impressions Tracker', () => {
   const strategy = strategyDebugFactory(impressionObserverCSFactory());
 
   test('Should be able to track impressions (in DEBUG mode without Previous Time).', () => {
-    const tracker = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategy, fakeWhenInit);
+    const tracker = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategy);
 
     const imp1 = {
       feature: '10',
@@ -73,7 +71,7 @@ describe('Impressions Tracker', () => {
   });
 
   test('Tracked impressions should be sent to impression listener and integration manager when we invoke .track()', (done) => {
-    const tracker = impressionsTrackerFactory(fakeSettingsWithListener, fakeImpressionsCache, fakeNoneStrategy, strategy, fakeWhenInit, fakeIntegrationsManager);
+    const tracker = impressionsTrackerFactory(fakeSettingsWithListener, fakeImpressionsCache, fakeNoneStrategy, strategy, fakeIntegrationsManager);
 
     const fakeImpression = {
       feature: 'impression'
@@ -147,8 +145,8 @@ describe('Impressions Tracker', () => {
     impression3.time = 1234567891;
 
     const trackers = [
-      impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategyDebugFactory(impressionObserverSSFactory()), fakeWhenInit, undefined),
-      impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategyDebugFactory(impressionObserverCSFactory()), fakeWhenInit, undefined)
+      impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategyDebugFactory(impressionObserverSSFactory()), undefined),
+      impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategyDebugFactory(impressionObserverCSFactory()), undefined)
     ];
 
     expect(fakeImpressionsCache.track).not.toBeCalled(); // storage method should not be called until impressions are tracked.
@@ -175,7 +173,7 @@ describe('Impressions Tracker', () => {
     impression3.time = Date.now();
 
     const impressionCountsCache = new ImpressionCountsCacheInMemory();
-    const tracker = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategyOptimizedFactory(impressionObserverCSFactory(), impressionCountsCache), fakeWhenInit, undefined, fakeTelemetryCache as any);
+    const tracker = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategyOptimizedFactory(impressionObserverCSFactory(), impressionCountsCache), undefined, fakeTelemetryCache as any);
 
     expect(fakeImpressionsCache.track).not.toBeCalled(); // cache method should not be called by just creating a tracker
 
@@ -198,7 +196,7 @@ describe('Impressions Tracker', () => {
   test('Should track or not impressions depending on user consent status', () => {
     const settings = { ...fullSettings };
 
-    const tracker = impressionsTrackerFactory(settings, fakeImpressionsCache, fakeNoneStrategy, strategy, fakeWhenInit);
+    const tracker = impressionsTrackerFactory(settings, fakeImpressionsCache, fakeNoneStrategy, strategy);
 
     tracker.track([{ imp: impression }]);
     expect(fakeImpressionsCache.track).toBeCalledTimes(1); // impression should be tracked if userConsent is undefined