feature #544 Allow to call configureBabel with an external Babel configuration (Lyrkan, weaverryan)

This PR was squashed before being merged into the master branch (closes #544).

Discussion
----------

Allow to call configureBabel with an external Babel configuration

This PR fixes #543 by allowing to call `Encore.configureBabel()` even if an external Babel configuration exists.

Basically:

* `null` (or any falsy value in reality, to keep it simple) is always allowed as a first argument
* a warning will be displayed if the first argument is a callback and something like a `.babelrc` file is detected
* a warning will be displayed if the second argument contains a non-whitelisted option (ie. not `includeNodeModules`/`exclude`) and something like a `.babelrc` file is detected

```js
// Allowed without any warning
Encore.configureBabel(null, {
  includeNodeModules: ['foo']
});

// Displays a warning and the
// callback is ignored
Encore.configureBabel(
  () => { /* ... */ }
);

// Displays a warning and the
// useBuiltIns option is ignored.
Encore.configureBabel(null, {
  useBuiltIns: 'foo',
  includeNodeModules: ['foo'],
});
```

Commits
-------

a9af955 Merge branch 'master' into configure-babel-optional-callback
ffe3054 Recommend null instead of false for the first parameter of configureBabel()
644d1ac Allow to call configureBabel with an external Babel configuration

Author: weaverryan

index.js

@@ -766,6 +766,11 @@ class Encore {
      *
      * Encore.configureBabel(function(babelConfig) {
      *      // change the babelConfig
+     *      // if you use an external Babel configuration
+     *      // this callback will NOT be used. In this case
+     *      // you can pass null as the first parameter to
+     *      // still be able to use some of the options below
+     *      // without a warning.
      * }, {
      *      // set optional Encore-specific options, for instance:
      *
@@ -792,12 +797,16 @@ class Encore {
      *              A Webpack Condition passed to the JS/JSX rule that
      *              determines which files and folders should not be
      *              processed by Babel (https://webpack.js.org/configuration/module/#condition).
-     *              Cannot be used if the "include_node_modules" option is
+     *              Can be used even if you have an external Babel configuration
+     *              (a .babelrc file for instance)
+     *              Cannot be used if the "includeNodeModules" option is
      *              also set.
      *      * {string[]} includeNodeModules
      *              If set that option will include the given Node modules to
-     *              the files that are processed by Babel. Cannot be used if
-     *              the "exclude" option is also set.
+     *              the files that are processed by Babel.
+     *              Can be used even if you have an external Babel configuration
+     *              (a .babelrc file for instance).
+     *              Cannot be used if the "exclude" option is also set
      *      * {'usage'|'entry'|false} useBuiltIns (default=false)
      *              Set the "useBuiltIns" option of @babel/preset-env that changes
      *              how it handles polyfills (https://babeljs.io/docs/en/babel-preset-env#usebuiltins)
@@ -806,12 +815,15 @@ class Encore {
      *              by individual polyfills. Using it with 'usage' will try to
      *              automatically detect which polyfills are needed for each file and
      *              add them accordingly.
+     *              Cannot be used if you have an external Babel configuration (a .babelrc
+     *              file for instance). In this case you can set the option directly into
+     *              that configuration file.
      *      * {number|string} corejs (default=not set)
      *              Set the "corejs" option of @babel/preset-env.
      *              It should contain the version of core-js you added to your project
      *              if useBuiltIns isn't set to false.
      *
-     * @param {function} callback
+     * @param {function|null} callback
      * @param {object} encoreOptions
      * @returns {Encore}
      */

lib/WebpackConfig.js

@@ -337,15 +337,22 @@ class WebpackConfig {
     }
 
     configureBabel(callback, options = {}) {
-        if (typeof callback !== 'function') {
-            throw new Error('Argument 1 to configureBabel() must be a callback function.');
-        }
+        if (callback) {
+            if (typeof callback !== 'function') {
+                throw new Error('Argument 1 to configureBabel() must be a callback function or null.');
+            }
 
-        if (this.doesBabelRcFileExist()) {
-            throw new Error('configureBabel() cannot be called because your app already has Babel configuration (a `.babelrc` file, `.babelrc.js` file or `babel` key in `package.json`). Either put all of your Babel configuration in that file, or delete it and use this function.');
+            if (this.doesBabelRcFileExist()) {
+                logger.warning('The "callback" argument of configureBabel() will not be used because your app already provides an external Babel configuration (a ".babelrc" file, ".babelrc.js" file or "babel" key in "package.json"). Use null as a first argument to remove that warning.');
+            }
         }
 
-        this.babelConfigurationCallback = callback;
+        this.babelConfigurationCallback = callback || (() => {});
+
+        // Whitelist some options that can be used even if there
+        // is an external Babel config. The other ones won't be
+        // applied and a warning message will be displayed instead.
+        const allowedOptionsWithExternalConfig = ['includeNodeModules', 'exclude'];
 
         for (const optionKey of Object.keys(options)) {
             let normalizedOptionKey = optionKey;
@@ -354,6 +361,11 @@ class WebpackConfig {
                 normalizedOptionKey = 'includeNodeModules';
             }
 
+            if (this.doesBabelRcFileExist() && !allowedOptionsWithExternalConfig.includes(normalizedOptionKey)) {
+                logger.warning(`The "${normalizedOptionKey}" option of configureBabel() will not be used because your app already provides an external Babel configuration (a ".babelrc" file, ".babelrc.js" file or "babel" key in "package.json").`);
+                continue;
+            }
+
             if (normalizedOptionKey === 'includeNodeModules') {
                 if (Object.keys(options).includes('exclude')) {
                     throw new Error('"includeNodeModules" and "exclude" options can\'t be used together when calling configureBabel().');

test/WebpackConfig.js

@@ -520,6 +520,15 @@ describe('WebpackConfig object', () => {
     });
 
     describe('configureBabel', () => {
+        beforeEach(() => {
+            logger.reset();
+            logger.quiet();
+        });
+
+        afterEach(() => {
+            logger.quiet(false);
+        });
+
         it('Calling method sets it', () => {
             const config = createConfig();
             const testCallback = () => {};
@@ -581,13 +590,31 @@ describe('WebpackConfig object', () => {
             }).to.throw('must be a callback function');
         });
 
-        it('Calling when .babelrc is present throws an exception', () => {
+        it('Calling with a callback when .babelrc is present displays a warning', () => {
             const config = createConfig();
             config.runtimeConfig.babelRcFileExists = true;
+            config.configureBabel(() => {});
 
-            expect(() => {
-                config.configureBabel(() => {});
-            }).to.throw('configureBabel() cannot be called because your app already has Babel configuration');
+            const warnings = logger.getMessages().warning;
+            expect(warnings).to.have.lengthOf(1);
+            expect(warnings[0]).to.contain('your app already provides an external Babel configuration');
+        });
+
+        it('Calling with a whitelisted option when .babelrc is present works fine', () => {
+            const config = createConfig();
+            config.runtimeConfig.babelRcFileExists = true;
+            config.configureBabel(null, { includeNodeModules: ['foo'] });
+            expect(logger.getMessages().warning).to.be.empty;
+        });
+
+        it('Calling with a non-whitelisted option when .babelrc is present displays a warning', () => {
+            const config = createConfig();
+            config.runtimeConfig.babelRcFileExists = true;
+            config.configureBabel(null, { useBuiltIns: 'foo' });
+
+            const warnings = logger.getMessages().warning;
+            expect(warnings).to.have.lengthOf(1);
+            expect(warnings[0]).to.contain('your app already provides an external Babel configuration');
         });
 
         it('Pass invalid config', () => {