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

Use standard .*rc files #410

Closed
EnoahNetzach opened this issue Aug 9, 2016 · 19 comments · Fixed by #705
Closed

Use standard .*rc files #410

EnoahNetzach opened this issue Aug 9, 2016 · 19 comments · Fixed by #705
Milestone

Comments

@EnoahNetzach
Copy link
Contributor

EnoahNetzach commented Aug 9, 2016

After ejecting it is not immediately clear how to add, for example, new scripts that use babel-cli.

The configuration I'm used to is in a .babelrc file which is loaded automatically by babel, right now I'm not sure how to replicate this standard behaviour with babel.dev.js.

@EnoahNetzach
Copy link
Contributor Author

EnoahNetzach commented Aug 9, 2016

The same goes for .eslintrc

@denkristoffer
Copy link
Contributor

I agree, that seems like expectable behaviour to me as well.

@gaearon
Copy link
Contributor

gaearon commented Aug 9, 2016

It's not trivial to implement in a way that won't break in the future. Do you have implementation suggestions for this?

@gaearon
Copy link
Contributor

gaearon commented Aug 9, 2016

(If we do this we also can't guarantee that ejecting is safe. For example if there is a broken babelrc in a parent folder, it would not be taken into account after eject, but with your suggestion, it could start throwing).

@jamesblight
Copy link
Contributor

I ran into an issue trying to use the source-preview atom package https://atom.io/packages/source-preview. It expects to find a .babelrc file and won't be able to generate the source if plugins/presets are required. This may be a problem for other packages that expect a .babelrc.

Setting the babel extends config option in package.json to extend the internal react-scripts config would work if there was a .babelrc.

It would be even better if the babel extends option accepted a JavaScript file like eslintConfig extends does. Then, your package.json could simply include:
"babel": { "extends": "./node_modules/react-scripts/config/babel.dev.js" }

And you could add your own config on top of that.

@gaearon
Copy link
Contributor

gaearon commented Aug 25, 2016

If you can figure out a way to make this project use .babelrc internally and then eject it, please send a PR. This is not currently a high priority for us but we’ll be happy to review that effort.

@jamesblight
Copy link
Contributor

I'm not across the issues caused by using .babelrc internally, but I can have a look.

@hillscottc
Copy link

The MobX docs tell me I must "use the transform plugin transform-decorators-legacy and make sure it is first in the plugins list", in order for the decorators to work. How can I do that when there is no .babelrc? Is there some workaround?

@gaearon
Copy link
Contributor

gaearon commented Sep 1, 2016

@hillscottc

You can’t use decorators with this setup currently.
I explained why here: #411 (comment).

However you don’t need decorators for MobX. I wish its docs put more emphasis on this.
See here how to use it without decorators: #214.

cc @mweststrate, this confusion came up 5 times, maybe worth making it more apparent that they’re not necessary?

@mweststrate
Copy link

mweststrate commented Sep 1, 2016

@gaearon yep, had on my todo list where to investigate where the confusion comes from, to make it more clear that is optional.

@hillscottc where would you have expected that for example? The readme currently starts with an example how to do things both with and without decorators. Was that clear?

For people stumbling into this in the future, here is a small create-react-app based repo with MobX, without decorators:

https://github.com/mobxjs/create-react-app-mobx

Edit see also: http://mobxjs.github.io/mobx/best/decorators.html

@gaearon
Copy link
Contributor

gaearon commented Sep 1, 2016

Thanks for taking the time to do that!

@jamesblight
Copy link
Contributor

jamesblight commented Sep 2, 2016

@gaearon is the main issue with using .babelrc internally the potential conflict with an existing .babelrc on eject?

@gaearon
Copy link
Contributor

gaearon commented Sep 2, 2016

The main issue is that nobody did this :-). If you submit a PR we can start looking at individual problems.

@gaearon
Copy link
Contributor

gaearon commented Sep 2, 2016

(I'm not talking about supporting decorators or merging configs which is unrelated to this thread. I meant outputting RC files on eject.)

@EnoahNetzach
Copy link
Contributor Author

EnoahNetzach commented Sep 7, 2016

I have an almost working hacked together script that does it, in a couple of days I should be able to open a decent PR.

It tries to merge the (optionally) existing .babelrc with a version extracted from the .js configs.
Not sure what to do when it fails (e.g. for config clashes):

  • fail with a list of clashing configs
  • output a .babelrc.fail file
  • overwrite clashes
  • ask interactively what to do

Ideas?

@gaearon
Copy link
Contributor

gaearon commented Sep 7, 2016

I think we won't support any attempts to merge configs. It is just impossible to do safely because of how Babel is implemented (some plugins can only be used together in a specific order).

@gaearon
Copy link
Contributor

gaearon commented Sep 22, 2016

This is implemented in #705 and will be released in 0.5.0.

@gaearon
Copy link
Contributor

gaearon commented Sep 23, 2016

0.5.0 is out with this.
Please read the release notes for migration instructions.

feiqitian pushed a commit to feiqitian/create-react-app that referenced this issue Oct 25, 2016
@tuchk4
Copy link
Contributor

tuchk4 commented Jan 6, 2017

Suggested implementation for custom babel configuration #1357

@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants