Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

☂️ [PAUSED] React lint rules #341

Closed
sebmck opened this issue Apr 27, 2020 · 58 comments
Closed

☂️ [PAUSED] React lint rules #341

sebmck opened this issue Apr 27, 2020 · 58 comments
Labels
A-Linter Area: linter umbrella Issue to track a collection of other issues

Comments

@sebmck
Copy link
Contributor

sebmck commented Apr 27, 2020

This issue serves to discuss what React rules we should have in Rome. Ideally these rules would be blessed by the React team.

What rules do you think are the most important? Keep in mind that Rome does not allow disabling lint rules. You can only suppress them. The React rules will not be an exception. Rome allows AST autofixes which drastically simplifies the existence of many lint rules by allowing them to be automatically fixed, and with the Rome review system, unsafe suggestions are also easily added and amended.

@sebmck
Copy link
Contributor Author

sebmck commented Apr 27, 2020

It would be nice to have a strong set of accessibility rules. We could have autofixes/suggestions that infer from usage what props the author probably meant.

@Daniel15
Copy link
Contributor

Daniel15 commented Apr 27, 2020

FWIW here's the list of lint rules included with Create React App, which is probably a good guide as to which rules a lot of people are using today: https://github.com/facebook/create-react-app/blob/master/packages/eslint-config-react-app/index.js

The "Rules of Hooks" rule is a pretty important one for React hooks: https://github.com/facebook/react/tree/master/packages/eslint-plugin-react-hooks

It would be nice to have a strong set of accessibility rules.

Create React App uses eslint-plugin-jsx-a11y by @evcohen and @jessebeach which has some decent rules: https://github.com/evcohen/eslint-plugin-jsx-a11y/

@jamiebuilds
Copy link
Contributor

jamiebuilds commented Apr 27, 2020

Spreadsheet of all rules with related info and statuses

@sebmck
Copy link
Contributor Author

sebmck commented Apr 27, 2020

A lot of those seems so superfluous. We can pretty much ignore any of the rule that are specific to createClass. Also there’s many there that just get around ESLints poor handling of JSX, that we already handle such as variables in JSX being marked as used.

@ljharb
Copy link

ljharb commented Apr 27, 2020

There’s also the Airbnb config’s react rules worth considering.

@diokey
Copy link
Contributor

diokey commented Apr 27, 2020

Are react specific rules going to be implemented as expansion packs as discussed in #173 ? It didn't seem like the discussion was conclusive. Maybe this is a good time to make that decision.

@sebmck
Copy link
Contributor Author

sebmck commented Apr 27, 2020

Thinking about expansion packs, it feels like having an on switch just for the sake of it. We can detect the React code pretty unambiguously so there's no reason to have configuration to turn it on.

@tatchi
Copy link
Contributor

tatchi commented Apr 29, 2020

Something that we might consider: https://twitter.com/dan_abramov/status/1255237989548134405?s=20

@macovedj
Copy link
Contributor

Is it safe to assume that the recommended rules are not superfluous? If there's not going to be an expansion pack, could people have at those ones?

@VictorHom
Copy link
Contributor

VictorHom commented May 10, 2020

If no one has started working on jsx-a11y/accessible-emoji, can I work on it?

@VictorHom
Copy link
Contributor

I'm going to take a look at implementing: react/no-did-update-set-state if no one has started 🙏

@sebmck
Copy link
Contributor Author

sebmck commented May 16, 2020

Before submitting React or JSX lint rules of any kind please get confirmation in this issue or on the Discord.

@sebmck
Copy link
Contributor Author

sebmck commented May 16, 2020

@jamiebuilds Could you look at finalizing the rule list? The comment at the top with a link in big bold letters looks like it's finalized and ready for people to work on. While a lot of them are fine, I don't want someone to spend a lot of time on ones that we don't want only to have their PR rejected.

@VictorHom
Copy link
Contributor

There's been other things happening recently, but I am still working on these rules:
no-this-in-sfc
react-hooks/rules-of-hooks
react/no-array-index-key

@yeonjuan
Copy link
Contributor

yeonjuan commented Jun 4, 2020

Can I take a react/require-render-return? - #578

@Kelbie
Copy link
Contributor

Kelbie commented Jun 10, 2020

I'll take autocomplete-valid

@yeonjuan
Copy link
Contributor

I'll take react/no-render-return-value.

@VictorHom
Copy link
Contributor

gonna start on react-hooks/rules-of-hooks

I'll also take a look at react/jsx-props-no-spreading

@stefanuros
Copy link
Contributor

Ill take a look at react/sort-comp

@VictorHom
Copy link
Contributor

Still working on react-hooks/rules-of-hooks. I should have something soon

@iaravindreddyp
Copy link
Contributor

can i try react/state-in-constructor ?

@sebmck sebmck changed the title React lint rules [PAUSED] React lint rules Jun 30, 2020
@sebmck
Copy link
Contributor Author

sebmck commented Jun 30, 2020

Hey everyone! I'd like to pause development of new React lint rules (if you're currently working on or have reserved something in this thread then feel free to complete it!). We're preparing for our first release and should focus on stability and polish.

We need to take a more comprehensive look on the sort of React rules and patterns we want to encourage (ideally with approval from the React team or "community leaders" (whatever that means lol)). For that reason there may be some rules that will undergo significant tweaks or even removal. I'll make sure to keep this thread updated with those discussions and as we finalize any guidelines in case anyone wants to get involved.

Thank you everyone for your participation so far! Really exciting to see the contributions.

@sebmck sebmck changed the title [PAUSED] React lint rules ☂️ [PAUSED] React lint rules Jul 8, 2020
@sebmck sebmck added umbrella Issue to track a collection of other issues A-Linter Area: linter and removed discussion labels Jul 8, 2020
@sebmck sebmck unpinned this issue Jul 8, 2020
@sebmck sebmck added this to the Post-release milestone Jul 14, 2020
@ematipico ematipico changed the title ☂️ [PAUSED] React lint rules ☂️ React lint rules Oct 16, 2020
@ematipico ematipico changed the title ☂️ React lint rules ☂️ [PAUSED] React lint rules Oct 16, 2020
@sebmck
Copy link
Contributor Author

sebmck commented Apr 11, 2021

Going to close this and use more finegrained issues.

@sebmck sebmck closed this as completed Apr 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Linter Area: linter umbrella Issue to track a collection of other issues
Projects
None yet
Development

No branches or pull requests