Refactor inspector logic to use emitter (#718)

Author: silesky

.changeset/fair-beans-pay.md

@@ -0,0 +1,5 @@
+---
+'@segment/analytics-next': patch
+---
+
+Refactor inspector to use emitter

.changeset/forty-fireants-smile.md

@@ -0,0 +1,5 @@
+---
+'@segment/analytics-core': patch
+---
+
+Change emitter message to dispatch_start

packages/browser/src/browser/__tests__/analytics-pre-init.integration.test.ts

@@ -27,6 +27,9 @@ describe('Pre-initialization', () => {
   const trackSpy = jest.spyOn(Analytics.prototype, 'track')
   const identifySpy = jest.spyOn(Analytics.prototype, 'identify')
   const onSpy = jest.spyOn(Analytics.prototype, 'on')
+  const getOnSpyCalls = (event: string) =>
+    onSpy.mock.calls.filter(([arg1]) => arg1 === event)
+
   const readySpy = jest.spyOn(Analytics.prototype, 'ready')
   const browserLoadSpy = jest.spyOn(AnalyticsBrowser, 'load')
   const consoleErrorSpy = jest.spyOn(console, 'error')
@@ -236,11 +239,9 @@ describe('Pre-initialization', () => {
       expect(trackSpy).toBeCalledWith('bar')
       expect(trackSpy).toBeCalledTimes(2)
 
-      expect(identifySpy).toBeCalledWith()
       expect(identifySpy).toBeCalledTimes(1)
 
-      expect(onSpy).toBeCalledTimes(1)
-
+      expect(getOnSpyCalls('track').length).toBe(1)
       expect(onTrackCb).toBeCalledTimes(2) // gets called once for each track event
       expect(onTrackCb).toBeCalledWith('foo', {}, undefined)
       expect(onTrackCb).toBeCalledWith('bar', {}, undefined)
@@ -269,9 +270,10 @@ describe('Pre-initialization', () => {
       expect(identifySpy).toBeCalledWith()
       expect(identifySpy).toBeCalledTimes(1)
       expect(consoleErrorSpy).toBeCalledTimes(1)
+
       expect(consoleErrorSpy).toBeCalledWith('identity rejection')
 
-      expect(onSpy).toBeCalledTimes(1)
+      expect(getOnSpyCalls('track').length).toBe(1)
 
       expect(onTrackCb).toBeCalledTimes(2) // gets called once for each track event
       expect(onTrackCb).toBeCalledWith('foo', {}, undefined)
@@ -288,7 +290,7 @@ describe('Pre-initialization', () => {
 
       await ajsBrowser
       expect(onSpy).toBeCalledWith(...args)
-      expect(onSpy).toHaveBeenCalledTimes(1)
+      expect(getOnSpyCalls('track').length).toBe(1)
     })
 
     test('If, before initialization .on("track") is called and then .track is called, the callback method should be called after analytics loads', async () => {
@@ -302,10 +304,10 @@ describe('Pre-initialization', () => {
       await Promise.all([analytics, trackCtxPromise])
 
       expect(onSpy).toBeCalledWith('track', onFnCb)
-      expect(onSpy).toHaveBeenCalledTimes(1)
+      expect(getOnSpyCalls('track').length).toBe(1)
 
       expect(onFnCb).toHaveBeenCalledWith('foo', { name: 123 }, undefined)
-      expect(onFnCb).toHaveBeenCalledTimes(1)
+      expect(onFnCb).toBeCalledTimes(1)
     })
 
     test('If, before initialization, .ready is called, the callback method should be called after analytics loads', async () => {

packages/browser/src/browser/__tests__/inspector.integration.test.ts

@@ -0,0 +1,44 @@
+import { createSuccess } from '../../test-helpers/factories'
+import { AnalyticsBrowser } from '../..'
+
+import unfetch from 'unfetch'
+jest.mock('unfetch')
+jest
+  .mocked(unfetch)
+  .mockImplementation(() => createSuccess({ integrations: {} }))
+
+const writeKey = 'foo'
+
+describe('Inspector', () => {
+  const triggeredSpy = jest.fn()
+  const attachedSpy = jest.fn()
+  const deliveredSpy = jest.fn()
+  beforeEach(() => {
+    Object.assign((window.__SEGMENT_INSPECTOR__ ??= {}), {
+      triggered: triggeredSpy,
+      attach: attachedSpy,
+      delivered: deliveredSpy,
+    })
+  })
+  it('attaches to inspector', async () => {
+    await AnalyticsBrowser.load({
+      writeKey,
+    })
+    expect(attachedSpy).toBeCalledTimes(1)
+  })
+
+  it('calls triggered and delivered when an event is sent', async () => {
+    const [analytics] = await AnalyticsBrowser.load({
+      writeKey,
+    })
+    expect(attachedSpy).toBeCalledTimes(1)
+    expect(triggeredSpy).toBeCalledTimes(0)
+    expect(deliveredSpy).toBeCalledTimes(0)
+
+    await analytics.track('foo', {})
+
+    expect(triggeredSpy.mock.lastCall[0].event.type).toBe('track')
+    expect(triggeredSpy).toBeCalledTimes(1)
+    expect(deliveredSpy).toBeCalledTimes(1)
+  })
+})

packages/browser/src/browser/__tests__/standalone-analytics.test.ts

@@ -7,6 +7,7 @@ import unfetch from 'unfetch'
 import { PersistedPriorityQueue } from '../../lib/priority-queue/persisted'
 import { sleep } from '../../lib/sleep'
 import * as Factory from '../../test-helpers/factories'
+import { EventQueue } from '@segment/analytics-core'
 
 const track = jest.fn()
 const identify = jest.fn()
@@ -26,9 +27,7 @@ jest.mock('@/core/analytics', () => ({
     register,
     emit: jest.fn(),
     on,
-    queue: {
-      queue: new PersistedPriorityQueue(1, 'event-queue'),
-    },
+    queue: new EventQueue(new PersistedPriorityQueue(1, 'event-queue') as any),
     options,
   }),
 }))
@@ -237,8 +236,11 @@ describe('standalone bundle', () => {
 
     await sleep(0)
 
-    expect(on).toHaveBeenCalledTimes(1)
-    expect(on).toHaveBeenCalledWith('initialize', expect.any(Function))
+    const initializeCalls = on.mock.calls.filter(
+      ([arg1]) => arg1 === 'initialize'
+    )
+
+    expect(initializeCalls.length).toBe(1)
 
     expect(operations).toEqual([
       // should run before any plugin is registered

packages/browser/src/browser/index.ts

@@ -23,8 +23,8 @@ import {
   flushOn,
 } from '../core/buffer'
 import { popSnippetWindowBuffer } from '../core/buffer/snippet'
-import { inspectorHost } from '../core/inspector'
 import { ClassicIntegrationSource } from '../plugins/ajs-destination/types'
+import { attachInspector } from '../core/inspector'
 
 export interface LegacyIntegrationConfiguration {
   /* @deprecated - This does not indicate browser types anymore */
@@ -275,7 +275,7 @@ async function loadAnalytics(
   const opts: InitOptions = { retryQueue, ...options }
   const analytics = new Analytics(settings, opts)
 
-  inspectorHost.attach?.(analytics as any)
+  attachInspector(analytics)
 
   const plugins = settings.plugins ?? []
   const classicIntegrations = settings.classicIntegrations ?? []

packages/browser/src/core/analytics/index.ts

@@ -47,7 +47,6 @@ import type {
 import { version } from '../../generated/version'
 import { PriorityQueue } from '../../lib/priority-queue'
 import { getGlobal } from '../../lib/get-global'
-import { inspectorHost } from '../inspector'
 import { AnalyticsClassic, AnalyticsCore } from './interfaces'
 
 const deprecationWarning =
@@ -372,7 +371,7 @@ export class Analytics
   ): Promise<DispatchedEvent> {
     const ctx = new Context(event)
 
-    inspectorHost.triggered?.(ctx as any)
+    this.emit('dispatch_start', ctx)
 
     if (isOffline() && !this.options.retryQueue) {
       return ctx

packages/browser/src/core/inspector/__tests__/index.test.ts

@@ -1,4 +1,5 @@
-import { Analytics } from '../../analytics'
+import { attachInspector } from '..'
+import { Analytics } from '../../..'
 
 let analytics: Analytics
 
@@ -13,6 +14,7 @@ describe('Inspector interface', () => {
     analytics = new Analytics({
       writeKey: 'abc',
     })
+    attachInspector(analytics)
   })
 
   it('notifies the connected inspector client about each event API call and delivery', async () => {

packages/browser/src/core/inspector/index.ts

@@ -1,5 +1,6 @@
 import type { InspectBroker } from '@segment/inspector-webext'
 import { getGlobal } from '../../lib/get-global'
+import type { Analytics } from '../analytics'
 
 declare global {
   interface Window {
@@ -12,6 +13,19 @@ const env = getGlobal()
 // The code below assumes the inspector extension will use Object.assign
 // to add the inspect interface on to this object reference (unless the
 // extension code ran first and has already set up the variable)
-export const inspectorHost: Partial<InspectBroker> = ((env as any)[
+const inspectorHost: Partial<InspectBroker> = ((env as any)[
   '__SEGMENT_INSPECTOR__'
 ] ??= {})
+
+export const attachInspector = (analytics: Analytics) => {
+  inspectorHost.attach?.(analytics as any)
+
+  analytics.on('dispatch_start', (ctx) => inspectorHost.triggered?.(ctx))
+
+  analytics.queue.on('message_enriched', (ctx) => inspectorHost.enriched?.(ctx))
+
+  analytics.queue.on('message_delivered', (ctx) =>
+    // FIXME: Resolve browsers destinations that the event was sent to
+    inspectorHost.delivered?.(ctx, ['segment.io'])
+  )
+}

packages/browser/src/core/queue/event-queue.ts

@@ -9,7 +9,6 @@ import { Integrations, JSONObject } from '../events'
 import { Plugin } from '../plugin'
 import { createTaskGroup, TaskGroup } from '../task/task-group'
 import { attempt, ensure } from './delivery'
-import { inspectorHost } from '../inspector'
 import { UniversalStorage } from '../user'
 
 type PluginsByType = {
@@ -287,7 +286,7 @@ export class EventQueue extends Emitter {
       }
     }
 
-    inspectorHost.enriched?.(ctx as any)
+    this.emit('message_enriched', ctx)
 
     // Enrichment and before plugins can re-arrange the deny list dynamically
     // so we need to pluck them at the end
@@ -306,8 +305,7 @@ export class EventQueue extends Emitter {
 
     ctx.stats.increment('message_delivered')
 
-    // FIXME: Resolve browsers destinations that the event was sent to
-    inspectorHost.delivered?.(ctx as any, ['segment.io'])
+    this.emit('message_delivered', ctx)
 
     const afterCalls = after.map((after) => attempt(ctx, after))
     await Promise.all(afterCalls)