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

Add support for .babelrc.js files #4892

Merged
merged 3 commits into from Mar 7, 2017

Conversation

@kaicataldo
Member

kaicataldo commented Nov 24, 2016

Q A
New feature? yes
Tests added/pass? yes
Fixed tickets Fixes #4630
License MIT
Doc PR
Dependency Changes

Took a stab at adding support for .babelrc.js configuration files. Backfilled a test for when .babelrc contains malformed JSON. Let me know what you think!

@codecov-io

This comment has been minimized.

codecov-io commented Nov 24, 2016

Codecov Report

Merging #4892 into 7.0 will increase coverage by 0.12%.
The diff coverage is 98.68%.

@@            Coverage Diff            @@
##              7.0   #4892      +/-   ##
=========================================
+ Coverage   85.27%   85.4%   +0.12%     
=========================================
  Files         203     203              
  Lines        9531    9532       +1     
  Branches     2690    2702      +12     
=========================================
+ Hits         8128    8141      +13     
+ Misses        915     904      -11     
+ Partials      488     487       -1
Impacted Files Coverage Δ
packages/babel-generator/src/index.js 88% <100%> (-0.24%)
.../transformation/file/options/build-config-chain.js 93.93% <100%> (+4.06%)
packages/babel-generator/src/printer.js 98.4% <100%> (ø)
packages/babel-generator/src/node/parentheses.js 97% <97.36%> (+5.4%)
...bel-plugin-transform-es2015-classes/src/vanilla.js 90.17% <0%> (+0.42%)
packages/babel-traverse/src/path/context.js 85.34% <0%> (+0.86%)
packages/babel-traverse/src/visitors.js 86.66% <0%> (+0.95%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39eca84...d054cd0. Read the comment docs.

throw err;
if (path.extname(loc) === ".js") {
try {
options = require(loc);

This comment has been minimized.

@DrewML

DrewML Nov 27, 2016

Member

Might be worth checking for a .default prop on the returned object. Someone could be using babel-register and have export default {} for .babelrc.js

This comment has been minimized.

@kaicataldo

kaicataldo Nov 27, 2016

Member

Hadn't considered that - thanks. Updated!

@DrewML

This comment has been minimized.

Member

DrewML commented Dec 5, 2016

Thinking about this more, I'm personally a bit hesitant to support dynamic config.

We've discussed in the past having a babel init command that can generate a config for you. I think it would be huge if you could use the same tooling to allow editing your existing configuration (adding new plugins, configuring preset options, etc). If we allow dynamic configuration, we won't be able to programmatically modify the config, and then serialize it back to disk.

@kaicataldo

This comment has been minimized.

Member

kaicataldo commented Dec 5, 2016

@DrewML Do you think it would be harmful to simply not support JS configs with that new feature? I ask because I feel like those who prefer dynamic JS configs would most likely not be interested in a CLI tool that manages it for you anyway.

We already do have the env option which gives us a way to share a single config file among scripts that might require different setups, but it ends up being verbose and cumbersome if there are shared configs because order for plugins matters and it ends up forcing you to duplicate plugin entries. I know I'd personally prefer to have control over that.

Didn't know about the discussion about a babel init command. We have a similar command for ESLint (though it doesn't edit existing configs), but it's mainly geared towards a) giving those who are new to the tool/ecosystem a quick way to get started using the tool and b) give those who aren't interested in managing their own configuration a way to get started using a pre-existing preset. Having what is essentially a babel plugin manager sounds interesting.

@ORESoftware

This comment has been minimized.

ORESoftware commented Dec 8, 2016

@DrewML why not just call JSON.parse(JSON.stringify(require('./whatever/.babelrc.js')) early on in program lifecycle, if it fails, then tell the user what's wrong, they can fix it. It's not that hard for users to ensure that their config is serializable.

@DrewML

This comment has been minimized.

Member

DrewML commented Dec 8, 2016

@DrewML why not just call JSON.parse(JSON.stringify(require('./whatever/.babelrc.js')) early on in program lifecycle, if it fails, then tell the user what's wrong, they can fix it. It's not that hard for users to ensure that their config is serializable.

@ORESoftware That doesn't solve the use-case I am referencing. That would verify that the value returned from evaluating the file is serializable. If someone has a JS configuration, we can't make modifications to the parsed value, and then blow away their JS and replace it with JSON.

@ORESoftware

This comment has been minimized.

ORESoftware commented Dec 8, 2016

@DrewML yeah but when would you overwrite the user provided config, and write it back to the filesystem? Probably never, right? Or are you? Their of course are projects that do this, but it doesn't get written back to the developer's source code. I do see it as a slightly risky move.

@DrewML

This comment has been minimized.

Member

DrewML commented Dec 8, 2016

@ORESoftware:

We've discussed in the past having a babel init command that can generate a config for you. I think it would be huge if you could use the same tooling to allow editing your existing configuration (adding new plugins, configuring preset options, etc). If we allow dynamic configuration, we won't be able to programmatically modify the config, and then serialize it back to disk.

It is an idea relating to a bigger discussion we've had regarding how to make editing the .babelrc less error prone

@ORESoftware

This comment has been minimized.

ORESoftware commented Dec 8, 2016

@DrewML this one is easy. Have both .babelrc and .babelrc.js. The latter is optional, the former required. The latter requires/imports the former and can override things. Babel edits the former.

might be kind of annoying to implement though, and there might be some gotchas in there.

In general, I like dynamic config and using .js instead of .json, that's just my preference. I like Webpacks use of .js config and suman also will use .js config.

@DrewML

This comment has been minimized.

Member

DrewML commented Dec 8, 2016

I don't think it's the greatest solution to require anyone that wants to write their config in JS maintain 2 separate files for configuration. I'm much prefer @kaicataldo's solution of just not having any tooling like that support custom .babelrc.js files.

@ORESoftware

This comment has been minimized.

ORESoftware commented Dec 8, 2016

@DrewML sure, I don't have anything at stake in this particular convo, just adding my 2 cents. I am more interested in the issue regarding dynamically specifying the exact path to .babelrc

@DrewML

This comment has been minimized.

Member

DrewML commented Dec 8, 2016

Cool! No worries.

@DrewML

This comment has been minimized.

Member

DrewML commented Dec 9, 2016

@DrewML Do you think it would be harmful to simply not support JS configs with that new feature? I ask because I feel like those who prefer dynamic JS configs would most likely not be interested in a CLI tool that manages it for you anyway.

@kaicataldo You do make a good point. It's also, in no way, set in stone that we'd be implementing CLI editing of existing config. Just an idea I had floated at one point.

@ForsakenHarmony

This comment has been minimized.

ForsakenHarmony commented Dec 11, 2016

@DrewML so you put an idea above something that would already work?

@DrewML

This comment has been minimized.

Member

DrewML commented Dec 15, 2016

@ForsakenHarmony Nothing has been put above anything. This is a discussion.

@ForsakenHarmony

This comment has been minimized.

ForsakenHarmony commented Dec 15, 2016

@kenzierocks

This comment has been minimized.

kenzierocks commented Dec 15, 2016

Personally I would leave .babelrc as it is, and any CLI tool will edit that. If it's missing and a .babelrc.js exists, you don't do anything and warn the user. If there is not .babelrc(.js), creating .babelrc is fine. This would allow people to choose CLI or JS and prevent them from doing something weird.

If both files are present, it might be best to just warn the user, or it could be an optional feature that passes the contents of .babelrc into a function in .babelrc.js.

@DrewML

This comment has been minimized.

Member

DrewML commented Dec 18, 2016

@hzoo @loganfsmyth @danez @Kovensky @existentialism Anyone else have any opinions on this?

@aymericbeaumet

This comment has been minimized.

aymericbeaumet commented Dec 25, 2016

Not sure if this should be posted in this PR or on the corresponding issues, so i'm just dropping it here.

At first, one might say most users will still use .babelrc vs .babelrc.js files to configure their compilation and that only advanced ones will benefit from it.

That said, it also allows for cleaner ways to configure the compilation, for example NODE_ENV based specific options. All can benefit from that.

Supporting the .js format will also allow for more creativity in the babel community (I have in mind it would allow the end-user to extend a plugin exporting a babel .js configuration via its main).

Regarding the loading, IMO the .js file should have the higher priority, and if it loaded, the resolution should stop. That way experienced user could also leverage the babel init CLI once it lands, and inherit the generated file in their dynamic configuration, e.g. (on mobile, sorry about typos if any):

const generated = JSON.parse(require('fs').readFileSync('./.babelrc'));

module.exports = Object.assign(generated, {
  // dynamic stuff
});

With that in mind I don't see how a tool enabling the user to generate JSON5 .babelrc files will create backward compatibility issues, while it benefits all the users.

@kaicataldo

This comment has been minimized.

Member

kaicataldo commented Dec 31, 2016

Any other thoughts on what we want to do about this?

@xtuc

This comment has been minimized.

Member

xtuc commented Dec 31, 2016

I don't have a strong opinion on this but I would prefer an API in Babel to allow to run it with a given configuration object. This would allow user to implement whatever configuration they want (#4980).

I aggree with @DrewML, users will have more flexebility but will also be a lot more error prone.

I think #4970 (Specifying a different file (different filename, same file type) for config other than .babelrc) could solve this and why not #5039 (Add support for DOTFILES_PATH environment variable).

To be clear, I think dynamic configuration should be done using Babel's API.

@kaicataldo

This comment has been minimized.

Member

kaicataldo commented Jan 3, 2017

@hzoo Looks like you thumbed the original comment at the top, but what are your thoughts? I'm still pretty new to the project - so I might be missing something here - but supporting different file types in ESLint has been pretty unproblematic.

@jonbretman

This comment has been minimized.

jonbretman commented Jan 5, 2017

I think JS based config files are extremely useful and also quite common e.g. ESLint, Webpack, and PostCSS all support JS config files.

In my experience it is quite common to change the value of some config option based on an environment variable, or to want to share some common value between different config files e.g. between babel and webpack.

Would love to see this land.

assert.deepEqual(chain, expected);
});
it("js-json-config - should use .babelrc if both are present", function () {

This comment has been minimized.

@hzoo

hzoo Jan 5, 2017

Member

Ok I think we should be erroring if there's both a .babelrc and a .babelrc.js since there doesn't need to be both at the same level

This comment has been minimized.

@kaicataldo

kaicataldo Jan 5, 2017

Member

That does feel like better behavior than defaulting to one (currently .babelrc)

@hzoo hzoo merged commit 02473a7 into babel:7.0 Mar 7, 2017

3 checks passed

codecov/patch 98.68% of diff hit (target 85.27%)
Details
codecov/project 85.4% (+0.12%) compared to 39eca84
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@kaicataldo kaicataldo deleted the kaicataldo:babelrcjs branch Mar 7, 2017

@hzoo

This comment has been minimized.

Member

hzoo commented Mar 7, 2017

Nice work on this one!! 🎉

@kaicataldo

This comment has been minimized.

Member

kaicataldo commented Mar 7, 2017

Appreciate all the input on this one ❤️

@kentcdodds

This comment has been minimized.

Member

kentcdodds commented Mar 7, 2017

Hooray! 🎉

@fatfisz

This comment has been minimized.

fatfisz commented Mar 9, 2017

Would you be so kind as to backport this to 6.x? Otherwise what is the ETA on 7.x stable release? This feature would help me preserve my sanity when dealing with a particular preset 😄

this.errorMultipleConfigs(arr.pop(), config);
}
arr.push(config);

This comment has been minimized.

@loganfsmyth

loganfsmyth Mar 27, 2017

Member

Looking into #5543 (comment), it looks like there is a regression here. This should be

if (configAdded) arr.push(config);

otherwise it will always stop at the first package.json it encounters, even if it doesn't contain a "babel" key. While I actually like that behavior, we should probably do it on purpose instead of on accident :P Would you be up for PRing a fix, or should I?

This comment has been minimized.

@kaicataldo

kaicataldo Mar 27, 2017

Member

I'm happy to make a PR - thanks for the heads up!

@jhen0409 jhen0409 referenced this pull request Mar 30, 2017

Merged

Initial Webpack DLL Support #860

1 of 1 task complete
@hulkish

This comment has been minimized.

Contributor

hulkish commented Mar 30, 2017

has this been released? I'm finding that the babel compiler doesnt notice my .babelrc.js at all

@hzoo

This comment has been minimized.

Member

hzoo commented Mar 30, 2017

you'll need to use 7.0 (alpha), or wait

@MrLoh

This comment has been minimized.

MrLoh commented Apr 9, 2017

@hulkish I saw you mentioned in #4630 that this would be useful for isomorphic apps. I'm having the same issue using babel and babel-node in the same directory. Did you get this working with .babelrc.js, would you mind sharing your setup.

@hulkish

This comment has been minimized.

Contributor

hulkish commented Apr 21, 2017

@MrLoh I ended up being forced to sidestep babelrc by directly configuring babel via babel-loader and using the babelrc: false option

@fatfisz

This comment has been minimized.

fatfisz commented May 28, 2017

Possible workaround (works for me):

{
  "presets": [
    "./.babelrc.js"
  ]
}

Just create the .babelrc.js file and export an object using module.exports.

Afterwards you can just remove .babelrc and babel 7 will work with .babelrc.js.

@kentcdodds

This comment has been minimized.

Member

kentcdodds commented May 28, 2017

This is awesome! Any reason I shouldn't suggest to people that they do this?

@hzoo

This comment has been minimized.

Member

hzoo commented May 29, 2017

Not really, I probably would of suggested it earlier really 😄 , pretty funny though

@fatfisz

This comment has been minimized.

fatfisz commented May 29, 2017

In some cases there may be problems with caching when changing the contents of .babelrc.js, e.g. I had some probs when working with Next.js. I don't really know how to turn caching off atm, so I have to manually remove the cache each time 😛

But that's only when changing the config, no other problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment