facebook / create-react-app

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

Support for standard #1847

Closed shai32 closed 7 years ago

shai32 commented 7 years ago

I added standard for syntax but I am getting error like: "'it' is not defined." it can easily fix by adding to .eslintrc but modifying .eslintrc is not recommended, what should I do?

gaearon commented 7 years ago

We don’t officially recommend Standard and don’t have suggestions for integrating with it. Certain features that CRA supports may not work with Standard. If you want to configure it with ESLint config, please refer to Standard’s documentation on how to do it. This would not be related to CRA’s ESLint config because CRA does not integrate with Standard in any way.

If you just want to enforce style, we recommend using Prettier. It supports all CRA features and formats code automatically.

shai32 commented 7 years ago

Do you plan to make Prettier integrated in create-react-app?

gaearon commented 7 years ago

Yes, eventually.

ArmorDarks commented 7 years ago

@gaearon Can you be more specific what exactly might not work in CRA if Standard were integrated?

gaearon commented 7 years ago

I just don’t know if the parser has feature parity. For example I don’t know if Standard works with class properties or Flow.

In any case, manually fixing style issues is the opposite of what we want CRA experience to be like. So is mixing real issues (as reported by ESLint) with style violations.

I really recommend you to check out Prettier if you’re using Standard. I think it fits the task better.

ArmorDarks commented 7 years ago

I just don’t know if the parser has feature parity. For example I don’t know if Standard works with class properties or Flow.

That's what troubles me the most. I thought team has made some research before taking solution into core of create-react-app.

It works, through flow eslint plugin.

In any case, manually fixing style issues is the opposite of what we want CRA experience to be like.

ESLint and Standard have great function hidden under --fix flag, which does relatively same thing, so you won't need to fix lint errors manually. However, it does much less work on code formatting field, and much better work in lint errors domain.

So is mixing real issues (as reported by ESLint) with style violations.

Well, this is something worth thinking about. Though, I'm not sure. The line between those two often is too thin to say exactly which solution should, and which shouldn't be used. It goes to the point, that usually decoupling those two sounds like unreasonable idea.

I really recommend you to check out Prettier if you’re using Standard. I think it fits the task better.

Actually, this makes me unhappy. Once again instead of contributing to already great solution, JS community decides to invent yet another bicycle with slightly different pedals, thus fragmenting community further.

gaearon commented 7 years ago

That's what troubles me the most. I thought team has made some research before taking solution into core of create-react-app.

I am talking specifically about Standard which is not in the core and is the tool I am suggesting not to use. Because we can't control which parser features it enables in ESLint and whether they match our set.

ESLint and Standard have great function hidden under --fix flag, which does relatively same thing

It doesn't do the same thing in practice. Please refer to Prettier README which explains the difference very well.

Once again instead of contributing to already great solution, JS community decides to invent yet another bicycle with slightly different pedals, thus fragmenting community further.

I'm sorry you are unhappy. I think Prettier README also describes well why they couldn't just contribute to Standard or ESLint. ESLint was not originally written for catching style violations (as confirmed by ESLint creator), and Prettier employs a different approach. ESLint can't fix those problems in principle because of a different architecture and goals.

Really, I would rather not recite Prettier README in this issue. It touches on all of the points you brought up, and I recommend reading it.

mrhyde commented 7 years ago

@shai32 you just need to add this line /* global it */ to test files like App.test.js or edit .eslintrc in your project, that is exactly what it was made for. And don't listen to Dan, CRA works just fine with standard

ArmorDarks commented 7 years ago

Actually it would be better to add /* eslint-env jest */

ArmorDarks commented 7 years ago

Because we can't control which parser features it enables in ESLint and whether they match our set.

Standard allows definition of parsers through ESLint-like configuration. It isn't an issue, it doesn't enforce nothing, except linting rules.

Please, read Standard readme.

I'm sorry you are unhappy. I think Prettier README also describes well why they couldn't just contribute to Standard or ESLint. ESLint was not originally written for catching style violations (as confirmed by ESLint creator), and Prettier employs a different approach. ESLint can't fix those problems in principle because of a different architecture and goals.

After giving it a better though, I think we are missing a point here.

Prettier enforces style formatting only, it doesn't lint. For this purpose CRA already uses ESLint. Standard is exactly set of standardized and widely accepted by community ESLint rules. Yes, it crossfires with Prettier slightly, but still it's main purpose is that you drop it in repository as replacement of ESLint and never touch ESLint configs anymore, just follow the guidance.

Generally it is good practice and I think that both tools greatly complementing each other.

So the purpose of this issue was to remove direct usage of ESLint and use Standard instead, not to drop Prettier in favor of Standard.

ArmorDarks commented 7 years ago

As a side information, despite being ESLint-based solution, Standard is much more capable in terms of lint rules enforcement and --fix formatting than ESLint, and there is quite great community stands behind Standard rules compilation.

Since CRA has very large impact on community decision, it would be way better if CRA will embrace Standard and participate in its further development of JS standards and decisions taking process, as well as in Prettier's, instead of making own set of rules with blackjack and... you know. All communities will only win from this.

gaearon commented 7 years ago

