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

[WIP] Add react.config.js #7866

Closed
wants to merge 30 commits into from
Closed

[WIP] Add react.config.js #7866

wants to merge 30 commits into from

Conversation

yordis
Copy link

@yordis yordis commented Oct 23, 2019

A combination of webpack-chain and react.config.js would allow us to extend CRA without having to eject.

// react.config.js

module.exports = function() {
  return {
    webpackOverride(config) {
        // ... use webpack-chain API
    }
  }
}

I would love to have a conversation about this topic since we were asking for years about this and many people could take advantage of this feature.

Or

We could expose a webpack-chain config object as well as the existing plain object. The reasoning is that webpack-merge could do that much and webpack-chain configurations allow us to customize the config easier and more robust.

This will be useful for those who want to use CRA configuration since there is a lot of knowledge on the configuration but are forced to eject.

@andriijas
Copy link
Contributor

First of all, great work with the PR and taking your time!

This is a two edged sword kind of thing. CRA has always primary been the goto project for new comers to react. Meanwhile during the years many of us has started to build production apps with it and it works great. The upside is obvious, we get a community maintained stable webpack configuration without having to dig deep on our own.

The concern before has been that developers new to react and cra might copy and paste extended configuration from else where without really knowing whats going on and when it breaks it might cascade into reported issues here on github.

Over the years CRA has given in more and more to become a multi purpose build tool adding support for scss, typescript and more.

I think its valid to revisit this disucssion now that tools like webpack-chain

Custom templates are about to drop soon, maybe we should go a similar way to provide an API for custom webpack configuration recipes?

One thing to keep in mind though, adding such tool makes CRA more difficult to follow webpack releases. Not that CRA needs to have the latest webpack on day 0 but just to put it out there, using for example webpack-chain would need CRA to track a version of webpack that also webpack-chain supports.

@yordis
Copy link
Author

yordis commented Oct 23, 2019

Meanwhile during the years many of us has started to build production apps with it and it works great.

I wouldn't propose this if it weren't the case that I am forced to eject.

Legacy software, with legacy infrastructure, not that easy to decide to use CRA.

I had done it and I had to keep track of every release simply because an organization with 14 years of development can't use CRA a the moment (we are almost there).

I wish I could import the config from CRA (just an everyday object), and that's all.

But once you get into implementation details, you would understand how webpack-chain API will be beneficial, sometimes you need the especial API to be able to configure some options of a plugin or particular loader.

This is a two edged sword kind of thing.
The upside is obvious, we get a community maintained stable webpack configuration without having to dig deep on our own.

I would ask for some understand that some people do understand Webpack and these configurations.

One thing to keep in mind, though, adding such a tool makes CRA more challenging to follow webpack releases. Not that CRA needs to have the latest webpack on day 0 but just to put it out there, using, for example, webpack-chain would need CRA to track a version of webpack that also webpack-chain supports.

I don't know what to do about this one, and I can move webpack-chain into CRA. 😄

This incentive good collaboration in the ecosystem, we may have an issue, or we may not 🤷‍♂ , in any case, all it matters is the support form multiple parties.

I am not sure, you are 100% right.

Custom templates are about to drop soon; maybe we should go a similar way to provide an API for custom webpack configuration recipes?

What do you mean? I am not sure what templates are.

P.S:

I hope we can discuss this topic, for such a long-time CRA had been in a strong position of rejecting any proposal, and I don't think that it is fair for people that do have to understand Webpack and underline tools and can't merely use CRA.

I do understand, however, that CRA doesn't want to take responsibility for those who screwed the configurations.

Still, I genuinely believe that those that are trying to mess around internationally with the config are willing to accept this.

Just asking for some sympathy for those that are forced to eject 😢 My life for the last year had been watching releases and making sure that I keep my code in sync, I know how valuable these configurations are.

Especially that I know that most people reject for simple things like adding support for styled-components or it used to be because of TypeScript and so on.

Maybe enabling the feature will allow people to have a better experience on their end without having to ask CRA to include everything that is cool out there.

Hopefully, we can have a conversation about it, I will try to help as much as I can, just want to be proactive.

@andriijas
Copy link
Contributor

andriijas commented Oct 23, 2019

What do you mean? I am not sure what templates are.

Templates is an upcoming feature to allow bootstraping new cra apps with custom templates to get some foundation to start build upon. Imagine there's going to be community maintained templates that includes design systems, router etc.
#7716

An alternative to your proposal could be to allow for custom packages. For examplecra-config-styled-components or similar.

P.S:

I hope we can discuss this topic, for such a long-time CRA had been in a strong position of rejecting any proposal, and I don't think that it is fair for people that do have to understand Webpack and underline tools and can't merely use CRA.

