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

Not compatible with VS Code + ESLint plugin #124

Closed
kyleect opened this issue Jul 23, 2016 · 21 comments · Fixed by #149
Closed

Not compatible with VS Code + ESLint plugin #124

kyleect opened this issue Jul 23, 2016 · 21 comments · Fixed by #149

Comments

@kyleect
Copy link

kyleect commented Jul 23, 2016

Perhaps this is an issue not in the scope of this project but...

Steps

  1. $ create-react-app testApp && cd testApp
  2. Open project in VS Code with ESLint plugin installed
  3. Open any javascript file

Expected

ESLint plugin lints my code and displays errors.

Actual

VS Code display this error: No ESLint configuration found


This makes sense because there isn't an eslint config file anywhere. I ran npm run eject and changed config/eslint.js to config/.eslintrc.js to match '**/.eslintr{c.js,c.yaml,c.yml,c,c.json}' per https://github.com/Microsoft/vscode-eslint/blob/master/eslint/extension.ts#L40. This didn't fix the issue. Does anyone have any suggestions?

@lacker
Copy link
Contributor

lacker commented Jul 23, 2016

Is the issue that it doesn't work with VS Code + ESLint out of the box, or that even after ejecting you couldn't get it to work with VS Code + ESLint?

@kyleect
Copy link
Author

kyleect commented Jul 23, 2016

After ejection. I think its reasonable that it doesn't work out of the box as there is no config file by design.

@mxstbr
Copy link
Contributor

mxstbr commented Jul 23, 2016

After ejection is definitely a bug, but I wonder if we can somehow fix this before ejection too?

@gaearon
Copy link
Contributor

gaearon commented Jul 23, 2016

We should probably put eslint "extends" into package.json (eslint supports that), and point extends to our config.

@vjeux
Copy link
Contributor

vjeux commented Jul 23, 2016

cc @jamesgpearce who mentioned this for Nuclide as well

@gaearon
Copy link
Contributor

gaearon commented Jul 23, 2016

One constraint I'd like to operate under is we don't create configuration files as part of project init.

It's okay to add a key-value pair to package.json but please let's try to keep this as simple as possible.

@gaearon
Copy link
Contributor

gaearon commented Jul 23, 2016

Ideally I'd like different tools to adopt a convention of configuration field in package.json that would specify base directory for all configs. We would just put react-scripts/config there, and Flow and ESLint could autodiscover them there. Tools don't currently support this because zero conf environments are rare in JS.

@kyleect
Copy link
Author

kyleect commented Jul 24, 2016

A temporary workaround (preventing the error from keep popping up) in VS Code is setting in my workspace settings.

{
  "eslint.enabled": false
}

@kevinastone
Copy link

Dumb question, isn't that implied by the absence of an eslint config? Is VSCode being a bit obnoxious here?

@kyleect
Copy link
Author

kyleect commented Jul 24, 2016

Totally reasonable pre-ejection but post-ejection there is a eslint config file. I'd be fine ejecting to get this to work but what I've tried hasn't fixed the issue.

@keyz
Copy link
Contributor

keyz commented Jul 24, 2016

#149 should fix this.

@kevinastone
Copy link

Worth highlighting that #149 requires the IDE to have the same eslint plugins since it's not using the embedded copy of eslint within react-scripts.

(took me a bit to figure out my version of babel-eslint was out of date)

@gaearon
Copy link
Contributor

gaearon commented Jul 24, 2016

Worth highlighting that #149 requires the IDE to have the same eslint plugins since it's not using the embedded copy of eslint within react-scripts.

This is probably an issue only with npm 2 so I think we can live with this (and warn about it somewhere). npm 3 should just work.

@gaearon
Copy link
Contributor

gaearon commented Jul 27, 2016

Projects created with 0.2.0 alpha shouldn't have this issue (but only if you use npm 3). Please help us test this: #190

@dburles
Copy link

dburles commented Jul 29, 2016

Just installed 0.2.0 (I'm on npm 3, node 6) and vscode complains:

"Failed to load eslint library. Please install eslint in your workspace folder using 'npm install eslint' or globally using 'npm install -g eslint' and then press Retry."

Seems like it's still not finding eslint within react-scripts.

@alexzherdev
Copy link
Contributor

Try following this comment #272 (comment). It's required for now to install eslint globally.

@gaearon
Copy link
Contributor

gaearon commented Jul 29, 2016

Please follow these instructions for integrating linting with your editor:

https://github.com/facebookincubator/create-react-app/blob/master/template/README.md#display-lint-output-in-the-editor

It is unfortunate that we require global installation but this is a problem on ESLint’s end, and they intend to solve it: eslint/eslint#3458.

@IanVS
Copy link

IanVS commented Jul 29, 2016

To be clear, it's a problem we would love to solve in ESLint, but so far have not been able to find a way to do so. The main problem is determining what should be the behavior if two plugins depend on two different versions of a third plugin. I haven't seen any proposed solutions of how to handle that. That's just to say, I wouldn't expect this to change any time terribly soon, unfortunately.

@gaearon
Copy link
Contributor

gaearon commented Jul 29, 2016

@IanVS Thanks for chiming in! Do you know how Babel solves this? It allows specifying resolved paths for plugins and presets, and presets may include other plugins and presets. Whatever strategy they use, it seems to work great for most people.

@IanVS
Copy link

IanVS commented Jul 29, 2016

I'm not super-familiar with the inner workings of Babel's plugin resolution, but @nzakas has said:

This is quite a bit more difficult than Babel plugins because you can directly reference plugins in config files. If a shareable config depends on a plugin foo, and a user has manually installed that plugin as well, then when the end user references foo, what is the expected behavior? Which instance of the plugin should it refer to?

Although, from what you say, it sounds like Babel may have indeed solved this, if they allow presets to extend other presets and plugins. I must say I've personally had difficulty in the past with Babel presets, when trying to override other presets. But maybe that's been refined by now. It's definitely something that bears more investigation.

@gaearon
Copy link
Contributor

gaearon commented Jul 29, 2016

I don’t think I quite understand that comment, to be honest.

If a shareable config depends on a plugin foo, and a user has manually installed that plugin as well, then when the end user references foo, what is the expected behavior?

Whatever is determined by Node resolution mechanism relative to the config folder.

@lock lock bot locked and limited conversation to collaborators Jan 23, 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.

10 participants