fix: refactor initialization sequence to guarantee readiness before event upload (#765)

Author: oscb

packages/core/package.json

@@ -4,7 +4,8 @@
   "description": "The hassle-free way to add Segment analytics to your React-Native app.",
   "main": "lib/commonjs/index",
   "scripts": {
-    "prebuild": "node constants-generator.js",
+    "prebuild": "node constants-generator.js && eslint --fix ./src/info.ts",
+    "postversion": "yarn prebuild",
     "build": "bob build",
     "test": "jest",
     "typescript": "tsc --noEmit",

packages/core/src/__tests__/__helpers__/mockSegmentStore.ts

@@ -81,7 +81,6 @@ export class MockSegmentStore implements Storage {
       return this.data.isReady;
     }),
     onChange: (_callback: (value: boolean) => void) => {
-      // Not doing anything cause this mock store is always ready, this is just legacy from the redux persistor
       return () => {};
     },
   };

packages/core/src/__tests__/__helpers__/setupSegmentClient.ts

@@ -1,6 +1,6 @@
 import { SegmentClient } from '../../analytics';
 import { UtilityPlugin } from '../../plugin';
-import { PluginType, SegmentEvent } from '../../types';
+import { Config, PluginType, SegmentEvent } from '../../types';
 import { getMockLogger } from './mockLogger';
 import { MockSegmentStore, StoreData } from './mockSegmentStore';
 
@@ -10,7 +10,10 @@ jest
   .spyOn(Date.prototype, 'toISOString')
   .mockReturnValue('2010-01-01T00:00:00.000Z');
 
