[preset-env] Include / exclude module plugins properly

packages/babel-preset-env/src/index.js

@@ -1,5 +1,6 @@
 //@flow
 
+import { SemVer } from "semver";
 import { logPluginOrPolyfill } from "./debug";
 import getOptionSpecificExcludesFor from "./get-option-specific-excludes";
 import filterItems from "./filter-items";
@@ -20,6 +21,9 @@ import availablePlugins from "./available-plugins";
 import { filterStageFromList, prettifyTargets } from "./utils";
 import { declare } from "@babel/helper-plugin-utils";
 
+import typeof ModuleTransformationsType from "./module-transformations";
+import type { BuiltInsOption, Targets, ModuleOption } from "./types";
+
 export { isPluginRequired } from "./filter-items";
 
 const pluginListWithoutProposals = filterStageFromList(
@@ -56,6 +60,103 @@ export const transformIncludesAndExcludes = (opts: Array<string>): Object => {
   );
 };
 
+export const getModulesPluginNames = ({
+  modules,
+  transformations,
+  shouldTransformESM,
+  shouldTransformDynamicImport,
+}: {
+  modules: ModuleOption,
+  transformations: ModuleTransformationsType,
+  shouldTransformESM: boolean,
+  shouldTransformDynamicImport: boolean,
+}) => {
+  const modulesPluginNames = [];
+  if (modules !== false && transformations[modules]) {
+    if (shouldTransformESM) {
+      modulesPluginNames.push(transformations[modules]);
+    }
+
+    if (
+      shouldTransformDynamicImport &&
+      shouldTransformESM &&
+      modules !== "umd"
+    ) {
+      modulesPluginNames.push("proposal-dynamic-import");
+    } else {
+      if (shouldTransformDynamicImport) {
+        console.warn(
+          "Dynamic import can only be supported when transforming ES modules" +
+            " to AMD, CommonJS or SystemJS. Only the parser plugin will be enabled.",
+        );
+      }
+      modulesPluginNames.push("syntax-dynamic-import");
+    }
+  } else {
+    modulesPluginNames.push("syntax-dynamic-import");
+  }
+  return modulesPluginNames;
+};
+
+export const getPolyfillPlugins = ({
+  useBuiltIns,
+  corejs,
+  polyfillTargets,
+  include,
+  exclude,
+  proposals,
+  shippedProposals,
+  regenerator,
+  debug,
+}: {
+  useBuiltIns: BuiltInsOption,
+  corejs: typeof SemVer | null | false,
+  polyfillTargets: Targets,
+  include: Set<string>,
+  exclude: Set<string>,
+  proposals: boolean,
+  shippedProposals: boolean,
+  regenerator: boolean,
+  debug: boolean,
+}) => {
+  const polyfillPlugins = [];
+  if (useBuiltIns === "usage" || useBuiltIns === "entry") {
+    const pluginOptions = {
+      corejs,
+      polyfillTargets,
+      include,
+      exclude,
+      proposals,
+      shippedProposals,
+      regenerator,
+      debug,
+    };
+
+    if (corejs) {
+      if (useBuiltIns === "usage") {
+        if (corejs.major === 2) {
+          polyfillPlugins.push([addCoreJS2UsagePlugin, pluginOptions]);
+        } else {
+          polyfillPlugins.push([addCoreJS3UsagePlugin, pluginOptions]);
+        }
+        if (regenerator) {
+          polyfillPlugins.push([addRegeneratorUsagePlugin, pluginOptions]);
+        }
+      } else {
+        if (corejs.major === 2) {
+          polyfillPlugins.push([replaceCoreJS2EntryPlugin, pluginOptions]);
+        } else {
+          polyfillPlugins.push([replaceCoreJS3EntryPlugin, pluginOptions]);
+          if (!regenerator) {
+            polyfillPlugins.push([removeRegeneratorEntryPlugin, pluginOptions]);
+          }
+        }
+      }
+    }
+  }
+  return polyfillPlugins;
+};
+
 function supportsStaticESM(caller) {
   return !!(caller && caller.supportsStaticESM);
 }