Just to balance, over 2700 PRs has been merged so its not fair to say "any propsal gets rejected". My reply was meant to start discussing the topic, because yeah custom webpack config has been proposed before so I just gave some background. Personally I like your proposal and my reply wasn't meant as criticism, more like initial neutral.

Hopefully, we can have a conversation about it, I will try to help as much as I can, just want to be proactive.

This is a conversation about it, just have patience and stick around to hear what other thinks about it. Again, good job with the initial work.

@yordis
Copy link
Author

yordis commented Oct 23, 2019

Oooh the way I see it, templates are not quite the same thou, but I am happy to see templates becoming part of CRA actually ❤️

Just to balance, over 2700 PRs has been merged so its not fair to say "any propsal gets rejected".

Well I had been following this topic about extending CRA for years, and every single time the conversation came out, we get nowhere and gets rejected (I am talking about having some control over the configs btw)

This is why I want to do the work and then talk about it 😄

Personally I like your proposal and my reply wasn't meant as criticism, more like initial neutral.

Oh kool, I have all good feelings about your replies so don't worry.

@yordis
Copy link
Author

yordis commented Oct 25, 2019

@andriijas I need your support,

At this point, we need to decide whatever direction we wanna take,

  • Add react.config.js with webpackOverride
  • Add webpack-chain to people can take advantage of the underline webpack config and be able to customize the configuration since webpack-merge will no cut it.
  • Reject either proposal

@yordis yordis marked this pull request as ready for review October 25, 2019 03:03
@mrmckeb
Copy link
Contributor

mrmckeb commented Oct 25, 2019

Hi @yordis, we like the idea of a config and have been talking about if for a while, but the concern is always the complexity it adds - for users, and for us as maintainers too.

The webpackOverride feature would be a much harder sell, it's something that has been rejected numerous times before... @andriijas has summed all of this up nicely above.

We have also talked about the idea of allowing developers to set different scripts versions in templates too, but that also introduces risk.

It would be good to involve @iansu and @ianschmitz in this conversation.

@yordis
Copy link
Author

yordis commented Oct 25, 2019

Hi @yordis, we like the idea of a config and have been talking about if for a while, but the concern is always the complexity it adds - for users, and for us as maintainers too.

@mrmckeb I am more than happy to write an article explaining before and after, and videos for you if that is the case. I am not sure what to do about that one other than be here to help you as much as I can.

The webpackOverride feature would be a much harder sell, it's something that has been rejected numerous times before... @andriijas has summed all of this up nicely above.

I dream of the day where we remove // @remove-on-eject-begin and we make the tool 0Config always, but friendly to extend.

As I said before, I am more than happy to document every line, how it works for maintainers and make sure that I do my part on explaining every piece of the software, but you have the final say regardless.

But if at least I get the config I guess that is beyond enough personally speaking.

@mrmckeb
Copy link
Contributor

mrmckeb commented Oct 25, 2019

I am more than happy to write an article explaining before and after, and videos for you if that is the case.

This is not about the documentation of the feature. It's about the long-term impact. We already have trouble responding to all issues and PRs as it is, which is not great.

@andriijas
Copy link
Contributor

@yordis This PR and the direction will be discussed. You have done excellent work so far so just enjoy the weekend and have some patience :) thanks!

@johnnyreilly
Copy link
Contributor

johnnyreilly commented Oct 30, 2019

I was pointed to this issue by the wonderful @ianschmitz: https://www.twitter.com/schmitz_ian/status/1189296593654165507

I just wanted to chime in and say how a mechanism along these lines would be delightful to me and I suspect many others. Let me explain my own situation.