-export const createTestClient = (storeData?: Partial<StoreData>) => {
+export const createTestClient = (
+  storeData?: Partial<StoreData>,
+  config?: Partial<Config>
+) => {
   const store = new MockSegmentStore({
     isReady: true,
     ...storeData,
@@ -20,15 +23,22 @@ export const createTestClient = (storeData?: Partial<StoreData>) => {
     config: {
       writeKey: 'mock-write-key',
       autoAddSegmentDestination: false,
+      ...config,
     },
     logger: getMockLogger(),
     store: store,
   };
 
   const client = new SegmentClient(clientArgs);
-
   class ObservablePlugin extends UtilityPlugin {
     type = PluginType.after;
+
+    override execute(
+      event: SegmentEvent
+    ): SegmentEvent | Promise<SegmentEvent | undefined> | undefined {
+      super.execute(event);
+      return event;
+    }
   }
 
   const mockPlugin = new ObservablePlugin();

packages/core/src/__tests__/internal/checkInstalledVersion.test.ts

@@ -206,26 +206,14 @@ describe('internal #checkInstalledVersion', () => {
     );
   });
 
-  it('executes callback when context is updated in store', async () => {
+  it('executes callback when client is ready', async () => {
     client = new SegmentClient(clientArgs);
     const callback = jest.fn().mockImplementation(() => {
       expect(store.context.get()).toEqual(currentContext);
     });
-    client.onContextLoaded(callback);
+    client.isReady.onChange(callback);
     jest.spyOn(context, 'getContext').mockResolvedValueOnce(currentContext);
     await client.init();
     expect(callback).toHaveBeenCalled();
   });
-
-  it('executes callback immediatley if registered after context was already loaded', async () => {
-    client = new SegmentClient(clientArgs);
-    jest.spyOn(context, 'getContext').mockResolvedValueOnce(currentContext);
-    await client.init();
-    // Register callback after context is loaded
-    const callback = jest.fn().mockImplementation(() => {
-      expect(store.context.get()).toEqual(currentContext);
-    });
-    client.onContextLoaded(callback);
-    expect(callback).toHaveBeenCalled();
-  });
 });

packages/core/src/__tests__/internal/handleAppStateChange.test.ts

@@ -1,8 +1,9 @@
 import { AppState, AppStateStatus } from 'react-native';
-import { SegmentClient } from '../../analytics';
-import { EventType } from '../../types';
-import { getMockLogger } from '../__helpers__/mockLogger';
-import { MockSegmentStore } from '../__helpers__/mockSegmentStore';
+import type { SegmentClient } from '../../analytics';
+import type { UtilityPlugin } from '../../plugin';
+import { EventType, SegmentEvent } from '../../types';
+import type { MockSegmentStore } from '../__helpers__/mockSegmentStore';
+import { createTestClient } from '../__helpers__/setupSegmentClient';
 
 jest.mock('../../uuid');
 jest.mock('../../context');
@@ -13,37 +14,24 @@ jest
   .mockReturnValue('2010-01-01T00:00:00.000Z');
 
 describe('SegmentClient #handleAppStateChange', () => {
-  const store = new MockSegmentStore();
-
-  const clientArgs = {
-    config: {
-      writeKey: 'mock-write-key',
-      trackAppLifecycleEvents: true,
-    },
-    logger: getMockLogger(),
-    store: store,
-  };
-
+  let store: MockSegmentStore;
   let client: SegmentClient;
+  let appStateChangeListener: ((state: AppStateStatus) => void) | undefined;
+  let expectEvent: (event: Partial<SegmentEvent>) => void;
+  let mockPlugin: UtilityPlugin;
 
   afterEach(() => {
     jest.clearAllMocks();
-    client.cleanup();
-  });
-
-  beforeEach(() => {
     store.reset();
+    client.cleanup();
   });
 
   const setupTest = async (
-    segmentClient: SegmentClient,
     from: AppStateStatus,
-    to: AppStateStatus
+    to: AppStateStatus,
+    initialTrackAppLifecycleEvents: boolean = false,
+    trackAppLifecycleEvents: boolean = true
   ) => {
-    // @ts-ignore
-    segmentClient.appState = from;
-
-    let appStateChangeListener: ((state: AppStateStatus) => void) | undefined;
     AppState.addEventListener = jest
       .fn()
       .mockImplementation(
@@ -52,37 +40,48 @@ describe('SegmentClient #handleAppStateChange', () => {
         }
       );
 
-    await segmentClient.init();
-    const clientProcess = jest.spyOn(segmentClient, 'process');
+    const stuff = createTestClient(undefined, {
+      trackAppLifecycleEvents: initialTrackAppLifecycleEvents,
+    });
+    store = stuff.store;
+    client = stuff.client;
+    expectEvent = stuff.expectEvent;
+    mockPlugin = stuff.plugin;
+
+    // @ts-ignore
+    client.appState = from;
+
+    await client.init();
+
+    // @ts-ignore settings the track here to filter out initial events
+    client.config.trackAppLifecycleEvents = trackAppLifecycleEvents;
 
     expect(appStateChangeListener).toBeDefined();
 
     appStateChangeListener!(to);
-    return clientProcess;
+    // Since the calls to process lifecycle events are not awaitable we have to await for ticks here
+    await new Promise(process.nextTick);
+    await new Promise(process.nextTick);
+    await new Promise(process.nextTick);
   };
 
   it('does not send events when trackAppLifecycleEvents is not enabled', async () => {
-    client = new SegmentClient({
-      ...clientArgs,
-      config: {
-        writeKey: 'mock-write-key',
-        trackAppLifecycleEvents: false,
-      },
-    });
-    const processSpy = await setupTest(client, 'active', 'background');
+    await setupTest('active', 'background', false, false);
 
-    expect(processSpy).not.toHaveBeenCalled();
+    expect(mockPlugin.execute).not.toHaveBeenCalled();
 
     // @ts-ignore
     expect(client.appState).toBe('background');
   });
 
   it('sends an event when inactive => active', async () => {
-    client = new SegmentClient(clientArgs);
-    const processSpy = await setupTest(client, 'inactive', 'active');
+    await setupTest('inactive', 'active');
 
-    expect(processSpy).toHaveBeenCalledTimes(1);
-    expect(processSpy).toHaveBeenCalledWith({
+    // @ts-ignore
+    expect(client.appState).toBe('active');
+
+    expect(mockPlugin.execute).toHaveBeenCalledTimes(1);
+    expectEvent({
       event: 'Application Opened',
       properties: {
         from_background: true,
@@ -91,16 +90,16 @@ describe('SegmentClient #handleAppStateChange', () => {
       },
       type: EventType.TrackEvent,
     });
-    // @ts-ignore
-    expect(client.appState).toBe('active');
   });
 
   it('sends an event when background => active', async () => {
-    client = new SegmentClient(clientArgs);
-    const processSpy = await setupTest(client, 'background', 'active');
+    await setupTest('background', 'active');
 
-    expect(processSpy).toHaveBeenCalledTimes(1);
-    expect(processSpy).toHaveBeenCalledWith({
+    // @ts-ignore
+    expect(client.appState).toBe('active');
+
+    expect(mockPlugin.execute).toHaveBeenCalledTimes(1);
+    expectEvent({
       event: 'Application Opened',
       properties: {
         from_background: true,
@@ -109,62 +108,60 @@ describe('SegmentClient #handleAppStateChange', () => {
       },
       type: EventType.TrackEvent,
     });
-    // @ts-ignore
-    expect(client.appState).toBe('active');
   });
 
   it('sends an event when active => inactive', async () => {
-    client = new SegmentClient(clientArgs);
-    const processSpy = await setupTest(client, 'active', 'inactive');
+    await setupTest('active', 'inactive');
 
-    expect(processSpy).toHaveBeenCalledTimes(1);
-    expect(processSpy).toHaveBeenCalledWith({
+    // @ts-ignore
+    expect(client.appState).toBe('inactive');
+
+    expect(mockPlugin.execute).toHaveBeenCalledTimes(1);
+    expectEvent({
       event: 'Application Backgrounded',
       properties: {},
       type: EventType.TrackEvent,
     });
-    // @ts-ignore
-    expect(client.appState).toBe('inactive');
   });
 
   it('sends an event when active => background', async () => {
-    client = new SegmentClient(clientArgs);
-    const processSpy = await setupTest(client, 'active', 'background');
+    await setupTest('active', 'background');
 
-    expect(processSpy).toHaveBeenCalledTimes(1);
-    expect(processSpy).toHaveBeenCalledWith({
+    // @ts-ignore
+    expect(client.appState).toBe('background');
+
+    expect(mockPlugin.execute).toHaveBeenCalledTimes(1);
+    expectEvent({
       event: 'Application Backgrounded',
       properties: {},
       type: EventType.TrackEvent,
     });
-    // @ts-ignore
-    expect(client.appState).toBe('background');
   });
 
-  it('does not send an event when unknown => active', async () => {
-    client = new SegmentClient(clientArgs);
-    const processSpy = await setupTest(client, 'unknown', 'active');
+  it('sends an event when unknown => active', async () => {
+    await setupTest('unknown', 'active');
 
-    expect(processSpy).not.toHaveBeenCalled();
     // @ts-ignore
     expect(client.appState).toBe('active');
+
+    expect(mockPlugin.execute).not.toHaveBeenCalled();
   });
 
-  it('does not send an event when unknown => background', async () => {
-    client = new SegmentClient(clientArgs);
-    const processSpy = await setupTest(client, 'unknown', 'background');
+  it('sends an event when unknown => background', async () => {
+    await setupTest('unknown', 'background');
 
-    expect(processSpy).not.toHaveBeenCalled();
     // @ts-ignore
     expect(client.appState).toBe('background');
+
+    expect(mockPlugin.execute).not.toHaveBeenCalled();
   });
 
-  it('does not send an event when unknown => inactive', async () => {
-    client = new SegmentClient(clientArgs);
-    const processSpy = await setupTest(client, 'unknown', 'inactive');
+  it('sends an event when unknown => inactive', async () => {
+    await setupTest('unknown', 'inactive');
 
-    expect(processSpy).not.toHaveBeenCalled();
     // @ts-ignore
     expect(client.appState).toBe('inactive');
+
+    expect(mockPlugin.execute).not.toHaveBeenCalled();
   });
 });

packages/core/src/__tests__/methods/alias.test.ts

@@ -17,9 +17,10 @@ describe('methods #alias', () => {
     userInfo: initialUserInfo,
   });
 
-  beforeEach(() => {
+  beforeEach(async () => {
     store.reset();
     jest.clearAllMocks();
+    await client.init();
   });
 
   it('adds the alias event correctly', async () => {
@@ -33,14 +34,17 @@ describe('methods #alias', () => {
 
     expectEvent(expectedEvent);
 
-    expect(client.userInfo.get()).toEqual({
+    const info = await client.userInfo.get(true);
+    expect(info).toEqual({
       anonymousId: 'anonymousId',
       userId: 'new-user-id',
       traits: undefined,
     });
   });
 
   it('uses anonymousId in event if no userId in store', async () => {
+    await client.init();
+
     await store.userInfo.set({
       anonymousId: 'anonymousId',
       userId: undefined,

packages/core/src/__tests__/methods/identify.test.ts

@@ -14,9 +14,10 @@ describe('methods #identify', () => {
     userInfo: initialUserInfo,
   });
 
-  beforeEach(() => {
+  beforeEach(async () => {
     store.reset();
     jest.clearAllMocks();
+    await client.init();
   });
 
   it('adds the identify event correctly', async () => {