Standard is exactly set of standardized and widely accepted by community ESLint rules. Yes, it crossfires with Prettier slightly, but still it's main purpose is that you drop it in repository as replacement of ESLint and never touch ESLint configs anymore, just follow the guidance.

I'd be happy to consider this if it didn't include style rules and would be open to considering React-specific rules. I don't think either would happen though.

gaearon commented 7 years ago

If I read through http://standardjs.com/rules.html, a very large portion of these are indeed style rules. And Standard calls itself a "JavaScript style". I don't think it's fair to say it complements Prettier—it targets the very same niche, but is very opinionated.

Our ESLint integration is invasive on purpose. We want to highlight potential mistakes so we print warnings in terminal and console. Once we add style rules there, it just becomes noise beginners who copy and paste examples will ignore.

I'm fine with opinionated style if it's always enforced automatically and doesn't show up in warning reports. That's what Prettier does. I have no doubts Standard attempts that too, but due to architecture there are some formatting issues (such as being unaware of line length) it just can't solve.

For a standardized set of non-style rules, eslint:recommended is a much more focused set than Standard. So if you're pushing for community standards, IMO it would be more sensible to adopt that. However last time I checked it, it didn't really match my opinions of what is important and what isn't in a React project. My opinions are shaped by issues I read and my experience of helping beginners use React so it's biased but hopefully you can agree having some opinions helps drive a project like this. So far I have appreciated having full control over our ESLint config, and I'm happy we don't have to seek consensus with a wider community using ESLint for all kinds of purposes just when we want to enforce something in CRA. We know our scope very well and I think this control is good leverage I would not want to give up.

ArmorDarks commented 7 years ago

I'd be happy to consider this if it didn't include style rules

I don't think those really matters. Both encouraging best practices, and if someone will change format after another, nobody will die

Anyway, it is more a matter of decision, whant CRA embrace it or no, other things are solvable. I think it is possible to direct Standard and Prettier communities to find compromise and work together. But it definitely won't work if we will continue that war and take decision based only at our guts feelings.

considering React-specific rules. I don't think either would happen though.

It supports React in same way as ESLint do. You just pass in as a parser babel-eslint, and it works. In fact, right now I'm working on React app with JSX and ES6 modules, all passed through Standard.

mrhyde commented 7 years ago

Ok, allow me to jump on this discussion train with simple statements:

ESLint - let you know when you fked up Standard - that, and will show you how not to do that in the future (best practice enforcement) Prettier - format your code taking in consideration the maximum line length** (yep, is that useless)

Now if you want to have a marry this two things and have a nice and happy life - people made that for you already: prettier-standard

Problem solved.

P.S. I personally can't see how a tool that promotes syntax mistakes and bad code style is supposed to help JS community (they even do such examples in readme and blog). If you are making mistakes - you should be aware of them, especially if you are a beginner, so you can look them up and learn. And this is where Standard is doing a job right, and Prettier - not so much.

gaearon commented 7 years ago

I don't think those really matters. Both encouraging best practices, and if someone will change format after another, nobody will die

"Nobody will die" is not how we make decisions in this project. 😉 As I already explained, I don't think Standard's goals (enforce style via sometimes autofixable warnings) match our goals (use linting to find bugs, and enforce style completely automatically with regards for line width).

I think it is possible to direct Standard and Prettier communities to find compromise and work together

I already mentioned this before, but Prettier README explains very clearly why its approach would not even be possible in any tool built on top of ESLint. Standard is such a tool.

Nobody is at "war" here. There is more than one possible solution to the problem. I appreciate that you enjoy Standard, but it's built on top of ESLint which was not intended for enforcing style and has fundamental issues in that field. And similarly Standard won't work for finding bugs in CRA because it nevertheless has style rules (and indeed enforcing style is one of its goals).

I'm afraid I'm repeating the same two arguments in this thread. The gist is: we don't intend to integrate with Standard because it doesn't solve our problems. It is focused on enforcing style but it's built on top of a tool that wasn't designed for that. No, there is no way to fix it without fundamentally changing how it works. You can bring it up with Standard maintainers but I'm pretty sure they don't plan to rewrite it on top of something other than ESLint. And at this point we might as well use a project that was designed around different tradeoffs that make more sense for our use case (Prettier is).

It supports React in same way as ESLint do.

I don't mean parsing in this context, but our ability to add React-specific rules (for example for depreciations) to our config. That's pretty important for us which is why we don't outsource our config to, for example, ESLint.

gaearon commented 7 years ago

ESLint - let you know when you f**ked up Standard - that, and will show you how not to do that in the future (best practice enforcement)

I think this is at the root of our mutual misunderstanding in this issue. What are the "best practices" that Standard enforces but our ESLint configuration doesn't? I'll be happy to consider adding these valuable rules to our config. Can you share a few examples?

cyrilchapon commented 6 years ago

@gaearon

What are the "best practices" that Standard enforces but our ESLint configuration doesn't?

Not ending every single line of code with a useless semicolon, for example.

...and as everyone knows, this is a subjective topic. I like it, some don't. That's why a tool like standard was created. Not being subjective.