fix: init Destination with config

amplitude · Aug 26, 2023 · 51a5ff1 · 51a5ff1
1 parent 8f26ef8
commit 51a5ff1
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 18 deletions.
diff --git 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
@@ -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
@@ -1,20 +1,19 @@
 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
   // eslint-disable-next-line @typescript-eslint/ban-ts-comment
   // @ts-ignore
   config: BrowserConfig;
 
-  async setup(config: BrowserConfig): Promise<undefined> {
+  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
@@ -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);
     });
   });
 });