fix: Internal require, Fix integration exports (#1679)

HazAT · web-flow · commit 7d0b4ef5f84a · 2018-10-25T10:35:55.000+02:00
* fix: Do not reexport integration setup functions

* fix: Also ship src with npm package

* fix: Remove shim for domain

* fix: Tests

* meta: Changelog

* fix: Add bundler safe import

* fix: Linter
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,11 @@
 # Changelog
 
 ## Unreleased
+
+## 4.2.3
+
+- [core]: Several internal fixes regarding integration, exports and domain.
+
 ## 4.2.2
 
 - [core]: "De-deprecate" name of `Integration` interface.
diff --git a/packages/browser/.npmignore b/packages/browser/.npmignore
@@ -1,3 +1,4 @@
 *
 !/dist/**/*
+!/src/**/*
 !/build/**/*
diff --git a/packages/browser/package.json b/packages/browser/package.json
@@ -42,7 +42,6 @@
     "rollup-plugin-license": "^0.6.0",
     "rollup-plugin-node-resolve": "^3.3.0",
     "rollup-plugin-npm": "^2.0.0",
-    "rollup-plugin-shim": "^1.0.0",
     "rollup-plugin-typescript2": "^0.13.0",
     "rollup-plugin-uglify": "^3.0.0",
     "sinon": "^5.0.3",
diff --git a/packages/browser/rollup.config.js b/packages/browser/rollup.config.js
@@ -3,7 +3,6 @@ import uglify from 'rollup-plugin-uglify';
 import resolve from 'rollup-plugin-node-resolve';
 import typescript from 'rollup-plugin-typescript2';
 import license from 'rollup-plugin-license';
-import shim from 'rollup-plugin-shim';
 
 const commitHash = require('child_process')
   .execSync('git rev-parse --short HEAD', { encoding: 'utf-8' })
@@ -18,9 +17,6 @@ const bundleConfig = {
   },
   context: 'window',
   plugins: [
-    shim({
-      domain: `export default {}`,
-    }),
     typescript({
       tsconfig: 'tsconfig.build.json',
       tsconfigOverride: { compilerOptions: { declaration: false } },
diff --git a/packages/core/.npmignore b/packages/core/.npmignore
@@ -1,2 +1,3 @@
 *
 !/dist/**/*
+!/src/**/*
diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts
@@ -16,7 +16,7 @@ import { consoleSandbox, uuid4 } from '@sentry/utils/misc';
 import { truncate } from '@sentry/utils/string';
 import { BackendClass } from './basebackend';
 import { Dsn } from './dsn';
-import { IntegrationIndex, setupIntegrations } from './integrations';
+import { IntegrationIndex, setupIntegrations } from './integration';
 import { Backend, Client, Options } from './interfaces';
 
 /**
diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts
@@ -0,0 +1,98 @@
+import { Integration } from '@sentry/types';
+import { logger } from '@sentry/utils/logger';
+import { Options } from './interfaces';
+
+export const installedIntegrations: string[] = [];
+
+/** Map of integrations assigned to a client */
+export interface IntegrationIndex {
+  [key: string]: Integration;
+}
+
+/** Gets integration to install */
+export function getIntegrationsToSetup(options: Options): Integration[] {
+  const defaultIntegrations = (options.defaultIntegrations && [...options.defaultIntegrations]) || [];
+  const userIntegrations = options.integrations;
+  let integrations: Integration[] = [];
+  if (Array.isArray(userIntegrations)) {
+    const userIntegrationsNames = userIntegrations.map(i => i.name);
+    const pickedIntegrationsNames = [];
+
+    // Leave only unique default integrations, that were not overridden with provided user integrations
+    for (const defaultIntegration of defaultIntegrations) {
+      if (
+        userIntegrationsNames.indexOf(getIntegrationName(defaultIntegration)) === -1 &&
+        pickedIntegrationsNames.indexOf(getIntegrationName(defaultIntegration)) === -1
+      ) {
+        integrations.push(defaultIntegration);
+        pickedIntegrationsNames.push(getIntegrationName(defaultIntegration));
+      }
+    }
+
+    // Don't add same user integration twice
+    for (const userIntegration of userIntegrations) {
+      if (pickedIntegrationsNames.indexOf(getIntegrationName(userIntegration)) === -1) {
+        integrations.push(userIntegration);
+        pickedIntegrationsNames.push(getIntegrationName(userIntegration));
+      }
+    }
+  } else if (typeof userIntegrations === 'function') {
+    integrations = userIntegrations(defaultIntegrations);
+    integrations = Array.isArray(integrations) ? integrations : [integrations];
+  } else {
+    return [...defaultIntegrations];
+  }
+
+  return integrations;
+}
+
+/** Setup given integration */
+export function setupIntegration(integration: Integration, options: Options): void {
+  if (installedIntegrations.indexOf(getIntegrationName(integration)) !== -1) {
+    return;
+  }
+
+  try {
+    integration.setupOnce();
+  } catch (_Oo) {
+    /** @deprecated */
+    // TODO: Remove in v5
+    logger.warn(`Integration ${getIntegrationName(integration)}: The install method is deprecated. Use "setupOnce".`);
+
+    // tslint:disable:deprecation
+    if (integration.install) {
+      integration.install(options);
+    }
+    // tslint:enable:deprecation
+  }
+
+  installedIntegrations.push(getIntegrationName(integration));
+  logger.log(`Integration installed: ${getIntegrationName(integration)}`);
+}
+
+/**
+ * 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 Options>(options: O): IntegrationIndex {
+  const integrations: IntegrationIndex = {};
+  getIntegrationsToSetup(options).forEach(integration => {
+    integrations[getIntegrationName(integration)] = integration;
+    setupIntegration(integration, options);
+  });
+  return integrations;
+}
+
+/**
+ * Returns the integration static id.
+ * @param integration Integration to retrieve id
+ */
+function getIntegrationName(integration: Integration): string {
+  /**
+   * @depracted
+   */
+  // tslint:disable-next-line:no-unsafe-any
+  return (integration as any).constructor.id || integration.name;
+}
diff --git a/packages/core/src/integrations/index.ts b/packages/core/src/integrations/index.ts
@@ -1,105 +1,7 @@
-// tslint:disable:deprecation
-import { Integration } from '@sentry/types';
-import { logger } from '@sentry/utils/logger';
-import { Options } from '../interfaces';
-
 export { Dedupe } from './dedupe';
 export { FunctionToString } from './functiontostring';
 export { SDKInformation } from './sdkinformation';
 export { InboundFilters } from './inboundfilters';
 
 export { Debug } from './pluggable/debug';
 export { RewriteFrames } from './pluggable/rewriteframes';
-
-export const installedIntegrations: string[] = [];
-
-/** Map of integrations assigned to a client */
-export interface IntegrationIndex {
-  [key: string]: Integration;
-}
-
-/** Gets integration to install */
-export function getIntegrationsToSetup(options: Options): Integration[] {
-  const defaultIntegrations = (options.defaultIntegrations && [...options.defaultIntegrations]) || [];
-  const userIntegrations = options.integrations;
-  let integrations: Integration[] = [];
-  if (Array.isArray(userIntegrations)) {
-    const userIntegrationsNames = userIntegrations.map(i => i.name);
-    const pickedIntegrationsNames = [];
-
-    // Leave only unique default integrations, that were not overridden with provided user integrations
-    for (const defaultIntegration of defaultIntegrations) {
-      if (
-        userIntegrationsNames.indexOf(getIntegrationName(defaultIntegration)) === -1 &&
-        pickedIntegrationsNames.indexOf(getIntegrationName(defaultIntegration)) === -1
-      ) {
-        integrations.push(defaultIntegration);
-        pickedIntegrationsNames.push(getIntegrationName(defaultIntegration));
-      }
-    }
-
-    // Don't add same user integration twice
-    for (const userIntegration of userIntegrations) {
-      if (pickedIntegrationsNames.indexOf(getIntegrationName(userIntegration)) === -1) {
-        integrations.push(userIntegration);
-        pickedIntegrationsNames.push(getIntegrationName(userIntegration));
-      }
-    }
-  } else if (typeof userIntegrations === 'function') {
-    integrations = userIntegrations(defaultIntegrations);
-    integrations = Array.isArray(integrations) ? integrations : [integrations];
-  } else {
-    return [...defaultIntegrations];
-  }
-
-  return integrations;
-}
-
-/** Setup given integration */
-export function setupIntegration(integration: Integration, options: Options): void {
-  if (installedIntegrations.indexOf(getIntegrationName(integration)) !== -1) {
-    return;
-  }
-
-  try {
-    integration.setupOnce();
-  } catch (_Oo) {
-    /** @deprecated */
-    // TODO: Remove in v5
-    logger.warn(`Integration ${getIntegrationName(integration)}: The install method is deprecated. Use "setupOnce".`);
-
-    if (integration.install) {
-      integration.install(options);
-    }
-  }
-
-  installedIntegrations.push(getIntegrationName(integration));
-  logger.log(`Integration installed: ${getIntegrationName(integration)}`);
-}
-
-/**
- * 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 Options>(options: O): IntegrationIndex {
-  const integrations: IntegrationIndex = {};
-  getIntegrationsToSetup(options).forEach(integration => {
-    integrations[getIntegrationName(integration)] = integration;
-    setupIntegration(integration, options);
-  });
-  return integrations;
-}
-
-/**
- * Returns the integration static id.
- * @param integration Integration to retrieve id
- */
-function getIntegrationName(integration: Integration): string {
-  /**
-   * @depracted
-   */
-  // tslint:disable-next-line:no-unsafe-any
-  return (integration as any).constructor.id || integration.name;
-}
diff --git a/packages/core/test/lib/integration.test.ts b/packages/core/test/lib/integration.test.ts
@@ -1,6 +1,6 @@
 // tslint:disable:deprecation
 import { Integration } from '@sentry/types';
-import { getIntegrationsToSetup } from '../../../src/integrations';
+import { getIntegrationsToSetup } from '../../src/integration';
 
 /** JSDoc */
 class MockIntegration implements Integration {
diff --git a/packages/core/test/lib/sdk.test.ts b/packages/core/test/lib/sdk.test.ts
@@ -1,5 +1,5 @@
 import { Integration } from '@sentry/types';
-import { installedIntegrations } from '../../src/integrations';
+import { installedIntegrations } from '../../src/integration';
 import { initAndBind } from '../../src/sdk';
 import { TestClient } from '../mocks/client';
 
diff --git a/packages/hub/.npmignore b/packages/hub/.npmignore
@@ -1,2 +1,3 @@
 *
 !/dist/**/*
+!/src/**/*
diff --git a/packages/hub/src/hub.ts b/packages/hub/src/hub.ts
@@ -8,7 +8,7 @@ import {
   Severity,
 } from '@sentry/types';
 import { logger } from '@sentry/utils/logger';
-import { getGlobalObject, uuid4 } from '@sentry/utils/misc';
+import { bundlerSafeRequire, getGlobalObject, uuid4 } from '@sentry/utils/misc';
 import { Carrier, Layer } from './interfaces';
 import { Scope } from './scope';
 
@@ -334,7 +334,10 @@ export function getCurrentHub(): Hub {
 
   // Prefer domains over global if they are there
   try {
-    const domain = require('domain');
+    // We need to use `bundlerSafeRequire` because `require` on it's own will be optimized by webpack.
+    // We do not want this to happen, we need to try `require` the domain node module and fail if we are in browser
+    // for example so we do not have to shim it and use `getCurrentHub` universally.
+    const domain = bundlerSafeRequire('domain');
     const activeDomain = domain.active;
 
     // If there no active domain, just return global hub
diff --git a/packages/minimal/.npmignore b/packages/minimal/.npmignore
@@ -1,2 +1,3 @@
 *
 !/dist/**/*
+!/src/**/*
diff --git a/packages/node/.npmignore b/packages/node/.npmignore
@@ -1,2 +1,3 @@
 *
 !/dist/**/*
+!/src/**/*
diff --git a/packages/types/.npmignore b/packages/types/.npmignore
@@ -1,2 +1,3 @@
 *
 !/dist/**/*
+!/src/**/*
diff --git a/packages/utils/.npmignore b/packages/utils/.npmignore
@@ -3,3 +3,4 @@
 !*.d.ts
 !*.js
 jest.config.js
+!/src/**/*
diff --git a/packages/utils/src/misc.ts b/packages/utils/src/misc.ts
@@ -1,6 +1,18 @@
 import { SentryEvent, SentryWrappedFunction } from '@sentry/types';
 import { isString } from './is';
 
+/**
+ * Requires a module which is protected against bundler minification.
+ *
+ * @param request The module path to resolve
+ */
+export function bundlerSafeRequire(request: string): any {
+  // We need to do this check for global.module || module here because if we are calling this inside a
+  // active domain, global.module is undefined.
+  // tslint:disable-next-line
+  return ((getGlobalObject() as any).module || module).require(request);
+}
+
 /**
  * Checks whether we're in the Node.js or Browser environment
  *
diff --git a/yarn.lock b/yarn.lock

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,4 @@`
`1`	`1`	`*`
`2`	`2`	`!/dist/*/`
	`3`	`+!/src/*/`
`3`	`4`	`!/build/*/`
Original file line number	Diff line number	Diff line change
`@@ -1,2 +1,3 @@`
`1`	`1`	`*`
`2`	`2`	`!/dist/*/`
	`3`	`+!/src/*/`