facebook / create-react-app

Set up a modern web app by running one command.
https://create-react-app.dev
MIT License
102.52k stars 26.77k forks source link

Discussing Advanced Configuration Options #6303

Open harrysolovay opened 5 years ago

harrysolovay commented 5 years ago

I believe there's a great need for CRA to support advanced configuration. React-app-rewired was the first to let users patch their CRA configs. Then CRA changed (2.0+ and the merging of dev & prod builds). So... I made Rescripts, which similarly patches react-scripts. These tools work... but they're subject to break whenever CRA changes. Not to mention, they operate via node's require cache and can lag (big time).

I agree with everything that's been said about prioritizing simplicity for beginners. Nevertheless, there's a missed opportunity to provide better DX for experienced devs (who should also be a priority). No experienced devs want to miss out on updates to react-scripts... but they also don't want to miss out on a lot of the beautiful tooling that hasn't made its way into CRA.

CRA is––for many––the go-to starter (~1.75 million downloads this week alone according to react-scripts npm stats). I feel a little guilty as I ask this question (for what is surely the trillionth time maintainers have heard it), but why can't there be a built-in mechanism––that isn't ejection––for overriding the config?

I'd be more than happy to build this into CRA as an optional back door for manipulating the Webpack config. Something simple, where users can define a function which takes in, operates on, and returns the config object. This way, there's no need to use a 3rd party patching tool, nor would there be a need to eject in most cases.

Beginner devs stay happy. Other devs become overjoyed. Thoughts?

mrmckeb commented 5 years ago

I agree that we need to start doing this better, and we're talking about it already ;) There is of course the option of forking, or as you said using things like rescripts, but we do need to keep allowing different people to make CRA what they need it to be - without breaking behaviours for others, and without making it maintainable for us.

I'd be interested in hearing some of your thoughts on how to do this? One thought was to have a config in package.json.

patricklafrance commented 5 years ago

As the creator of craco I cannot agree more with you.

Building craco wasn't something I wanted to do but it was out of necessity because no configuration options were available.

CRA offer a lot of advantages that my organization and I want to benefits from. You guys have an incredible expertise and we want to be able to leverage that.

At the same time, I cannot tell my team that we cannot use a widely adopted features of PostCSS because the CRA team doesn't enable it by default.

I understand the reasons why you didn't want to offer configurations but I feel like it's now something that the community want when I look at the number of NPM downloads for hackish package to configure CRA.

Patrick

patricklafrance commented 5 years ago

@mrmckeb I do like a lot how gatsby is doing it.

https://www.gatsbyjs.org/docs/add-custom-webpack-config/

I personnally feel like overcrowding the package.json file is not a good idea and a cra.config.js file would be a better way to do it.

mrmckeb commented 5 years ago

I think we'd not want to allow custom webpack configs, but instead have more options/settings available.

Personally, I agree that package.json can be overcrowded, but I also resent an overcrowded root directory... neither option is perfect.

harrysolovay commented 5 years ago

@mrmckeb & @patricklafrance thank you for your input.

I don't know how I feel about the idea of placing the configuration in the package.json. I enabled this in Rescripts, and I kind of regret it. The package.json has become too much of a hub for configuration, when its primary focus should be outlining key resources and scripts. I think a cleaner solution would be to optionally accept a config path in the CLI. The provided path could be relative to the project or system root, from a Git repo, or––if none of those are resolved––the NPM registry.

react-scripts start -c overrides.js would do the trick.

Then, in overrides.js...

// ESModules have been enabled for continuity between developing the app & its environment;
// How to do this though?... @babel/register v spawning a new fork with experimental
// modules enabled, and sending the updated config back to parent process...
// anyone have more ideas on speedy ESM support?

// takes in the Webpack config
export default originalConfig => {
  const newConfig = {...originalConfig}
  // manipulate newConfig
  return newConfig
}

Users can reference the env to differentiate between overrides for dev vs. prod.

We also need the ability to manipulate the Jest config. Similarly to the -c flag, a -j flag should do.

Some other things we need to discuss... even though they've been discussed far too much and rejected every time:

I see this in my package.json whenever I bootstrap a project with CRA:

