Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to call configureBabel with an external Babel configuration #544

Merged
merged 3 commits into from Mar 29, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 16 additions & 4 deletions index.js
Expand Up @@ -785,6 +785,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 false 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:
*
Expand All @@ -806,12 +811,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='entry')
* Set the "useBuiltIns" option of @babel/preset-env that changes
* how it handles polyfills (https://babeljs.io/docs/en/babel-preset-env#usebuiltins)
Expand All @@ -820,8 +829,11 @@ 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.
*
* @param {function} callback
* @param {function|false} callback
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest documenting as function|null rather than function|false. Using a boolean to represent the absence of value does not make much sense.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, the error message even says Argument 1 to configureBabel() must be a callback function or null.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I initially went with null but for some reasons switched to false (and didn't update that message).

I think my reasoning was something like "that parameter is disabled and won't be used", but now that you point it out I agree that it makes more sense to put null.

* @param {object} encoreOptions
* @returns {Encore}
*/
Expand Down
24 changes: 18 additions & 6 deletions lib/WebpackConfig.js
Expand Up @@ -329,15 +329,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;
Expand All @@ -346,6 +353,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().');
Expand Down
35 changes: 31 additions & 4 deletions test/WebpackConfig.js
Expand Up @@ -538,6 +538,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 = () => {};
Expand Down Expand Up @@ -599,13 +608,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(false, { 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(false, { 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', () => {
Expand Down