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

Add eslint-loader #362

Closed
KyleAMathews opened this issue Jul 22, 2016 · 15 comments
Closed

Add eslint-loader #362

KyleAMathews opened this issue Jul 22, 2016 · 15 comments
Assignees
Labels
help wanted Issue with a clear description that the community can help with.

Comments

@KyleAMathews
Copy link
Contributor

We ship starters with eslint configs but they're basically useless unless a user has their editor setup to support Eslint. Add eslint-loader would change that as errors would then show up in the console/browser. If we do this however, the eslint config in starters should probably be a lot less opinionated though so we're not pointlessly pushing the arbitrary eslint choices I've made here.

https://github.com/MoOx/eslint-loader

@KyleAMathews
Copy link
Contributor Author

I really like create-react-app's approach here as they define their own eslintrc that's focused on what will cause bugs not styling problems. We should just copy their approach. I think we can even just depend on their configuration directly.

https://github.com/facebookincubator/create-react-app/blob/c5f5b006d83316b3c016adc41b95c28912ecdda8/packages/react-scripts/config/webpack.config.dev.js#L106

https://github.com/facebookincubator/create-react-app/blob/c5f5b006d83316b3c016adc41b95c28912ecdda8/packages/eslint-config-react-app/index.js

@KyleAMathews KyleAMathews added help wanted Issue with a clear description that the community can help with. Hacktoberfest labels Oct 12, 2016
@karanjthakkar
Copy link

@KyleAMathews I think I can pick this up if you just want to shadow the config create-react-app has. Is there anything else you'd want to be considered in this?

@KyleAMathews
Copy link
Contributor Author

Thinking about it more, we might not want to use their exact lint rules as I think they disallow some things that we wouldn't want to disallow.

How about instead of directly depending on their config, you just copy it into our project so we can change things as needed.

@f0rr0
Copy link

f0rr0 commented Oct 14, 2016

Also consider checking out https://github.com/sindresorhus/xo. It comes with great defaults and the logging is really pretty. There is a loader similar to eslint for webpack as well.

@MoOx
Copy link

MoOx commented Oct 14, 2016

Xo is just an eslint config at the end ;)

@f0rr0
Copy link

f0rr0 commented Oct 14, 2016

@MoOx exactly the config can be dropped in as well if not the entire thing. https://github.com/sindresorhus/eslint-config-xo, but aesthetically speaking xo looks nicer. Not sure if that is a concern here :p

@KyleAMathews
Copy link
Contributor Author

XO is very nice. I haven't mentioned it before (was planning on making a new issue but perhaps we can just do it here) but I really like how Dan added support for showing lint errors in the browser in an overlay — https://twitter.com/dan_abramov/status/780017771447025672

I think we should copy that. It's much better than just showing them in the browser/terminal console.

@f0rr0
Copy link

f0rr0 commented Oct 14, 2016

create-react-app has it's own dev-server client: https://github.com/facebookincubator/create-react-app/blob/00c920806719de8f527b13c03bd17f2a2bc60d34/packages/react-dev-utils/webpackHotDevClient.js This is a drop in replacement so it should work for gatsby

@f0rr0
Copy link

f0rr0 commented Oct 14, 2016

@f0rr0
Copy link

f0rr0 commented Oct 14, 2016

If there's a consensus on using XO, I'd be happy to take that up

@KyleAMathews
Copy link
Contributor Author

My main thing is putting the warnings on the overlay. Beyond that I haven't
researched much.
On Fri, Oct 14, 2016 at 11:51 AM Siddharth Jain [email protected]
wrote:

If there's a consensus on using XO, I'd be happy to take that up


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#362 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEVh1gAUjbYfmRj4IV6xjPhtEbxf7wkks5qz882gaJpZM4JTJqS
.

@jbolda jbolda added DX labels Jun 5, 2017
@lfilho
Copy link

lfilho commented Aug 9, 2017

Hello fellas, any news on this? I'm also looking forward to xo support. Right now my editor (which already supports eslint and xo) shows me plenty of errors (like "react should be listed in the project's dependencies" for example).

Thanks for you great work

@KyleAMathews
Copy link
Contributor Author

@lfilho this issue won't solve that — if you're running your own eslint configuration then you'll need to adjust it to match Gatsby e.g. disabling the check that all dependencies are listed in your package.json.

@stale
Copy link

stale bot commented Feb 6, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Feb 6, 2018
@KyleAMathews KyleAMathews removed the stale? Issue that may be closed soon due to the original author not responding any more. label Feb 8, 2018
@kkemple kkemple self-assigned this Apr 16, 2018
@kkemple
Copy link
Contributor

kkemple commented Apr 16, 2018

this is added in #4893 🎉 (eslint config is based on create-react-app settings)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issue with a clear description that the community can help with.
Projects
None yet
Development

No branches or pull requests

8 participants