From 51a5ff118bda35c9dd4846724573aa64838b01fe Mon Sep 17 00:00:00 2001 From: Xinyi Ye Date: Fri, 25 Aug 2023 17:02:27 -0700 Subject: [PATCH] fix: init Destination with config --- .../analytics-browser/src/browser-client.ts | 5 +++-- packages/analytics-browser/src/config.ts | 2 +- .../src/plugins/destination.ts | 9 ++++---- .../test/plugins/destination.test.ts | 21 ++++++++++--------- 4 files changed, 19 insertions(+), 18 deletions(-) diff --git a/packages/analytics-browser/src/browser-client.ts b/packages/analytics-browser/src/browser-client.ts index 8c7c2b3c0..3bf0aed68 100644 --- a/packages/analytics-browser/src/browser-client.ts +++ b/packages/analytics-browser/src/browser-client.ts @@ -1,4 +1,4 @@ -import { AmplitudeCore, Destination, Identify, returnWrapper, Revenue, UUID } from '@amplitude/analytics-core'; +import { AmplitudeCore, Identify, returnWrapper, Revenue, UUID } from '@amplitude/analytics-core'; import { getAnalyticsConnector, getAttributionTrackingConfig, @@ -24,6 +24,7 @@ import { } from '@amplitude/analytics-types'; import { convertProxyObjectToRealObject, isInstanceProxy } from './utils/snippet-helper'; import { Context } from './plugins/context'; +import { Destination } from './plugins/destination'; import { useBrowserConfig, createTransport } from './config'; import { webAttributionPlugin } from '@amplitude/plugin-web-attribution-browser'; import { pageViewTrackingPlugin } from '@amplitude/plugin-page-view-tracking-browser'; @@ -86,7 +87,7 @@ export class AmplitudeBrowser extends AmplitudeCore implements BrowserClient { // Step 4: Install plugins // Do not track any events before this - await this.add(new Destination()).promise; + await this.add(new Destination(this.config)).promise; await this.add(new Context()).promise; await this.add(new IdentityEventSender()).promise; diff --git a/packages/analytics-browser/src/config.ts b/packages/analytics-browser/src/config.ts index e4dfcde7c..920df08e2 100644 --- a/packages/analytics-browser/src/config.ts +++ b/packages/analytics-browser/src/config.ts @@ -77,7 +77,7 @@ export class BrowserConfig extends Config implements IBrowserConfig { public transport: 'fetch' | 'xhr' | 'beacon' = 'fetch', public useBatch: boolean = false, userId?: string, - public diagnosticProvider: DiagnosticPlugin = new Diagnostic(), + public diagnosticProvider: DiagnosticPlugin | undefined = new Diagnostic(), ) { super({ apiKey, storageProvider, transportProvider: createTransport(transport) }); this._cookieStorage = cookieStorage; diff --git a/packages/analytics-browser/src/plugins/destination.ts b/packages/analytics-browser/src/plugins/destination.ts index e0e811083..e789fa6f8 100644 --- a/packages/analytics-browser/src/plugins/destination.ts +++ b/packages/analytics-browser/src/plugins/destination.ts @@ -1,6 +1,5 @@ import { Destination as CoreDestination, buildResult } from '@amplitude/analytics-core'; -import { BrowserConfig } from '../../src/config'; -import { DestinationContext as Context } from '@amplitude/analytics-types'; +import { BrowserConfig, DestinationContext as Context } from '@amplitude/analytics-types'; export class Destination extends CoreDestination { // this.config is defined in setup() which will always be called first @@ -8,13 +7,13 @@ export class Destination extends CoreDestination { // @ts-ignore config: BrowserConfig; - async setup(config: BrowserConfig): Promise { + constructor(config: BrowserConfig) { + super(); this.config = config; - return super.setup(config); } async fulfillRequest(list: Context[], code: number, message: string) { - await this.config.diagnosticProvider.track(list.length, code, message); + await this.config.diagnosticProvider?.track(list.length, code, message); this.saveEvents(); list.forEach((context) => context.callback(buildResult(context.event, code, message))); } diff --git a/packages/analytics-browser/test/plugins/destination.test.ts b/packages/analytics-browser/test/plugins/destination.test.ts index 9fc5f7ea6..13ee6df74 100644 --- a/packages/analytics-browser/test/plugins/destination.test.ts +++ b/packages/analytics-browser/test/plugins/destination.test.ts @@ -7,27 +7,28 @@ describe('destination', () => { const apiKey = core.UUID(); const someDiagnosticProvider: core.Diagnostic = expect.any(core.Diagnostic) as core.Diagnostic; - describe('setup', () => { - test('should setup plugin', async () => { - const destination = new Destination(); + describe('constructor', () => { + test('should init plugin with config', async () => { const config = (await useBrowserConfig(apiKey, undefined, new AmplitudeBrowser())) as BrowserConfig; - await destination.setup(config); + const destination = new Destination(config); expect(destination.config.diagnosticProvider).toEqual(someDiagnosticProvider); }); }); describe('fulfillRequest', () => { - test('should track diagnostics', async () => { - const destination = new Destination(); + test.each([ + [true, 1], + [false, 0], + ])('should track diagnostics by default', async (isDiagnosticEnabled, diagnosticTrackTimes) => { + const config = (await useBrowserConfig(apiKey, undefined, new AmplitudeBrowser())) as BrowserConfig; const mockDiagnosticProvider = { type: 'destination' as const, execute: jest.fn(), flush: jest.fn(), track: jest.fn(), }; - const config = (await useBrowserConfig(apiKey, undefined, new AmplitudeBrowser())) as BrowserConfig; - config.diagnosticProvider = mockDiagnosticProvider; - destination.config = config; + config.diagnosticProvider = isDiagnosticEnabled ? mockDiagnosticProvider : undefined; + const destination = new Destination(config); const callback = jest.fn(); const context = { attempts: 0, @@ -40,7 +41,7 @@ describe('destination', () => { await destination.fulfillRequest([context], 200, 'success'); expect(callback).toHaveBeenCalledTimes(1); expect(callback).toHaveBeenCalledWith(core.buildResult(context.event, 200, 'success')); - expect(mockDiagnosticProvider.track).toHaveBeenCalledTimes(1); + expect(mockDiagnosticProvider.track).toHaveBeenCalledTimes(diagnosticTrackTimes); }); }); });