...
"eslintConfig": {
  "extends": "react-app"
},
...

This is misleading. If I change this, ESLint behaves exactly the same.

I'm assuming this field is there just to spark IDE recognition? I understand wanting to shield beginners from linting... but I'd still recommend enforcing a root-level ESLint config (one which is malleable and gets read by the default Webpack config). This shouldn't be terribly intimidating for beginners.

Scanning for an optional root-level Babel config wouldn't be horrible either. With ESLint and Babel configurable through their standard conventions, devs wouldn't have to install a 3rd party override, nor would they have to sift through react-scripts and find the exact subpath to override (which is subject to break with updates to the config). I think this would cover a lot of developers' needs.

Please let me know what y'all think 🥳

harrysolovay commented 5 years ago

How do you feel about scanning for an overrides.js file in the root, and leaving the package.json and its scripts alone?

[EDIT]: ^ definitely not haha. Referencing Git & NPM overrides directly is a must.

patricklafrance commented 5 years ago

I understand @mrmckeb

If you want to offer a more controlled experience for configuration, I do like how the Vue CLI is doing it:

https://cli.vuejs.org/config/#pages

This is what inspired the style of configuration of craco.

patricklafrance commented 5 years ago

@mrmckeb a few arguments for not using package.json:

1- Your configuration file can export an object or a function and do imports.

[EDIT]: This is quite important for scenarios that requires a different configuration based on the current ENV.

2- By it's nature package.json file tends to change a lot because of NPM packages being added / removed for the project. Adding additional configuration to this file increase the possibility of merge conflicts.

Usually in a team, there are only few who really know and understand about the packages (and versions) being used and most of the dev doesn't want to mess around packages conflicts.

mrmckeb commented 5 years ago

I understand what is being asked for here. But there are two groups of people that I think can be served by different solutions. We need to decide what's best.

Some people would benefit by a simple config, turning things on or off, etc. This is something I think the team would be very open to.

Others need to extend webpack, and that's a far more complicated situation that I think would be harder to get into CRA right now - as it creates a lot more complexity in support, and slows down progress (as we need to test for more variables).

harrysolovay commented 5 years ago

@mrmckeb all that we would need is a way to specify an innocuous function: takes in, modifies and returns the updated Webpack config. This would amount to a single additional test. And as the result of this minor effort, thousands of devs could more comfortably use CRA as the starting point for their projects. I'm more than happy to work on this feature and demonstrate just how little of a footprint it would have.

There's so much incredible tooling out there... the fact that using these tools isn't a first-class feature is aggravating. Ejection, custom fork maintenance, and 3rd party patching amount to a bad DX. I understand your hesitation, but I'd love to find a solution that we agree on. After all, I think this might just be the greatest weakness of CRA.

mrmckeb commented 5 years ago

Yes, I understand. And I completely agree that we need to find better ways than forking (and definitely ejecting).

In the past this has been rejected though. And I think that a first step here would be enabling options, and then enabling full-control. The benefit of options is we can also offer support for those - whereas we can't for a full-control setup.

I think we can get something like that in quite soon, but I don't think we can get the full-control option in without a lot more buy-in from the greater team.

harrysolovay commented 5 years ago

I understand the rationale behind imposing limits (and agree with it). But this "back door" to manipulate the Webpack config is actually in the best interest of maintainability. Adding too many options will impede future changes to CRA. It will result in the need for more tests, documentation, and discussion. Also, few options are obvious: beyond accepting Babel and ESLint (and maybe PostCSS) configurations, developer needs vary drastically... so coming up with an options list is tough... especially considering the rapid pace at which the community outputs tools. Right now, there is effectively a lock-in; no support for most 3rd party build tools. I know that I'm not the only one frustrated by this. I don't want to put a burden on maintainers (which is why a back door should happen), so here's a simplest implementation that should satisfy experienced devs:

  1. pass a -c or --config argument to the react-scripts process, resolved in the following order: 1.1. is it a Git repo? 1.2. search relative to the project root 1.3. search for the absolute path on disk 1.4. is it on NPM?

  2. in the Webpack config file, check if that config path value has been passed

  3. if so, don't export the Webpack config; instead, require the config path (a function). Call that function with the Webpack config and export whatever's returned