@@ -115,114 +216,66 @@ export default declare((api, opts) => {
 
   const transformTargets = forceAllTransforms || hasUglifyTarget ? {} : targets;
 
-  const transformations = filterItems(
+  const modulesPluginNames = getModulesPluginNames({
+    modules,
+    transformations: moduleTransformations,
+    // TODO: Remove the 'api.caller' check eventually. Just here to prevent
+    // unnecessary breakage in the short term for users on older betas/RCs.
+    shouldTransformESM:
+      modules !== "auto" || !api.caller || !api.caller(supportsStaticESM),
+    shouldTransformDynamicImport:
+      modules !== "auto" || !api.caller || !api.caller(supportsDynamicImport),
+  });
+
+  const pluginNames = filterItems(
     shippedProposals ? pluginList : pluginListWithoutProposals,
     include.plugins,
     exclude.plugins,
     transformTargets,
-    null,
+    modulesPluginNames,
     getOptionSpecificExcludesFor({ loose }),
     pluginSyntaxMap,
   );
 
-  const plugins = [];
-  const pluginUseBuiltIns = useBuiltIns !== false;
-
-  if (modules !== false && moduleTransformations[modules]) {
-    // TODO: Remove the 'api.caller' check eventually. Just here to prevent
-    // unnecessary breakage in the short term for users on older betas/RCs.
-    const shouldTransformESM =
-      modules !== "auto" || !api.caller || !api.caller(supportsStaticESM);
-    const shouldTransformDynamicImport =
-      modules !== "auto" || !api.caller || !api.caller(supportsDynamicImport);
-
-    if (shouldTransformESM) {
-      // NOTE: not giving spec here yet to avoid compatibility issues when
-      // transform-modules-commonjs gets its spec mode
-      plugins.push([getPlugin(moduleTransformations[modules]), { loose }]);
-    }
-
-    if (
-      shouldTransformDynamicImport &&
-      shouldTransformESM &&
-      modules !== "umd"
-    ) {
-      plugins.push([getPlugin("proposal-dynamic-import"), { loose }]);
-    } else {
-      if (shouldTransformDynamicImport) {
-        console.warn(
-          "Dynamic import can only be supported when transforming ES modules" +
-            " to AMD, CommonJS or SystemJS. Only the parser plugin will be enabled.",
-        );
-      }
-      plugins.push(getPlugin("syntax-dynamic-import"));
-    }
-  } else {
-    plugins.push(getPlugin("syntax-dynamic-import"));
-  }
+  const polyfillPlugins = getPolyfillPlugins({
+    useBuiltIns,
+    corejs,
+    polyfillTargets: targets,
+    include: include.builtIns,
+    exclude: exclude.builtIns,
+    proposals,
+    shippedProposals,
+    regenerator: pluginNames.has("transform-regenerator"),
+    debug,
+  });
 
-  transformations.forEach(pluginName =>
-    plugins.push([
+  const pluginUseBuiltIns = useBuiltIns !== false;
+  const plugins = Array.from(pluginNames)
+    .map(pluginName => [
       getPlugin(pluginName),
       { spec, loose, useBuiltIns: pluginUseBuiltIns },
-    ]),
-  );
+    ])
+    .concat(polyfillPlugins);
 
   if (debug) {
     console.log("@babel/preset-env: `DEBUG` option");
     console.log("\nUsing targets:");
     console.log(JSON.stringify(prettifyTargets(targets), null, 2));
     console.log(`\nUsing modules transform: ${modules.toString()}`);
     console.log("\nUsing plugins:");
-    transformations.forEach(transform => {
-      logPluginOrPolyfill(transform, targets, pluginList);
+    pluginNames.forEach(pluginName => {
+      logPluginOrPolyfill(pluginName, targets, pluginList);
     });
 
     if (!useBuiltIns) {
       console.log(
         "\nUsing polyfills: No polyfills were added, since the `useBuiltIns` option was not set.",
       );
     } else {
+      // NOTE: Polyfill plugins are outputting debug info internally
       console.log(`\nUsing polyfills with \`${useBuiltIns}\` option:`);
     }
   }
 
-  if (useBuiltIns === "usage" || useBuiltIns === "entry") {
-    const regenerator = transformations.has("transform-regenerator");
-
-    const pluginOptions = {
-      corejs,
-      polyfillTargets: targets,
-      include: include.builtIns,
-      exclude: exclude.builtIns,
-      proposals,
-      shippedProposals,
-      regenerator,
-      debug,
-    };
-
-    if (corejs) {
-      if (useBuiltIns === "usage") {
-        if (corejs.major === 2) {
-          plugins.push([addCoreJS2UsagePlugin, pluginOptions]);
-        } else {
-          plugins.push([addCoreJS3UsagePlugin, pluginOptions]);
-        }
-        if (regenerator) {
-          plugins.push([addRegeneratorUsagePlugin, pluginOptions]);
-        }
-      } else {
-        if (corejs.major === 2) {
-          plugins.push([replaceCoreJS2EntryPlugin, pluginOptions]);
-        } else {
-          plugins.push([replaceCoreJS3EntryPlugin, pluginOptions]);
-          if (!regenerator) {
-            plugins.push([removeRegeneratorEntryPlugin, pluginOptions]);
-          }
-        }
-      }
-    }
-  }
-
   return { plugins };
 });

packages/babel-preset-env/src/normalize-options.js

@@ -32,23 +32,29 @@ const validateTopLevelOptions = (options: Options) => {
   }
 };
 
-const allPluginsList = [
-  ...Object.keys(pluginsList),
+const allPluginsList = Object.keys(pluginsList);
+
+// NOTE: Since module plugins are handled seperatly compared to other plugins (via the "modules" option) it
+// should only be possible to exclude and not include module plugins, otherwise it's possible that preset-env
+// will add a module plugin twice.
+const modulePlugins = [
+  "proposal-dynamic-import",
   ...Object.keys(moduleTransformations).map(m => moduleTransformations[m]),
 ];
 
-const validIncludesAndExcludesWithoutCoreJS = new Set(allPluginsList);
-
-const validIncludesAndExcludesWithCoreJS2 = new Set([
-  ...allPluginsList,
-  ...Object.keys(corejs2Polyfills),
-  ...defaultWebIncludes,
-]);
-
-const validIncludesAndExcludesWithCoreJS3 = new Set([
-  ...allPluginsList,
-  ...Object.keys(corejs3Polyfills),
-]);
+const getValidIncludesAndExcludes = (
+  type: "include" | "exclude",
+  corejs: number | false,
+) =>
+  new Set([
+    ...allPluginsList,
+    ...(type === "exclude" ? modulePlugins : []),
+    ...(corejs
+      ? corejs == 2
+        ? [...Object.keys(corejs2Polyfills), ...defaultWebIncludes]
+        : Object.keys(corejs3Polyfills)
+      : []),
+  ]);
 
 const pluginToRegExp = (plugin: PluginListItem) => {
   if (plugin instanceof RegExp) return plugin;
@@ -59,14 +65,14 @@ const pluginToRegExp = (plugin: PluginListItem) => {
   }
 };
 
-const selectPlugins = (regexp: RegExp | null, corejs: number | false) =>
-  Array.from(
-    corejs
-      ? corejs == 2
-        ? validIncludesAndExcludesWithCoreJS2
-        : validIncludesAndExcludesWithCoreJS3
-      : validIncludesAndExcludesWithoutCoreJS,
-  ).filter(item => regexp instanceof RegExp && regexp.test(item));
+const selectPlugins = (
+  regexp: RegExp | null,
+  type: "include" | "exclude",
+  corejs: number | false,
+) =>
+  Array.from(getValidIncludesAndExcludes(type, corejs)).filter(
+    item => regexp instanceof RegExp && regexp.test(item),
+  );
 
 const flatten = <T>(array: Array<Array<T>>): Array<T> => [].concat(...array);
 
@@ -78,7 +84,7 @@ const expandIncludesAndExcludes = (
   if (plugins.length === 0) return [];
 
   const selectedPlugins = plugins.map(plugin =>
-    selectPlugins(pluginToRegExp(plugin), corejs),
+    selectPlugins(pluginToRegExp(plugin), type, corejs),
   );
   const invalidRegExpList = plugins.filter(
     (p, i) => selectedPlugins[i].length === 0,

packages/babel-preset-env/test/debug-fixtures/corejs-without-usebuiltins/stdout.txt

@@ -40,6 +40,8 @@ Using plugins:
   transform-member-expression-literals {}
   transform-property-literals {}
   transform-reserved-words {}
+  transform-modules-commonjs {}
+  proposal-dynamic-import {}
 
 Using polyfills: No polyfills were added, since the `useBuiltIns` option was not set.
 Successfully compiled 1 file with Babel.

packages/babel-preset-env/test/debug-fixtures/entry-corejs2-android/stdout.txt

@@ -38,6 +38,8 @@ Using plugins:
   proposal-optional-catch-binding { "android":"4" }
   transform-named-capturing-groups-regex { "android":"4" }
   transform-reserved-words { "android":"4" }
+  transform-modules-commonjs { "android":"4" }
+  proposal-dynamic-import { "android":"4" }
 
 Using polyfills with `entry` option:
 

packages/babel-preset-env/test/debug-fixtures/entry-corejs2-electron/stdout.txt

@@ -34,6 +34,8 @@ Using plugins:
   transform-named-capturing-groups-regex { "electron":"0.36" }
   transform-member-expression-literals { "electron":"0.36" }
   transform-property-literals { "electron":"0.36" }
+  transform-modules-commonjs { "electron":"0.36" }
+  proposal-dynamic-import { "electron":"0.36" }
 
 Using polyfills with `entry` option:
 

packages/babel-preset-env/test/debug-fixtures/entry-corejs2-force-all-transforms/stdout.txt

@@ -40,6 +40,7 @@ Using plugins:
   transform-member-expression-literals {}
   transform-property-literals {}
   transform-reserved-words {}
+  syntax-dynamic-import { "chrome":"55" }
 
 Using polyfills with `entry` option:
 

packages/babel-preset-env/test/debug-fixtures/entry-corejs2-no-import/stdout.txt

@@ -20,6 +20,8 @@ Using plugins:
   proposal-json-strings { "node":"6" }
   proposal-optional-catch-binding { "node":"6" }
   transform-named-capturing-groups-regex { "node":"6" }
+  transform-modules-commonjs { "node":"6" }
+  proposal-dynamic-import { "node":"6" }
 
 Using polyfills with `entry` option:
 

packages/babel-preset-env/test/debug-fixtures/entry-corejs2-proposals-chrome-71/stdout.txt

@@ -12,6 +12,8 @@ Using plugins:
   syntax-object-rest-spread { "chrome":"71" }
   syntax-json-strings { "chrome":"71" }
   syntax-optional-catch-binding { "chrome":"71" }
+  transform-modules-commonjs { "chrome":"71" }
+  proposal-dynamic-import { "chrome":"71" }
 
 Using polyfills with `entry` option: