Enable ESLint react/prop-types rule for devtools

RESOLVED FIXED in Firefox 53

Status

P3
normal
RESOLVED FIXED
2 years ago
2 months ago

People

(Reporter: jdescottes, Assigned: jdescottes)

Tracking

Trunk
Firefox 53
Dependency tree / graph

Firefox Tracking Flags

(firefox52 affected, firefox53 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(6 attachments)

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
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
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
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
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 12

2 years ago
mozreview-review
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 13

2 years ago
mozreview-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 14

2 years ago
mozreview-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 15

2 years ago
mozreview-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 16

2 years ago
mozreview-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 hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 23

2 years ago
mozreview-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+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 37

2 years ago
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

Updated

2 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.