This method lends itself to highly readable, shareable override code (which describes changes to the default config, rather than forcing users to look at the entire config file). This implementation could be refined. That being said it solves the problem, and is pretty clean. To avoid "overrides" from breaking upon future changes to the Webpack config, we can recommend that portions of the config are selected dynamically as opposed to hard-coded.

As the creator of a CRA patching tool, you might think that I'd want to prevent this from happening... but it stands out to me as––what should be––a first-class feature of CRA. @mrmckeb –– can you please check in with the maintainers who previously rejected deeper configurability? My sense is that there is more working for this than against it. I could definitely be wrong, but we should be sure. After all, this impacts the lives of millions of devs.

mrmckeb commented 5 years ago

I'll raise this during the week, and get back to you on this.

harrysolovay commented 5 years ago

@mrmckeb what's the verdict?

harrysolovay commented 5 years ago

@mrmckeb 🖖

szpigielm commented 5 years ago

We are using monkey-react-scripts to modify web-pack in CRA without ejecting it. Solution like that could be supported by CRA team, this will tick all the boxes in terms of control for who needs it without touching the CRA.

harrysolovay commented 5 years ago

@szpigielm that appears to be another patching tool. This discussion is about overriding configuration without patching tools.

mrmckeb commented 5 years ago

Hi @harrysolovay, so the verdict is this: For now, we're moving forward with an approach to add templated projects. This will be our first step towards customisation. After that has been out for a short time, we'll come back to this.

I personally feel this is important for the future of the project, and I don't think anyone in the team disagrees. The concern is around the best way to do this, and how each approach impacts support and reliability.

harrysolovay commented 5 years ago

@mrmckeb gotcha. Will you ping this issue when you & the team are ready to move forward with offering advanced configurability?

ulrichb commented 5 years ago

@mrmckeb

Please also consider allowing to configure paths.js (or at least the root directory, i.e. the source root path) to support custom project layouts. For example src/frontend (as the CRA source root) next to some src/backend, ...