CRA is 95% perfect for my needs. However, I eject for these reasons:

  1. Because I want more control over workbox (that actually might get resolved in future by Added options to allow for overrides to workbox-webpack-plugin #5369 )

  2. I want to swap babel-loader for ts-loader. It's worth noting that I maintain the ts-loader project and there's some features in there that aren't in babel-loader which I'd like to use.

  3. Dropping the eslint-loader usage as fork-ts-checker-webpack-plugin (which CRA already uses to power TypeScript integration) supports this out of the box with nice formatting possibilities; you can format your TypeScript errors and ESLints in the same manner. (Also worth noting, I maintain that plugin as well)

I'd love it if a mechanism existed to allow to swap out the pieces of webpack config if you needed. I'd be more than happy if that was available on the same basis as eject is. That is to say: if you use it you can't raise support tickets around it; much as you can't with ejected configs. Buyer beware and all.

As it is, I find myself ejecting and tweaking and then, when new releases of CRA come out, creating new projects using it, ejecting that and migrating the changes piecemeal into my config. As painful as it sounds.

@andriijas
Copy link
Contributor

This is not to provoke, just need to get a better understanding - why isn't neutrinojs, razzle, next or gatsby an option for those rare occasions you need to eject?

@yordis What do you think about rather than exposing and having the webpack extensible via config file, rather refactor the internal webpack config in CRA to smaller chunks ("lego blocks"). Which can be independently tested and glued together to a complete config? And as a end user you could glue it together with your own additions (but probably still via a tool like react-app-rewire)

@yordis
Copy link
Author

yordis commented Oct 30, 2019

@yordis What do you think about rather than exposing and having the webpack extensible via config file, rather refactor the internal webpack config in CRA to smaller chunks ("lego blocks"). Which can be independently tested and glued together to a complete config?

This is what I actually have in some internal tool, I will recommend you to do so since testing chunks of configs is easier than a massive config.

Worth saying, you will trade-off discoverability since you don't see the full config, but you gain in maintainability.

But, I definitely recommend this, and I had done it and I know how we could extract the pieces today, just let me know if that is the direction that CRA wants.

I would create another package just for config related stuff, and react-scripts is the CLI + configs.

@andriijas
Copy link
Contributor

Related #3775

@andriijas
Copy link
Contributor

We are currently exploring ways to extend create-react-app with a config file rather than relying on environment variables only. We do appreciate you taking time for these efforts. Full webpack override is not something create-react-app is looking to support long term at this moment or in the near future. Sorry for any inconvenience!

@andriijas andriijas closed this Nov 13, 2019
@yordis yordis deleted the yordis/react-config branch November 13, 2019 20:21
@pavinthan
Copy link
Contributor

pavinthan commented Nov 14, 2019

@andriijas what about add new option in eject --config script that will only create react.config.js project root after that you need to worry about maintainene because It's already mention as

If your project needs more customization, you can "eject" and customize it, but then you will need to maintain this configuration.

we need this future because we don't want all configrations files to add a webpack plugin or something and It's simply upgradeable.

sorry if anything wrong and please advice me.

Thanks.

@andriijas
Copy link
Contributor

@pavinthan @yordis what would help a lot is to get a better understanding what kind of custom configuration is actually needed and we can maybe find another way. There are great tools like Neutrino js, razzle and even next js allows custom webpack config.

Everyone acknowledge that there is a small percentage of all users that demands full webpack overrides. The cost (in terms of time) in maintaining is unfortunatly too high.

@yordis
Copy link
Author

yordis commented Nov 14, 2019

@andriijas Honestly, based on your response, I feel profoundly offended, and your answer feels like you ignored all I said, or you genuinely don't understand; I will take the second one.

I don't believe that Mob mentality like you said "small percentage of all users" is something that we should encourage.

And we are not a small percentage, this is far from reality in the first place, you don't know what you don't know.

History lesson:

  • TypeScript support took forever to add
  • You have to eject because of GraphQL support, still
  • Styled-Components and Babel had to work together for Babel macros concept so you didn't have to mess around with configs because the Add missing semver dependency #1 community was forced to eject from a fantastic project, still
  • People keep asking for some configs because they are being forced to change, go and look up your existing and closed issues, and many of them could be simply solved without eject in their end.

I am sorry, you had missed history entirely on what happened regardless of this topic.

I hope you learn how to enable the community how the Vue community does it, as Angular does it, like almost every ecosystem out there that actually listen to their people.

@mrmckeb
Copy link
Contributor

mrmckeb commented Nov 17, 2019

@yordis, @andriijas is a respected member of our team, and the views he shares represent those of the entire team - there is no offense meant and none should be taken. Please speak to the team with respect. We're all volunteers, even though this is a Facebook project.

There's not a single member of the core team that doesn't want to enable more customisation and open up more configuration, and we will continue to do that progressively.

I'm not sure why you need to eject for GraphQL support, and I think the Babel macros solution works well. I agree that TypeScript took too long - but it is supported now, and we're continuing to improve support in that area.

@yordis
Copy link
Author

yordis commented Nov 17, 2019

@mrmckeb I didn't offend anyone.

I do respect his opinion, as I expressed previously in the thread I wanted to start the discussion knowing that you will reject it unless you want to ignore the full conversation.

  1. I answered multiple times, included privately on Twitter what the use cases are.
  2. His reply in support of Mob decisions is not something I will support. Maybe rephrase the sentence to be a more welcoming and inclusive community, instead of using "minority" as a way to win an argument.
  3. The fact is that he missed history on this topic and ignore all the issues opened and PR closed about it.

That being said, the work was done, and once again was rejected, totally fine with it, I didn't need an explanation for it honestly.

Anyway, I don't think anyone is getting anything out of the conversation.

@facebook facebook locked as too heated and limited conversation to collaborators Nov 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants