Closed Bug 1315922 Opened 3 years ago Closed 3 years ago

Enable ESLint react/prop-types rule for devtools

Categories

(DevTools :: General, defect, P3)

defect

Tracking

(firefox52 affected, firefox53 fixed)

RESOLVED FIXED
Firefox 53
Tracking Status
firefox52 --- affected
firefox53 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

Details

Attachments

(6 files)

Meta bug to list all the remaining effort before we can switch the react/prop-types ESLint rule from 0 to 2 :

http://searchfox.org/mozilla-central/rev/f5c9e9a249637c9abd88754c8963ecb3838475cb/devtools/.eslintrc.js#59
Blocks: 1251271
Depends on: 1264929
Depends on: 1315923
Depends on: 1315925
Bug 1315925 will land soon, but since the eslint validation is not enabled for propTypes, more violations have been introduced in the meantime.
The main offenders are now :
- client/memory/
- client/webconsole/
- client/netmonitor/

I will add patches in this bug to solve them so that we can finally enable the rule.
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Summary: [meta] Enable ESLint react/prop-types rule for devtools → Enable ESLint react/prop-types rule for devtools
I'd like to move forward with this bug because the more we wait, the more violations to the rule get created.

I've been trying to catch up progressively, but violations appear faster than I can fix them and rebasing is starting to be a headache :) 

From now on I'll simply eslint ignore (for react/prop-types) new offending files and will log follow up bugs once this lands.
With the set of patches under review here, only 3 files are ignored. Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a93351871f03a14b0c3fd97f3593908b9514b37
Comment on attachment 8825389 [details]
Bug 1315922 - fix react/prop-types issues in client/webconsole;

https://reviewboard.mozilla.org/r/103564/#review104166
Attachment #8825389 - Flags: review?(chevobbe.nicolas) → review+
Comment on attachment 8825387 [details]
Bug 1315922 - reps: avoid using props as variable name in react classes;

https://reviewboard.mozilla.org/r/103560/#review104168

LGTM !
Attachment #8825387 - Flags: review?(chevobbe.nicolas) → review+
Comment on attachment 8808553 [details]
Bug 1315922 - fix react/prop-types issues in client/responsive.html;

https://reviewboard.mozilla.org/r/91366/#review104254

Thanks, looks good!
Attachment #8808553 - Flags: review?(jryans) → review+
Comment on attachment 8808554 [details]
Bug 1315922 - enable react/prop-types eslint rule for devtools;

https://reviewboard.mozilla.org/r/91368/#review104252

Thanks, seems good to have this enabled.

::: devtools/.eslintrc.js:59
(Diff revision 2)
>      "react/no-did-mount-set-state": "error",
>      "react/no-did-update-set-state": "error",
>      "react/no-direct-mutation-state": "error",
>      "react/no-unknown-property": "error",
>      "react/prefer-es6-class": ["warn", "never"],
>      // Disabled temporarily until errors are fixed.

Seems like this comment should be removed since you're doing this work! :)
Attachment #8808554 - Flags: review?(jryans) → review+
Comment on attachment 8825390 [details]
Bug 1315922 - fix react/prop-types issues in client/netmonitor;

https://reviewboard.mozilla.org/r/103566/#review104908

LGTM!
Attachment #8825390 - Flags: review?(odvarko) → review+
Comment on attachment 8825388 [details]
Bug 1315922 - fix react/prop-types issues in client/memory;

https://reviewboard.mozilla.org/r/103562/#review105928

It all looks reasonable to me! This will be nice to have, thanks.
Attachment #8825388 - Flags: review?(gtatum) → review+
Thanks for the reviews everyone! Rebased + one last try push before landing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6351f58115494eed5c4c6759b9f1482e9c458bf6
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/62adfbcebf9d
reps: avoid using props as variable name in react classes;r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/dcc242748505
fix react/prop-types issues in client/webconsole;r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/869ab0083632
fix react/prop-types issues in client/netmonitor;r=Honza
https://hg.mozilla.org/integration/autoland/rev/911f62d371e7
fix react/prop-types issues in client/responsive.html;r=jryans
https://hg.mozilla.org/integration/autoland/rev/62c0f953fd6d
fix react/prop-types issues in client/memory;r=gregtatum
https://hg.mozilla.org/integration/autoland/rev/56150dbd34b3
enable react/prop-types eslint rule for devtools;r=jryans
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.