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

Remove eslintConfig from package.json #649

Merged
merged 1 commit into from
Sep 14, 2016

Conversation

fson
Copy link
Contributor

@fson fson commented Sep 14, 2016

tl;dr The eslintConfig setting is used to make editor plugins such as linter use our ESLint configuration, but at the moment this requires additional packages and configuration to work, and we should not create the project with broken configuration. The configuration can still be added manually.

At the moment, when you create an app, the eslintConfig setting gets added to package.json. If you have an ESLint package (e.g. linter with linter-eslint) installed in your editor, you'll get a big error:

screen shot 2016-09-14 at 18 32 55
This error is shown every time you save a file, so you're forced to figure out what to do, even if you were just starting out and didn't want to configure the integrations yet.

After a little digging you'll find the instructions for installing the ESLint integration. We instruct the user to install ESLint and all the ESLint plugins globally and to configure the editor plugin to use the global ESLint. This is less than ideal for a few reasons:

  • The user now needs to track the ESLint plugins dependencies used by CRA and their versions manually and keep their global packages up-to-date.
  • You can't have different projects with different ESLint or ESLint plugin versions. Opening a file from a (non-CRA) project using an incompatible version now creates an error because your editor tries to use the globally installed ESLint and plugins:
    screen shot 2016-09-14 at 16 37 36

I've also seen people trying to edit the eslintConfig in package.json in order to use their own ESLint configuration, which doesn't work, which also makes having the configuration there misleading.

As we mention in the readme, having to rely on global ESlint will be fixed when ESlint is going to add support for having plugins as dependencies in packages like react-scripts, so we can enable the integration by default at that point. But until this has been fixed, it's better to not add this configuration by default. People can still find the instructions and add it manually when they want it and they're ready to do the plugin setup, but we should not create a configuration that doesn't work out of the box.

@ghost ghost added the CLA Signed label Sep 14, 2016
@gaearon gaearon merged commit f4a53ee into facebook:master Sep 14, 2016
@gaearon gaearon added this to the 0.4.2 milestone Sep 14, 2016
@gaearon
Copy link
Contributor

gaearon commented Sep 14, 2016

Okay. I was hoping the ESLint issue would get solved faster but alas.

@fson fson deleted the remove-eslint-config branch September 14, 2016 17:08
@fson
Copy link
Contributor Author

fson commented Sep 14, 2016

Yeah, me too. Also thought about having our own executable like standard does, but it wouldn't solve the editor integrations because the plugins call ESLint directly.

@gaearon gaearon mentioned this pull request Sep 18, 2016
feiqitian pushed a commit to feiqitian/create-react-app that referenced this pull request Oct 25, 2016
caulagi added a commit to ishakuni/frontend that referenced this pull request Oct 24, 2018
caulagi added a commit to ishakuni/frontend that referenced this pull request Oct 24, 2018
* Remove eslintConfig from package.json - see facebook/create-react-app#649
* Use latest version of yarn
* Tidy packages - reason-react is a dependency, not dev-dependency
* Need to run bsb -make-world before running tests
* No need to run build before test
caulagi added a commit to ishakuni/frontend that referenced this pull request Oct 26, 2018
* Test real component being rendered on page

* Fix tests

* Remove eslintConfig from package.json - see facebook/create-react-app#649
* Use latest version of yarn
* Tidy packages - reason-react is a dependency, not dev-dependency
* Need to run bsb -make-world before running tests
* No need to run build before test

* Run clean before test

* reason-scripts is dev dependency
* Test if this caches ocaml building each time

* Fix tests - rescript-lang/rescript-compiler#3094

* Pin version correctly and remove linking bs-platform
@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 this pull request may close these issues.

2 participants