At the moment this is not possible with craco (see https://github.com/sharegate/craco/issues/46), which is the reason, I had to patch CRA, which by the way I'm doing since CRA 2.1.0 (and before with CRA TS) and it works great (had no issues with my custom paths).

jednano commented 5 years ago

@mrmckeb can you elaborate as to how templated projects will work or what they might support?


I realize a verdict has already been made, but nobody has even mentioned the option of just not supporting those who stray off the golden path. I propose the following:

  1. Default CRA is the same as it is today.
  2. Do what @harrysolovay suggested: "Specify an innocuous function: takes in, modifies and returns the updated Webpack config. This would amount to a single additional test."
  3. Add documentation and/or information in issue templates to convey that support ends where extending the webpack config begins.
mrmckeb commented 5 years ago

@jedmao Templates have been prototyped, but well shelved while we sorted out a few other things - I think we're about to get back to them.

I don't disagree with you, I'd like to keep working to open up CRA... I'll raise this again and see what others think.

iamandrewluca commented 4 years ago

This is how we made CRA somehow extendable.

When a new version of react-scripts is released:

We are aware that when something does not work, it may be our problem first. This is why is better to review new changes before rebasing.

It would be great if CRA would allow this, and warn users, they should take responsability when using this.

Example of extending:

config/webpack.config.override.js ```js const LodashModuleReplacementPlugin = require('lodash-webpack-plugin') const path = require('path') const paths = require('@winify/react-scripts/config/paths') const transformImports = ['transform-imports', { 'reactstrap': { transform: 'reactstrap/lib/${member}', /* eslint-disable-line no-template-curly-in-string */ preventFullImport: true }, }] // override webpack config module.exports = function (config, env) { // This will tell webpack to not resolve symlinks to actual folders // Symlinks into `src` folder will work with this option // default value is set to `true` // this allows to npm link our `private-package` into app config.resolve.symlinks = false // fix `redux-form` `instance of` comparison // the problem was that when testing `if (error instanceof SubmissionError)` in `private-package` // one instance was from `node_modules/redux-form` and other from `node_modules/private-package/node_modules/redux-form` // to prevent that we tell webpack to always resolve `redux-form` and other modules from root node_modules const nodeModules = paths.appNodeModules config.resolve.alias['redux'] = path.resolve(nodeModules, 'redux') config.resolve.alias['react-redux'] = path.resolve(nodeModules, 'react-redux') config.resolve.alias['redux-form'] = path.resolve(nodeModules, 'redux-form') config.resolve.alias['redux-api-middleware'] = path.resolve(nodeModules, 'redux-api-middleware') config.resolve.alias['react'] = path.resolve(nodeModules, 'react') config.resolve.alias['react-dom'] = path.resolve(nodeModules, 'react-dom') config.resolve.alias['react-router'] = path.resolve(nodeModules, 'react-router') config.resolve.alias['react-router-dom'] = path.resolve(nodeModules, 'react-router-dom') config.resolve.alias['i18next'] = path.resolve(nodeModules, 'i18next') config.resolve.alias['react-i18next'] = path.resolve(nodeModules, 'react-i18next') // disable built-in eslint to enable custom one // custom eslint is extending CRA config const eslintRule = config.module.rules[1] eslintRule.test = /\.(js|mjs|jsx|ts|tsx)$/ const eslintOptions = eslintRule.use[0].options delete eslintOptions.baseConfig delete eslintOptions.ignore delete eslintOptions.useEslintrc // add `private-package` to babel-loader config because is not transpiled const babelLoader = config.module.rules[2].oneOf[1] delete babelLoader.exclude const privatePackagePath = path.resolve(paths.appNodeModules, 'private-package') babelLoader.include = Array.isArray(babelLoader.include) ? [...babelLoader.include, privatePackagePath] : [babelLoader.include, privatePackagePath] // strip out unused imports const babelOptions = babelLoader.options babelOptions.plugins = [...babelOptions.plugins, transformImports, 'lodash'] config.plugins = [ new LodashModuleReplacementPlugin({ collections: true, paths: true, shorthands: true, currying: true, flattening: true, }), ...config.plugins ] return config } ```
config/webpackDevServer.config.override.js ```js // override webpackDevServer config module.exports = function (serverConfig) { serverConfig.watchOptions.ignored = [ /node_modules([\\]+|\/)+(?!private-package)/, /private-package([\\]+|\/)node_modules/ ] } ```
config/jest.config.override.js ```js // override jest config module.exports = function (config) { // fix `redux-form` `instance of` comparison // @see ./webpack.config.override.js config['moduleNameMapper']['^redux-form$'] = '/node_modules/redux-form' // whitelist `private-package` to be transpiled with babel // https://github.com/facebook/create-react-app/blob/782d71b957704cf40634771c0bc733fc4555f91a/packages/react-scripts/scripts/utils/createJestConfig.js#L56 config['transformIgnorePatterns'] = [ '[/\\\\]node_modules[/\\\\](?!private-package[/\\\\]).+\\.(js|jsx|ts|tsx)$', '^.+\\.module\\.(css|sass|scss)$', ] return config } ```
config/env.js ```js // custom dynamic environment variables process.env.REACT_APP_GIT_BRANCH = require('child_process') .execSync('git rev-parse --abbrev-ref HEAD') .toString().trim() const commitHash = require('child_process') .execSync('git rev-parse HEAD') .toString().trim() const commitDatetime = require('child_process') .execSync(`git show -s --format=%ci ${commitHash}`) .toString().trim() process.env.REACT_APP_GIT_HASH = `${commitHash.substr(0, 8)} (${commitDatetime})` process.env.REACT_APP_PRIVATE_PACKAGE_VERSION = require('private-package/package').version ```
philippefutureboy commented 4 years ago

I support @jedmao with his proposition of offering the ability of extension without further support. I think the policy should be very simple: You modify your configuration at your own risk. You could also have a warning that is displayed when bundling or when deploying the development server that points to the CRA team policy on configuration extension.

In terms of file structure, I think that something similar to @iamandrewluca is a great place to start. Either a folder config or overrides in which each file pertains to a specific config to override:

overrides/
  webpack.config.override.js
  webpackDevServer.config.override.js
  jest.config.override.js
  env.override.js

Each file would consist in a function that receives result of the default configuration as parameter and apply overrides as necessary.

This being said, thanks to all of the CRA team for your great work 🎉

Peace!

mrmckeb commented 4 years ago

I also support this - not necessarily the above structure - but overrides without support.

The problem is, how do we:

  1. ensure that people know that overriding means no support.
  2. ensure that beginners aren't copying needlessly complex guides, when the feature they want/need is now native to CRA (this happens a lot with TypeScript - people use react-scripts-ts or eject)
  3. know where issues actually come from? People may blame CRA, even if they've created the issue.

The other concern is that this would mean we can no longer make major bumps to dependencies in react-scripts without major releases. Right now, we can move from Webpack 4 to Webpack 22 in a minor release - which is great for us and the community that aren't customising, but would break a system like the one proposed.

hershmire commented 3 years ago

Looks like this conversation has been quite for a while. Has there been any recent progress into supporting more advanced configurations for those who need it?

fdc-viktor-luft commented 3 years ago

@mrmckeb I totally understand that this feature might lead to unnecessary bug reports, where people might want to blame CRA too fast.

Nevertheless adding more configuration options to CRA will just slow down further improvements. By giving your users the full power without ejecting, anyone can extend CRA. The community might write plugins that can be used on top. Writing custom adjustments of course might produce errors.

By the way I never blamed CRA for the few migration adjustments I had to do while using react-app-rewired and transitioning from react-scripts@0.x.x to the latest version in the last couple of years.

ashvin777 commented 3 years ago

I was able to change Webpack configurations without ejecting the CRA app. It was done using the rewire npm package with minimal changes -

First change the scripts in pkg.json to below

   "start": "node scripts/start.js",
   "build": "node scripts/build.js",

scripts/start.js

const rewire = require('rewire');
const defaults = rewire('react-scripts/scripts/start.js');
const webpackConfig = require('react-scripts/config/webpack.config');

//In order to override the webpack configuration without ejecting the create-react-app
defaults.__set__('configFactory', (webpackEnv) => {
  let config = webpackConfig(webpackEnv);

  //Customize the webpack configuration here, for reference I have updated webpack externals field
  config.externals = [..];

  return config;
}); 

scripts/build.js

const rewire = require('rewire');
const defaults = rewire('react-scripts/scripts/build.js');

//In order to override the webpack configuration without ejecting the create-react-app
const config = defaults.__get__('config');

//Customize the Webpack configuration here,  for reference I have updated Webpack externals field
config.externals =[..]; 

This is working as expected in one of my applications. The good thing about this is it doesn't break the create-react-app structure and is very simple to integrate compared to react-app-rewired

ashvin777 commented 3 years ago

You can also now use the npm package cra-webpack-rewired which supports these changes already. Follow the below steps to use it -

npm install --save-dev cra-webpack-rewired

Update package.json

 "scripts": {
-    "start": "react-scripts start",
+    "start": "cra-webpack-rewired start",
-    "build": "react-scripts build",
+    "build": "cra-webpack-rewired build",
  }

Add webpack extend file at path config/webpack.extend.js

module.exports = {
  dev: (config) => {
    //override webpack configuration
    config.externals =[..];
    return config;
  },
  prod: (config) => {
    //override webpack configuration
    config.externals =[..];
    return config;
  }
};

Thats it !

fkirc commented 9 months ago

The benefit of options is we can also offer support for those - whereas we can't for a full-control setup.

R.I.P. creare-react-app. create-react-app has helped me to learn React, but I have never seen a larger misjudgement from a high-ranking opensource-maintainer (and I also maintain some small projects myself). There will never be a support for “partial control options“ because no maintainer would ever do the work of supporting such a gigantic range of options in a rapidly changing environment. Therefore, the “change at your own risk“-approach is the only approach that would work with the time constraints of the maintainers.

Anyways, now it is too late and the damage is already done. Me and many others devs have already migrated to other frameworks like next.js.

fdc-viktor-luft commented 9 months ago

Or simply Vite

BraianS commented 4 months ago

Hello, I agree with @fdc-viktor-luft . I see a good alternative use Vite Any updates for this issue? Otherwise, you can close it as inactive. Thanks.