fix(types): Enable plugin to be imported under ES6 (#316)

Because the webpack plugin is written in vanilla JS (rather than TS), we have a manually-generated type declaration file, where we default-export the `SentryCliPlugin` class. We also have a file (`cjs.js`) which is meant to make the plugin play nicely with CommonJS imports. And if the user is indeed using CommonJS imports (`const SentryWebpackPlugin = require('@sentry/webpack-plugin')`), that all works great.

But if you try to do ES6 imports, the only form which gets you anything besides `undefined` is `import * as SentryWebpackPlugin from '@sentry/webpack-plugin`.* As far as TS is concerned, though, that creates a namespace rather than a class, and so it will refuse to let you call `new SentryWebpackPlugin`, instead telling you that `SentryWebpackPlugin` is not contructable. One solution is to use `esModuleInterop`, but that's [gotten us into trouble before](getsentry/sentry-javascript#3401 (comment)) and in fact breaks some of our own SDK code.

 *The other two options are `import SentryCliPlugin as SentryWebpackPlugin from '@sentry/webpack-plugin'` and `import { default as SentryWebpackPlugin } from '@sentry/webpack-plugin'`, both of which let you call `new` on them, but neither of which currently actually gives you anything other than `undefined`). 

The alternative (which is what's done in this change) is to do two things:

1) Use the older, more universal `exports = ...` syntax in our type declaration file rather than our current `export default ...` syntax (which, according to the TS docs, [actually won’t work](https://www.typescriptlang.org/docs/handbook/declaration-files/templates/module-d-ts.html#default-exports) unless `esModuleInterop` is set to `true`). This then necessitates moving the options interface into a namespace for the module.

2) Explicitly create an export under the name `default`, so that the two alternatives above actually come up with something rather than being undefined.

This shouldn't affect any current imports, as the first change just moves to an equivalent syntax, and the second change is purely additive.

.github/workflows/ci.yml

@@ -38,6 +38,6 @@ jobs:
         with:
           node-version: ${{ matrix.node }}
 
-      - run: yarn install
+      - run: yarn install --ignore-engines # Webpack 5 does not support Node v8, but we still do for Webpack 4
 
       - run: yarn ${{ matrix.test-command }}

index.d.ts

@@ -7,105 +7,129 @@ import {
   SourceMapsPathDescriptor,
 } from '@sentry/cli';
 
-export interface SentryCliPluginOptions
-  extends Pick<
-      SentryCliOptions,
-      'url' | 'authToken' | 'org' | 'project' | 'vscRemote' | 'dist' | 'silent'
-    >,
-    Pick<
-      SentryCliUploadSourceMapsOptions,
-      | 'ignoreFile'
-      | 'rewrite'
-      | 'sourceMapReference'
-      | 'stripPrefix'
-      | 'stripCommonPrefix'
-      | 'validate'
-      | 'urlPrefix'
-      | 'urlSuffix'
-      | 'ext'
-    > {
-  /**
-   * Filepaths to scan recursively for source and source map files
-   */
-  include:
-    | string
-    | SourceMapsPathDescriptor
-    | Array<string | SourceMapsPathDescriptor>;
+declare namespace SentryCliPlugin {
+  export interface SentryCliPluginOptions
+    extends Pick<
+        SentryCliOptions,
+        | 'url'
+        | 'authToken'
+        | 'org'
+        | 'project'
+        | 'vscRemote'
+        | 'dist'
+        | 'silent'
+      >,
+      Pick<
+        SentryCliUploadSourceMapsOptions,
+        | 'ignoreFile'
+        | 'rewrite'
+        | 'sourceMapReference'
+        | 'stripPrefix'
+        | 'stripCommonPrefix'
+        | 'validate'
+        | 'urlPrefix'
+        | 'urlSuffix'
+        | 'ext'
+      > {
+    /**
+     * Filepaths to scan recursively for source and source map files
+     */
+    include:
+      | string
+      | SourceMapsPathDescriptor
+      | Array<string | SourceMapsPathDescriptor>;
 
-  /**
-   * Filepaths to ignore when scanning for sources and source maps
-   */
-  ignore?: string | Array<string>;
+    /**
+     * Filepaths to ignore when scanning for sources and source maps
+     */
+    ignore?: string | Array<string>;
 
-  /**
-   * Unique name of a release, must be a string, should uniquely identify your release,
-   * defaults to sentry-cli releases propose-version command which should always return the correct version
-   * (requires access to git CLI and root directory to be a valid repository).
-   */
-  release?: string;
+    /**
+     * Unique name of a release, must be a string, should uniquely identify your release,
+     * defaults to sentry-cli releases propose-version command which should always return the correct version
+     * (requires access to git CLI and root directory to be a valid repository).
+     */
+    release?: string;
 
-  /**
-   * A filter for entry points that should be processed.
-   * By default, the release will be injected into all entry points.
-   */
-  entries?: string[] | RegExp | ((key: string) => boolean);
+    /**
+     * A filter for entry points that should be processed.
+     * By default, the release will be injected into all entry points.
+     */
+    entries?: string[] | RegExp | ((key: string) => boolean);
 
-  /**
-   * Path to Sentry CLI config properties, as described in https://docs.sentry.io/learn/cli/configuration/#properties-files.
-   * By default, the config file is looked for upwards from the current path and defaults from ~/.sentryclirc are always loaded.
-   */
-  configFile?: string;
+    /**
+     * Path to Sentry CLI config properties, as described in https://docs.sentry.io/learn/cli/configuration/#properties-files.
+     * By default, the config file is looked for upwards from the current path and defaults from ~/.sentryclirc are always loaded.
+     */
+    configFile?: string;
 
-  /**
-   * Determines whether processed release should be automatically finalized after artifacts upload.
-   * Defaults to `true`.
-   */
-  finalize?: boolean;
+    /**
+     * Determines whether processed release should be automatically finalized after artifacts upload.
+     * Defaults to `true`.
+     */
+    finalize?: boolean;
 
-  /**
-   * Determines whether plugin should be applied not more than once during whole webpack run.
-   * Useful when the process is performing multiple builds using the same config.
-   * Defaults to `false`.
-   */
-  runOnce?: boolean;
+    /**
+     * Determines whether plugin should be applied not more than once during whole webpack run.
+     * Useful when the process is performing multiple builds using the same config.
+     * Defaults to `false`.
+     */
+    runOnce?: boolean;
 
-  /**
-   * Attempts a dry run (useful for dev environments).
-   */
-  dryRun?: boolean;
+    /**
+     * Attempts a dry run (useful for dev environments).
+     */
+    dryRun?: boolean;
 
-  /**
-   * Print some useful debug information.
-   */
-  debug?: boolean;
+    /**
+     * Print some useful debug information.
+     */
+    debug?: boolean;
 
-  /**
-   * If true, will remove all previously uploaded artifacts from the configured release.
-   */
-  cleanArtifacts?: boolean;
+    /**
+     * If true, will remove all previously uploaded artifacts from the configured release.
+     */
+    cleanArtifacts?: boolean;
 
-  /**
-   * when Cli error occurs, plugin calls this function.
-   * webpack compilation failure can be chosen by calling invokeErr callback or not.
-   * defaults to `(err, invokeErr) => { invokeErr() }`
-   */
-  errorHandler?: (err: Error, invokeErr: () => void, compilation: Compilation) => void;
+    /**
+     * when Cli error occurs, plugin calls this function.
+     * webpack compilation failure can be chosen by calling invokeErr callback or not.
+     * defaults to `(err, invokeErr) => { invokeErr() }`
+     */
+    errorHandler?: (
+      err: Error,
+      invokeErr: () => void,
+      compilation: Compilation
+    ) => void;
 
-  /**
-   * Adds commits to sentry
-   */
-  setCommits?: SentryCliCommitsOptions;
+    /**
+     * Adds commits to sentry
+     */
+    setCommits?: SentryCliCommitsOptions;
 
-  /**
-   * Creates a new release deployment
-   */
-  deploy?: SentryCliNewDeployOptions;
+    /**
+     * Creates a new release deployment
+     */
+    deploy?: SentryCliNewDeployOptions;
+  }
 }
 
 declare class SentryCliPlugin implements WebpackPluginInstance {
-  options: SentryCliPluginOptions;
-  constructor(options: SentryCliPluginOptions);
+  options: SentryCliPlugin.SentryCliPluginOptions;
+  constructor(options: SentryCliPlugin.SentryCliPluginOptions);
   apply(compiler: Compiler): void;
 }
 
-export default SentryCliPlugin;
+// We need to use this older format (over `export default SentryCliPlugin`)
+// because we don't want people using the plugin in their TS projects to be
+// forced to set `esmoduleinterop` to `true`, which the newer syntax requires.
+// See
+// https://github.com/microsoft/TypeScript-Website/blob/6a36b3137182084c76cdf133c812fe3a5626dbf0/packages/documentation/copy/en/declaration-files/templates/module.d.ts.md#L95-L106
+// (linking to the docs in their raw form on GH rather than on the TS docs site
+// in case the docs site ever moves things around).
+//
+// Note that with this older format, no other top-level exports can exist, which
+// is why the exported interface above is wrapped in a namespace. See the
+// example in the above link and
+// https://github.com/microsoft/TypeScript-Website/blob/6a36b3137182084c76cdf133c812fe3a5626dbf0/packages/documentation/copy/en/declaration-files/templates/module.d.ts.md#L195-L214.
+export = SentryCliPlugin;

package.json

@@ -21,6 +21,7 @@
     "@sentry/cli": "^1.68.0"
   },
   "devDependencies": {
+    "@types/webpack": "^4.0.0 || ^5.0.0",
     "codecov": "^3.5.0",
     "eslint": "^5.16.0",
     "eslint-config-airbnb-base": "^13.1.0",
@@ -32,6 +33,9 @@
     "prettier-check": "^2.0.0",
     "webpack": "^4.39.3"
   },
+  "peerDependencies": {
+    "@types/webpack": "^4.0.0 || ^5.0.0"
+  },
   "scripts": {
     "lint": "run-s lint:prettier lint:eslint",
     "lint:prettier": "prettier-check 'src/**/*.js'",

src/cjs.js

@@ -1 +1,20 @@
 module.exports = require('./index').default;
+
+// The assignment to `default` below saves us from having to use
+// `esModuleInterop` (which then would force our users to set the same option),
+// by manually doing the one part of `esModuleInterop`'s job we actually need.
+//
+// (In order to avoid a breaking change, we need to stick with default-exporting
+// `SentryCliPlugin`. This means that if we want to use ES6 imports (in our own
+// use of the plugin), our options are:
+//
+// `import * as x from y`,
+// `import x from y`, and
+// `import {default as x} from y`.
+//
+// If we use the first option, it correctly pulls in the above `module.exports`
+// value, but it treats it as a namespace, not a class, and therefore refuses to
+// let it be used with `new`. If we use either of the other two, it looks for
+// `module.exports.default`, which will be undefined unless either we do (or
+// `esModuleInterop` does) the below.)
+module.exports.default = module.exports;