Merge pull request #6905 from loganfsmyth/config-options-merge

Merge all config & programmatic plugins/preset rather than duplicating

packages/babel-core/src/config/option-assertions.js

@@ -146,16 +146,31 @@ function assertPluginItem(
     if (value.length === 0) {
       throw new Error(`.${key}[${index}] must include an object`);
     }
-    if (value.length > 2) {
+
-      throw new Error(`.${key}[${index}] may only be a two-tuple`);
+    if (value.length > 3) {
+      throw new Error(
+        `.${key}[${index}] may only be a two-tuple or three-tuple`,
+      );
     }
 
     assertPluginTarget(key, index, true, value[0]);
 
-    if (value.length === 2) {
+    if (value.length > 1) {
       const opts = value[1];
-      if (opts != null && (typeof opts !== "object" || Array.isArray(opts))) {
+      if (
-        throw new Error(`.${key}[${index}][1] must be an object, or undefined`);
+        opts !== undefined &&
+        opts !== false &&
+        (typeof opts !== "object" || Array.isArray(opts))
+      ) {
+        throw new Error(
+          `.${key}[${index}][1] must be an object, false, or undefined`,
+        );
+      }
+    }
+    if (value.length === 3) {
+      const name = value[2];
+      if (name !== undefined && typeof name !== "string") {
+        throw new Error(`.${key}[${index}][2] must be a string, or undefined`);
       }
     }
   } else {

packages/babel-core/src/config/option-manager.js

@@ -30,13 +30,9 @@ export default function manageOptions(opts: {}): {
 }
 
 class OptionManager {
-  constructor() {
+  optionDefaults: ValidatedOptions = {};
-    this.options = {};
+  options: ValidatedOptions = {};
-    this.passes = [[]];
+  passes: Array<Array<Plugin>> = [[]];
-  }
-
-  options: ValidatedOptions;
-  passes: Array<Array<Plugin>>;
 
   /**
    * This is called when we want to merge the input `opts` into the
@@ -46,16 +42,23 @@ class OptionManager {
    *  - `loc` is used to point to the original config.
    *  - `dirname` is used to resolve plugins relative to it.
    */
-  mergeOptions(config: MergeOptions, pass: Array<Plugin>, envName: string) {
+  mergeOptions(
-    const result = loadConfig(config);
+    config: {
-
+      plugins: Array<BasicDescriptor>,
-    const plugins = result.plugins.map(descriptor =>
+      presets: Array<BasicDescriptor>,
+    },
+    pass: Array<Plugin>,
+    envName: string,
+  ) {
+    const plugins = config.plugins.map(descriptor =>
       loadPluginDescriptor(descriptor, envName),
     );
-    const presets = result.presets.map(descriptor => ({
+    const presets = config.presets.map(descriptor => {
-      pass: descriptor.ownPass ? [] : pass,
+      return {
-      preset: loadPresetDescriptor(descriptor, envName),
+        preset: loadPresetDescriptor(descriptor, envName),
-    }));
+        pass: descriptor.ownPass ? [] : pass,
+      };
+    });
 
     // resolve presets
     if (presets.length > 0) {
@@ -68,7 +71,18 @@ class OptionManager {
       );
 
       presets.forEach(({ preset, pass }) => {
-        this.mergeOptions(preset, pass, envName);
+        const loadedConfig = loadConfig(preset);
+        this.mergeOptions(
+          {
+            // Call dedupDescriptors() to remove 'false' descriptors.
+            plugins: dedupDescriptors(loadedConfig.plugins),
+            presets: dedupDescriptors(loadedConfig.presets),
+          },
+          pass,
+          envName,
+        );
+
+        merge(this.optionDefaults, normalizeOptions(loadedConfig.options));
       });
     }
 
@@ -76,24 +90,23 @@ class OptionManager {
     if (plugins.length > 0) {
       pass.unshift(...plugins);
     }
+  }
 
-    const options = Object.assign({}, result.options);
+  mergeConfigChain(chain: $ReadOnlyArray<MergeOptions>, envName: string) {
-    delete options.extends;
+    const config = dedupLoadedConfigs(chain.map(config => loadConfig(config)));
-    delete options.env;
-    delete options.plugins;
-    delete options.presets;
-    delete options.passPerPreset;
-    delete options.ignore;
-    delete options.only;
 
-    // "sourceMap" is just aliased to sourceMap, so copy it over as
+    this.mergeOptions(
-    // we merge the options together.
+      {
-    if (options.sourceMap) {
+        plugins: config.plugins,
-      options.sourceMaps = options.sourceMap;
+        presets: config.presets,
-      delete options.sourceMap;
+      },
-    }
+      this.passes[0],
+      envName,
+    );
 
-    merge(this.options, options);
+    config.options.forEach(opts => {
+      merge(this.options, normalizeOptions(opts));
+    });
   }
 
   init(inputOpts: {}) {
@@ -106,9 +119,7 @@ class OptionManager {
     if (!configChain) return null;
 
     try {
-      for (const config of configChain) {
+      this.mergeConfigChain(configChain, envName);
-        this.mergeOptions(config, this.passes[0], envName);
-      }
     } catch (e) {
       // There are a few case where thrown errors will try to annotate themselves multiple times, so
       // to keep things simple we just bail out if re-wrapping the message.
@@ -119,7 +130,7 @@ class OptionManager {
       throw e;
     }
 
-    const opts: Object = this.options;
+    const opts: Object = merge(this.optionDefaults, this.options);
 
     // Tack the passes onto the object itself so that, if this object is passed back to Babel a second time,
     // it will be in the right structure to not change behavior.
@@ -140,9 +151,29 @@ class OptionManager {
   }
 }
 
+function normalizeOptions(opts: ValidatedOptions): ValidatedOptions {
+  const options = Object.assign({}, opts);
+  delete options.extends;
+  delete options.env;
+  delete options.plugins;
+  delete options.presets;
+  delete options.passPerPreset;
+  delete options.ignore;
+  delete options.only;
+
+  // "sourceMap" is just aliased to sourceMap, so copy it over as
+  // we merge the options together.
+  if (options.sourceMap) {
+    options.sourceMaps = options.sourceMap;
+    delete options.sourceMap;
+  }
+  return options;
+}
+
 type BasicDescriptor = {
+  name: string | void,
   value: {} | Function,
-  options: {} | void,
+  options: {} | void | false,
   dirname: string,
   alias: string,
   ownPass?: boolean,
@@ -155,14 +186,16 @@ type LoadedDescriptor = {
   alias: string,
 };
 
+type LoadedConfig = {
+  options: ValidatedOptions,
+  plugins: Array<BasicDescriptor>,
+  presets: Array<BasicDescriptor>,
+};
+
 /**
  * Load and validate the given config into a set of options, plugins, and presets.
  */
-const loadConfig = makeWeakCache((config: MergeOptions): {
+const loadConfig = makeWeakCache((config: MergeOptions): LoadedConfig => {
-  options: {},
-  plugins: Array<BasicDescriptor>,
-  presets: Array<BasicDescriptor>,
-} => {
   const options = config.options;
 
   const plugins = (config.options.plugins || []).map((plugin, index) =>
@@ -172,25 +205,133 @@ const loadConfig = makeWeakCache((config: MergeOptions): {
     }),
   );
 
+  assertNoDuplicates(plugins);
+
   const presets = (config.options.presets || []).map((preset, index) =>
     createDescriptor(preset, loadPreset, config.dirname, {
       index,
       alias: config.alias,
-      ownPass: config.options.passPerPreset,
+      ownPass: options.passPerPreset,
     }),
   );
 
+  assertNoDuplicates(presets);
+
   return { options, plugins, presets };
 });
 
+function assertNoDuplicates(items: Array<BasicDescriptor>): void {
+  const map = new Map();
+
+  for (const item of items) {
+    if (typeof item.value !== "function") continue;
+
+    let nameMap = map.get(item.value);
+    if (!nameMap) {
+      nameMap = new Set();
+      map.set(item.value, nameMap);
+    }
+
+    if (nameMap.has(item.name)) {
+      throw new Error(
+        [
+          `Duplicate plugin/preset detected.`,
+          `If you'd like to use two separate instances of a plugin,`,
+          `they neen separate names, e.g.`,
+          ``,
+          `  plugins: [`,
+          `    ['some-plugin', {}],`,
+          `    ['some-plugin', {}, 'some unique name'],`,
+          `  ]`,
+        ].join("\n"),
+      );
+    }
+
+    nameMap.add(item.name);
+  }
+}
+
+function dedupLoadedConfigs(
+  items: Array<LoadedConfig>,
+): {
+  plugins: Array<BasicDescriptor>,
+  presets: Array<BasicDescriptor>,
+  options: Array<ValidatedOptions>,
+} {
+  const options = [];
+  const plugins = [];
+  const presets = [];
+
+  for (const item of items) {
+    plugins.push(...item.plugins);
+    presets.push(...item.presets);
+    options.push(item.options);
+  }
+
+  return {
+    options,
+    plugins: dedupDescriptors(plugins),
+    presets: dedupDescriptors(presets),
+  };
+}
+
+function dedupDescriptors(
+  items: Array<BasicDescriptor>,
+): Array<BasicDescriptor> {
+  const map: Map<
+    Function,
+    Map<string | void, { value: BasicDescriptor | null }>,
+  > = new Map();
+
+  const descriptors = [];
+
+  for (const item of items) {
+    if (typeof item.value === "function") {
+      const fnKey = item.value;
+      let nameMap = map.get(fnKey);
+      if (!nameMap) {
+        nameMap = new Map();
+        map.set(fnKey, nameMap);
+      }
+      let desc = nameMap.get(item.name);
+      if (!desc) {
+        desc = { value: null };
+        descriptors.push(desc);
+
+        // Treat passPerPreset presets as unique, skipping them
+        // in the merge processing steps.
+        if (!item.ownPass) nameMap.set(item.name, desc);
+      }
+
+      if (item.options === false) {
+        desc.value = null;
+      } else {
+        desc.value = item;
+      }
+    } else {
+      descriptors.push({ value: item });
+    }
+  }
+
+  return descriptors.reduce((acc, desc) => {
+    if (desc.value) acc.push(desc.value);
+    return acc;
+  }, []);
+}
+
 /**
  * Load a generic plugin/preset from the given descriptor loaded from the config object.
  */
 const loadDescriptor = makeWeakCache(
   (
-    { value, options = {}, dirname, alias }: BasicDescriptor,
+    { value, options, dirname, alias }: BasicDescriptor,
     cache: CacheConfigurator<{ envName: string }>,
   ): LoadedDescriptor => {
+    // Disabled presets should already have been filtered out
+    if (options === false) throw new Error("Assertion failure");
+
+    options = options || {};
+
     let item = value;
     if (typeof value === "function") {
       const api = Object.assign(Object.create(context), {
@@ -262,6 +403,7 @@ const instantiatePlugin = makeWeakCache(
 
     if (plugin.inherits) {
       const inheritsDescriptor = {
+        name: undefined,
         alias: `${alias}$inherits`,
         value: plugin.inherits,
         options,
@@ -327,10 +469,16 @@ function createDescriptor(
     ownPass?: boolean,
   },
 ): BasicDescriptor {
+  let name;
   let options;
   let value = pair;
   if (Array.isArray(value)) {
-    [value, options] = value;
+    if (value.length === 3) {
+      // $FlowIgnore - Flow doesn't like the multiple tuple types.
+      [value, options, name] = value;
+    } else {
+      [value, options] = value;
+    }
   }
 
   let filepath = null;
@@ -366,6 +514,7 @@ function createDescriptor(
   }
 
   return {
+    name,
     alias: filepath || `${alias}$${index}`,
     value,
     options,

packages/babel-core/src/config/options.js

@@ -187,8 +187,13 @@ export type EnvSet<T> = {
 export type IgnoreItem = string | Function | RegExp;
 export type IgnoreList = $ReadOnlyArray<IgnoreItem>;
 
+export type PluginOptions = {} | void | false;
 export type PluginTarget = string | {} | Function;
-export type PluginItem = PluginTarget | [PluginTarget, {} | void];
+export type PluginItem =
+  | Plugin
+  | PluginTarget
+  | [PluginTarget, PluginOptions]
+  | [PluginTarget, PluginOptions, string];
 export type PluginList = $ReadOnlyArray<PluginItem>;
 
 export type SourceMapsOption = boolean | "inline" | "both";

packages/babel-core/test/api.js

@@ -310,30 +310,30 @@ describe("api", function() {
     assert.equal(
       result.code,
       [
-        "argtwo;",
+        "thirteen;",
-        "argone;",
+        "fourteen;",
-        "eleven;",
+        "seventeen;",
-        "twelve;",
+        "eighteen;",
         "one;",
         "two;",
+        "eleven;",
+        "twelve;",
+        "argtwo;",
+        "argone;",
         "five;",
         "six;",
         "three;",
         "four;",
-        "seventeen;",
-        "eighteen;",
         "nineteen;",
         "twenty;",
-        "thirteen;",
-        "fourteen;",
         "fifteen;",
         "sixteen;",
-        "argthree;",
-        "argfour;",
         "seven;",
         "eight;",
         "nine;",
         "ten;",
+        "argthree;",
+        "argfour;",
       ].join("\n"),
     );
   });

packages/babel-core/test/config-loading.js

@@ -42,10 +42,10 @@ describe("@babel/core config loading", () => {
 
       const options1 = loadConfig(opts).options;
       expect(options1.plugins.map(p => p.key)).to.eql([
-        "plugin6",
-        "plugin5",
         "plugin1",
         "plugin2",
+        "plugin6",
+        "plugin5",
         "plugin4",
         "plugin3",
       ]);
@@ -86,7 +86,7 @@ describe("@babel/core config loading", () => {
       expect(options2.plugins.length).to.equal(options1.plugins.length);
 
       for (let i = 0; i < options1.plugins.length; i++) {
-        if (i === 2) {
+        if (i === 0) {
           expect(options2.plugins[i]).not.to.equal(options1.plugins[i]);
         } else {
           expect(options2.plugins[i]).to.equal(options1.plugins[i]);
@@ -99,7 +99,7 @@ describe("@babel/core config loading", () => {
       expect(options3.plugins.length).to.equal(options1.plugins.length);
 
       for (let i = 0; i < options1.plugins.length; i++) {
-        if (i === 2 || i === 5) {
+        if (i === 0 || i === 5) {
           expect(options3.plugins[i]).not.to.equal(options1.plugins[i]);
         } else {
           expect(options3.plugins[i]).to.equal(options1.plugins[i]);
@@ -150,7 +150,7 @@ describe("@babel/core config loading", () => {
       expect(options2.plugins.length).to.equal(options1.plugins.length);
 
       for (let i = 0; i < options1.plugins.length; i++) {
-        if (i === 2 || i === 3 || i === 4 || i === 5 || i === 6) {
+        if (i === 0 || i === 1 || i === 4 || i === 5 || i === 6) {
           expect(options2.plugins[i]).not.to.equal(options1.plugins[i]);
         } else {
           expect(options2.plugins[i]).to.equal(options1.plugins[i]);
@@ -185,7 +185,7 @@ describe("@babel/core config loading", () => {
       expect(options2.plugins.length).to.equal(options1.plugins.length);
 
       for (let i = 0; i < options2.plugins.length; i++) {
-        if (i === 0) {
+        if (i === 2) {
           expect(options2.plugins[i]).not.to.equal(options1.plugins[i]);
         } else {
           expect(options2.plugins[i]).to.equal(options1.plugins[i]);
@@ -205,7 +205,7 @@ describe("@babel/core config loading", () => {
       expect(options2.plugins.length).to.equal(options1.plugins.length);
 
       for (let i = 0; i < options2.plugins.length; i++) {
-        if (i === 1) {
+        if (i === 3) {
           expect(options2.plugins[i]).not.to.equal(options1.plugins[i]);
         } else {
           expect(options2.plugins[i]).to.equal(options1.plugins[i]);
@@ -224,7 +224,7 @@ describe("@babel/core config loading", () => {
       expect(options2.plugins.length).to.equal(options1.plugins.length);
 
       for (let i = 0; i < options1.plugins.length; i++) {
-        if (i === 0) {
+        if (i === 2) {
           expect(options2.plugins[i]).not.to.equal(options1.plugins[i]);
         } else {
           expect(options2.plugins[i]).to.equal(options1.plugins[i]);
@@ -243,7 +243,7 @@ describe("@babel/core config loading", () => {
       expect(options2.plugins.length).to.equal(options1.plugins.length);
 
       for (let i = 0; i < options1.plugins.length; i++) {
-        if (i === 1) {
+        if (i === 3) {
           expect(options2.plugins[i]).not.to.equal(options1.plugins[i]);
         } else {
           expect(options2.plugins[i]).to.equal(options1.plugins[i]);

packages/babel-core/test/option-manager.js

@@ -1,20 +1,127 @@
 import assert from "assert";
-import manageOptions from "../lib/config/option-manager";
+import { loadOptions } from "../lib";
 import path from "path";
 
 describe("option-manager", () => {
   it("throws for babel 5 plugin", () => {
     return assert.throws(() => {
-      manageOptions({
+      loadOptions({
         plugins: [({ Plugin }) => new Plugin("object-assign", {})],
       });
     }, /Babel 5 plugin is being run with an unsupported Babel/);
   });
 
+  describe("config plugin/preset flattening and overriding", () => {
+    function makePlugin() {
+      const calls = [];
+      const plugin = (api, opts) => {
+        calls.push(opts);
+        return {};
+      };
+      return { plugin, calls };
+    }
+
+    it("should throw if a plugin is repeated", () => {
+      const { calls, plugin } = makePlugin();
+
+      assert.throws(() => {
+        loadOptions({
+          plugins: [plugin, plugin],
+        });
+      }, /Duplicate plugin\/preset detected/);
+      assert.deepEqual(calls, []);
+    });
+
+    it("should not throw if a repeated plugin has a different name", () => {
+      const { calls: calls1, plugin: plugin1 } = makePlugin();
+      const { calls: calls2, plugin: plugin2 } = makePlugin();
+
+      loadOptions({
+        plugins: [[plugin1, { arg: 1 }], [plugin2, { arg: 2 }, "some-name"]],
+      });
+      assert.deepEqual(calls1, [{ arg: 1 }]);
+      assert.deepEqual(calls2, [{ arg: 2 }]);
+    });
+
+    it("should merge .env[] plugins with parent presets", () => {
+      const { calls: calls1, plugin: plugin1 } = makePlugin();
+      const { calls: calls2, plugin: plugin2 } = makePlugin();
+
+      loadOptions({
+        envName: "test",
+        plugins: [[plugin1, { arg: 1 }]],
+        env: {
+          test: {
+            plugins: [[plugin1, { arg: 3 }], [plugin2, { arg: 2 }]],
+          },
+        },
+      });
+      assert.deepEqual(calls1, [{ arg: 3 }]);
+      assert.deepEqual(calls2, [{ arg: 2 }]);
+    });
+
+    it("should throw if a preset is repeated", () => {
+      const { calls, plugin: preset } = makePlugin();
+
+      assert.throws(() => {
+        loadOptions({
+          presets: [preset, preset],
+        });
+      }, /Duplicate plugin\/preset detected/);
+      assert.deepEqual(calls, []);
+    });
+
+    it("should not throw if a repeated preset has a different name", () => {
+      const { calls: calls1, plugin: preset1 } = makePlugin();
+      const { calls: calls2, plugin: preset2 } = makePlugin();
+
+      loadOptions({
+        presets: [[preset1, { arg: 1 }], [preset2, { arg: 2 }, "some-name"]],
+      });
+      assert.deepEqual(calls1, [{ arg: 1 }]);
+      assert.deepEqual(calls2, [{ arg: 2 }]);
+    });
+
+    it("should merge .env[] presets with parent presets", () => {
+      const { calls: calls1, plugin: preset1 } = makePlugin();
+      const { calls: calls2, plugin: preset2 } = makePlugin();
+
+      loadOptions({
+        envName: "test",
+        presets: [[preset1, { arg: 1 }]],
+        env: {
+          test: {
+            presets: [[preset1, { arg: 3 }], [preset2, { arg: 2 }]],
+          },
+        },
+      });
+      assert.deepEqual(calls1, [{ arg: 3 }]);
+      assert.deepEqual(calls2, [{ arg: 2 }]);
+    });
+
+    it("should not merge .env[] presets with parent presets when passPerPreset", () => {
+      const { calls: calls1, plugin: preset1 } = makePlugin();
+      const { calls: calls2, plugin: preset2 } = makePlugin();
+
+      loadOptions({
+        envName: "test",
+        passPerPreset: true,
+        presets: [[preset1, { arg: 1 }]],
+        env: {
+          test: {
+            presets: [[preset1, { arg: 3 }], [preset2, { arg: 2 }]],
+          },
+        },
+      });
+      assert.deepEqual(calls1, [{ arg: 1 }, { arg: 3 }]);
+      assert.deepEqual(calls2, [{ arg: 2 }]);
+    });
+  });
+
   describe("mergeOptions", () => {
     it("throws for removed babel 5 options", () => {
       return assert.throws(() => {
-        manageOptions({
+        loadOptions({
           randomOption: true,
         });
       }, /Unknown option: .randomOption/);
@@ -23,7 +130,7 @@ describe("option-manager", () => {
     it("throws for removed babel 5 options", () => {
       return assert.throws(
         () => {
-          manageOptions({
+          loadOptions({
             auxiliaryComment: true,
             blacklist: true,
           });
@@ -35,7 +142,7 @@ describe("option-manager", () => {
 
     it("throws for resolved but erroring preset", () => {
       return assert.throws(() => {
-        manageOptions({
+        loadOptions({
           presets: [
             path.join(__dirname, "fixtures/option-manager/not-a-preset"),
           ],
@@ -47,7 +154,7 @@ describe("option-manager", () => {
   describe("presets", function() {
     function presetTest(name) {
       it(name, function() {
-        const { options, passes } = manageOptions({
+        const options = loadOptions({
           presets: [
             path.join(__dirname, "fixtures/option-manager/presets", name),
           ],
@@ -55,8 +162,7 @@ describe("option-manager", () => {
 
         assert.equal(true, Array.isArray(options.plugins));
         assert.equal(1, options.plugins.length);
-        assert.equal(1, passes.length);
+        assert.equal(0, options.presets.length);
-        assert.equal(1, passes[0].length);
       });
     }
 
@@ -64,7 +170,7 @@ describe("option-manager", () => {
       it(name, function() {
         assert.throws(
           () =>
-            manageOptions({
+            loadOptions({
               presets: [
                 path.join(__dirname, "fixtures/option-manager/presets", name),
               ],