ref(baseclient): Simplify the setup logic of integrations (#4952)

packages/core/src/baseclient.ts

@@ -75,9 +75,12 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
   /** The client Dsn, if specified in options. Without this Dsn, the SDK will be disabled. */
   protected readonly _dsn?: DsnComponents;
 
-  /** Array of used integrations. */
+  /** Array of set up integrations. */
   protected _integrations: IntegrationIndex = {};
 
+  /** Indicates whether this client's integrations have been set up. */
+  protected _integrationsInitialized: boolean = false;
+
   /** Number of calls being processed */
   protected _numProcessing: number = 0;
 
@@ -251,8 +254,9 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
    * Sets up the integrations
    */
   public setupIntegrations(): void {
-    if (this._isEnabled() && !this._integrations.initialized) {
-      this._integrations = setupIntegrations(this._options);
+    if (this._isEnabled() && !this._integrationsInitialized) {
+      this._integrations = setupIntegrations(this._options.integrations);
+      this._integrationsInitialized = true;
     }
   }
 

packages/core/src/integration.ts

@@ -1,6 +1,6 @@
 import { addGlobalEventProcessor, getCurrentHub } from '@sentry/hub';
-import { ClientOptions, Integration, Options } from '@sentry/types';
-import { addNonEnumerableProperty, logger } from '@sentry/utils';
+import { Integration, Options } from '@sentry/types';
+import { logger } from '@sentry/utils';
 
 import { IS_DEBUG_BUILD } from './flags';
 
@@ -9,7 +9,7 @@ export const installedIntegrations: string[] = [];
 /** Map of integrations assigned to a client */
 export type IntegrationIndex = {
   [key: string]: Integration;
-} & { initialized?: boolean };
+};
 
 /**
  * @private
@@ -54,31 +54,24 @@ export function getIntegrationsToSetup(options: Options): Integration[] {
   return integrations;
 }
 
-/** Setup given integration */
-export function setupIntegration(integration: Integration): void {
-  if (installedIntegrations.indexOf(integration.name) !== -1) {
-    return;
-  }
-  integration.setupOnce(addGlobalEventProcessor, getCurrentHub);
-  installedIntegrations.push(integration.name);
-  IS_DEBUG_BUILD && logger.log(`Integration installed: ${integration.name}`);
-}
-
 /**
  * Given a list of integration instances this installs them all. When `withDefaults` is set to `true` then all default
  * integrations are added unless they were already provided before.
  * @param integrations array of integration instances
  * @param withDefault should enable default integrations
  */
-export function setupIntegrations<O extends ClientOptions>(options: O): IntegrationIndex {
-  const integrations: IntegrationIndex = {};
-  options.integrations.forEach(integration => {
-    integrations[integration.name] = integration;
-    setupIntegration(integration);
+export function setupIntegrations(integrations: Integration[]): IntegrationIndex {
+  const integrationIndex: IntegrationIndex = {};
+
+  integrations.forEach(integration => {
+    integrationIndex[integration.name] = integration;
+
+    if (installedIntegrations.indexOf(integration.name) === -1) {
+      integration.setupOnce(addGlobalEventProcessor, getCurrentHub);
+      installedIntegrations.push(integration.name);
+      IS_DEBUG_BUILD && logger.log(`Integration installed: ${integration.name}`);
+    }
   });
-  // set the `initialized` flag so we don't run through the process again unecessarily; use `Object.defineProperty`
-  // because by default it creates a property which is nonenumerable, which we want since `initialized` shouldn't be
-  // considered a member of the index the way the actual integrations are
-  addNonEnumerableProperty(integrations, 'initialized', true);
-  return integrations;
+
+  return integrationIndex;
 }