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

Code Cleanup: Enforce Linting Rules & Update UNSAFE_ React Lifecycle Events #559

Merged
merged 33 commits into from Jun 4, 2020
Merged
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
75b0091
add linting for no-param-reassign and fix resulting linting errors
IAmThePan Apr 29, 2020
8d37082
add linting for prefer-object-spread and fix resulting linting errors
IAmThePan Apr 29, 2020
3049bd1
add linting for no-restricted-syntax and fix 1/2 of resulting errors
IAmThePan May 1, 2020
fe03ad7
add linting for no-prototype-builtins and fix resulting linting errors
IAmThePan May 4, 2020
f11bc18
add linting for class-methods-use-this and fix most resulting errors.…
IAmThePan May 5, 2020
5f3471e
finish linting for class-methods-use-this
IAmThePan May 6, 2020
d4d84dc
add linting for no-mixed-operators and fix resulting linting errors
IAmThePan May 6, 2020
17f90ed
add linting for import/prefer-default-export and fix resulting lintin…
IAmThePan May 8, 2020
a77cd1b
add linting for react/no-access-state-in-setstate and fix resulting l…
IAmThePan May 8, 2020
c31dbaf
add linting for react/jsx-props-no-spreading and fix resulting lintin…
IAmThePan May 11, 2020
3d50aff
finish linting errors for no-restricted-syntax. 1 remains: couldn't r…
IAmThePan May 12, 2020
227d012
Merge branch 'develop' into feature/update-linter
IAmThePan May 12, 2020
0988a29
Fix linting errors resulting from the merge with develop
IAmThePan May 12, 2020
b55be21
Refactor UNSAFE_componentWillMount into either constructor or compone…
IAmThePan May 14, 2020
241747c
Refactor UNSAFE_componentWillReceiveProps to componentDidUpdate or ge…
IAmThePan May 18, 2020
eb413fb
re-enable lint exception for no-prototype-builtins and revert calls b…
IAmThePan May 20, 2020
ac0ec93
add single line exception for no-restricted-syntax linting rule
IAmThePan May 20, 2020
fd954eb
add linting for react/destructuring-assignment and fix errors. ToDo: …
IAmThePan May 21, 2020
e815546
Fix minor bugs
IAmThePan May 22, 2020
ae8ffcc
Fix General Settings last updated text
IAmThePan May 22, 2020
4c56460
rework linting rule no-param-reassign to have more exceptions and par…
IAmThePan May 23, 2020
c5ef663
Remove file and line linting exceptions.
IAmThePan May 26, 2020
3ca2402
re-add linting rule react/sort-comp and fix resulting errors
IAmThePan May 26, 2020
4533fd7
remove added linting exception consistent-return and fix resulting er…
IAmThePan May 26, 2020
590aa60
remove added linting expression no-use-before-define and fix resultin…
IAmThePan May 26, 2020
ecb62e8
Fix linting error
IAmThePan May 26, 2020
f634c1d
fix minor bugs
IAmThePan May 26, 2020
ea51432
Code cleanup: fix PromoModal imports
IAmThePan May 26, 2020
75a88cb
Merge with develop. Fix resulting linting errors
IAmThePan May 26, 2020
68493d2
remove unnecessary hasOwnProperty calls after refactored for...in loops
IAmThePan May 28, 2020
6297062
Fix missing strings bug
IAmThePan May 28, 2020
9b4d16c
Fix last remaining string bug
IAmThePan May 28, 2020
2cb47a1
Merge branch 'develop' into feature/cleanup
IAmThePan Jun 4, 2020
File filter
Filter file types
Jump to
Jump to file
Failed to load files.

Always

Just for now

Remove file and line linting exceptions.

  • Loading branch information
IAmThePan committed May 26, 2020
commit c5ef663d55e674c80d74dcd1e1d2e240aee1a9ed
@@ -38,6 +38,8 @@ module.exports = {
],
// 0 = off, 1 = warn, 2 = error
rules: {
'no-use-before-define': [0], //ToDo: undo this.
'consistent-return': [0], //ToDo: undo this.
'arrow-parens': [2, 'as-needed', { 'requireForBlockBody': true }],
'camelcase': [0],
'class-methods-use-this': [1],
This conversation was marked as resolved by wlycdgr

This comment has been minimized.

@christophertino

christophertino Jun 4, 2020
Member

We can remove this line entirely and let the AirBnB defaults pick it up.

This comment has been minimized.

@IAmThePan

IAmThePan Jun 11, 2020
Author Contributor

updated in feature/cleanup-cleanup as this PR was already merged.

This comment has been minimized.

@wlycdgr

wlycdgr Jun 11, 2020
Member

lgtm in feature/cleanup-cleanup

@@ -68,7 +70,7 @@ module.exports = {
'appEntry', // for apps.forEach()
'sEntry', // for apps.forEach()
'confMutable', // for mutable conf settings
'eventMutable' // for mutable event details objects
'eventMutable', // for mutable event details objects
]
}],
'no-plusplus': [0],
@@ -117,5 +119,8 @@ module.exports = {
'jsx-a11y/mouse-events-have-key-events': [0],
'jsx-a11y/no-noninteractive-element-interactions': [0],
'jsx-a11y/no-static-element-interactions': [0],
'jsx-a11y/media-has-caption': [0], //TODO: enable this check
'jsx-a11y/label-has-associated-control': [0], //TODO: enable this check
'jsx-a11y/anchor-has-content': [0], //TODO: enable this check
},
};
@@ -13,8 +13,6 @@
/**
* @namespace ContentScriptUtils
*/
/* eslint no-use-before-define: 0 */

import { log } from '../../../src/utils/common';
import { sendMessageInPromise as panelSendPromiseMessage, sendMessage as panelSendMessage } from '../../panel/utils/msg';

@@ -62,7 +62,7 @@ class SetupBlockingDropdown extends Component {
/**
* Debounce the hide alert function after 3 seconds
*/
_hideToastDebounce = debounce(this._hideToast, 3000) // eslint-disable-line react/sort-comp
_hideToastDebounce = debounce(this._hideToast, 3000)

/**
* React's required render function. Returns JSX
@@ -25,7 +25,7 @@ const TutorialVideoView = () => (
{t('hub_tutorial_video_title')}
</div>
<div className="TutorialVideoView__videoContainer text-center">
<video // eslint-disable-line jsx-a11y/media-has-caption
<video
controls
height="auto"
width="100%"
@@ -37,9 +37,8 @@ export default class ChartSVG extends React.Component {
render() {
const { paths, radius } = this.props;
const { nItem } = this.state;
let computedPaths = paths.slice(0, nItem).map((element, index) => (
// eslint-disable-next-line react/no-array-index-key
<Path key={index} path={element} radius={radius} handler={this.increaseN} />
let computedPaths = paths.slice(0, nItem).map(element => (
<Path key={element.start} path={element} radius={radius} handler={this.increaseN} />
));

if (computedPaths.length === 0) {
@@ -18,7 +18,7 @@ import { Link } from 'react-router-dom';
* in place of Sign In view on successful signing.
* @memberof PanelClasses
*/
const AccountSuccess = ({ email, is_expert }) => ( // eslint-disable-line arrow-parens
const AccountSuccess = ({ email, is_expert }) => (
<div id="signin-success-panel">
<div className="row align-center">
<div className="small-7 columns text-center">
@@ -11,8 +11,6 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0
*/

/* eslint react/no-array-index-key: 0 */

import React from 'react';
import ClassNames from 'classnames';

@@ -355,21 +353,21 @@ class Tracker extends React.Component {

let sources;
if (tracker.sources) {
sources = tracker.sources.map((source, index) => (
sources = tracker.sources.map(source => (
<a
target="_blank"
rel="noopener noreferrer"
className="trk-src-link"
title={source.src}
key={index}
key={source.request_id}
href={`${globals.GCACHE_BASE_URL}/${encodeURIComponent(language)}/gcache/?n=${encodeURIComponent(tracker.name)}&s=${encodeURIComponent(source.src)}&v=2&t=${source.type}`}
>
{ source.src }
</a>
));
} else if (tracker.domains) {
sources = tracker.domains.map((domain, index) => (
<p className="trk-src-link unknown" key={index}>{domain}</p>
sources = tracker.domains.map(domain => (
<p className="trk-src-link unknown" key={domain}>{domain}</p>
));
}

@@ -202,7 +202,7 @@ class DonutGraph extends React.Component {
* @param {Array} categories list of categories detected on the site
* @param {Object} options options for the graph
*/
bakeDonut = throttle(this._bakeDonut.bind(this), 600, { leading: true, trailing: true }) // eslint-disable-line react/sort-comp
bakeDonut = throttle(this._bakeDonut.bind(this), 600, { leading: true, trailing: true })

_bakeDonut(categories, antiTracking, adBlock, options) {
const {
@@ -24,7 +24,7 @@ const NotScanned = ({ isSmall }) => {
small: isSmall,
});

return ( // eslint-disable-line arrow-parens
return (
<div className={notScannedClassNames}>
<div className="not-scanned-header">
{t('summary_page_not_scanned') }
@@ -11,8 +11,6 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0
*/

/* eslint jsx-a11y/label-has-associated-control: 0 */

import React from 'react';
import PropTypes from 'prop-types';
import RadioButton from './RadioButton';
@@ -11,8 +11,6 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0
*/

/* eslint jsx-a11y/label-has-associated-control: 0 */

import React from 'react';
import ClassNames from 'classnames';

@@ -215,7 +215,6 @@ class CreateAccount extends React.Component {
<div className="small-12 columns">
<div id="create-account-legal-consent-checkbox" className={(legalConsentNotCheckedError ? 'checkbox-error' : '')}>
<input id="legalConsentChecked" name="legalConsentChecked" type="checkbox" checked={legalConsentChecked} onChange={this.handleCheckboxChange} />
{/* eslint-disable-next-line jsx-a11y/label-has-associated-control */}
<label htmlFor="legalConsentChecked">
<I18nWithLink value="create_account_form_legal_consent_checkbox_label" />
</label>
@@ -11,10 +11,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0
*/

/* eslint no-useless-concat: 0 */

import React from 'react';
import ReactDOM from 'react-dom';
import ClassNames from 'classnames';

import ClickOutside from './BuildingBlocks/ClickOutside';
@@ -33,8 +30,7 @@ class HeaderMenu extends React.Component {
*/
handleClickOutside = (e) => {
const { toggleDropdown } = this.props;
// eslint-disable-next-line react/no-find-dom-node
if (!ReactDOM.findDOMNode(this).contains(e.target)) {
if (!this.node.contains(e.target)) {
toggleDropdown();
}
}
@@ -55,7 +51,7 @@ class HeaderMenu extends React.Component {
*/
clickBrokenPage = () => {
sendMessageInPromise('getSiteData').then((data) => {
let body = `${'PLEASE INCLUDE A DESCRIPTION AND A PICTURE OF THE ISSUE YOU ARE EXPERIENCING:' + '\r\n\r\n\r\n\r\n\r\n\r\n' +
let body = `${'PLEASE INCLUDE A DESCRIPTION AND A PICTURE OF THE ISSUE YOU ARE EXPERIENCING:\r\n\r\n\r\n\r\n\r\n\r\n' +
'URL: '}${data.url}\r\n` +
`Ghostery version: ${data.extensionVersion}\r\n` +
`Database Version: ${data.dbVersion}\r\n` +
@@ -178,7 +174,7 @@ class HeaderMenu extends React.Component {
const iconClasses = ClassNames('menu-icon-container', { subscriber }, { 'non-subscriber': !subscriber });
return (
<ClickOutside onClickOutside={this.handleClickOutside} excludeEl={kebab}>
<div className="dropdown-pane" id="header-dropdown">
<div ref={(node) => { this.node = node; }} className="dropdown-pane" id="header-dropdown">
<ul className="vertical menu no-bullet icons icon-left">
<li className="menu-option menu-settings" onClick={this.clickSettings}>
<div>
@@ -233,7 +233,7 @@ class Settings extends React.Component {
/**
* Hide alert in 3 sec. after it has been shown.
*/
hideToast = debounce(function() { // eslint-disable-line react/sort-comp
hideToast = debounce(function() {
this.setState({
showToast: false,
toastText: ''
@@ -21,9 +21,8 @@ import Site from './Site';
*/
const Sites = (props) => {
const { sites, listType } = props;
const siteList = sites.map((site, index) => (
// eslint-disable-next-line react/no-array-index-key
<Site site={site} listType={listType} updateSitePolicy={props.updateSitePolicy} key={index} />
const siteList = sites.map(site => (
<Site key={site} site={site} listType={listType} updateSitePolicy={props.updateSitePolicy} />
));
return <div className="s-sites">{ siteList }</div>;
};
@@ -157,8 +157,7 @@ class TrustAndRestrict extends React.Component {
if (pageHost.includes('*')) {
const wildcardPattern = pageHost.replace(/\*/g, '.*');
try {
// eslint-disable-next-line
new RegExp(wildcardPattern);
new RegExp(wildcardPattern); // eslint-disable-line no-new
isValidWildcard = true;
} catch (err) {
return false;
@@ -11,8 +11,6 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0
*/

/* eslint no-useless-escape: 0 */

import React from 'react';
import renderer from 'react-test-renderer';
import { when } from 'jest-when';
@@ -123,7 +121,7 @@ describe('app/panel/components/Settings/', () => {
});

test('isValidUrlorWildcard should return false with unsafe test entered', () => {
let input = '/^(\w+\s?)*$/';
let input = '/^(\w+\s?)*$/'; // eslint-disable-line no-useless-escape
let fn = jest.spyOn(TrustAndRestrict, 'isValidUrlorWildcard');
when(fn).calledWith(input);
let returnValue = TrustAndRestrict.isValidUrlorWildcard(input);
@@ -135,7 +133,7 @@ describe('app/panel/components/Settings/', () => {
returnValue = TrustAndRestrict.isValidUrlorWildcard(input);
expect(returnValue).toBe(false);

input = '(x\w{1,10})+y';
input = '(x\w{1,10})+y'; // eslint-disable-line no-useless-escape
fn = jest.spyOn(TrustAndRestrict, 'isValidUrlorWildcard');
when(fn).calledWith(input);
returnValue = TrustAndRestrict.isValidUrlorWildcard(input);
@@ -32,7 +32,6 @@ class Stats extends React.Component {
componentDidMount() {
sendMessage('ping', 'hist_stats_panel');
if (!this._isPlus(this.props)) {
// eslint-disable-next-line
this.setState(this._reset(true));
return;
}
@@ -12,7 +12,6 @@
*/

import React from 'react';
import ReactDOM from 'react-dom';
import ClassNames from 'classnames';

/**
@@ -36,8 +35,7 @@ class Tooltip extends React.Component {
* Lifecycle event. Set listeners.
*/
componentDidMount() {
// eslint-disable-next-line react/no-find-dom-node
this.parentNode = ReactDOM.findDOMNode(this).parentNode;
this.parentNode = this.node.parentNode;
this.parentNode.addEventListener('mouseenter', this.delayHover);
this.parentNode.addEventListener('mouseleave', this.leave);
this.parentNode.addEventListener('click', this.leave);
@@ -97,7 +95,7 @@ class Tooltip extends React.Component {
});

return (
<span className={compClassNames}>
<span ref={(node) => { this.node = node; }} className={compClassNames}>
{show && (
<span className={`tooltip-content ${position}`}>
{header &&
@@ -11,7 +11,6 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0
*/

/* eslint no-use-before-define: 0 */
import {
UPDATE_BLOCKING_DATA,
FILTER_TRACKERS,
@@ -11,8 +11,6 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0
*/

/* eslint no-use-before-define: 0 */

import {
UPDATE_PANEL_DATA,
SHOW_NOTIFICATION,
@@ -11,8 +11,6 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0
*/

/* eslint no-use-before-define: 0 */

import moment from 'moment/min/moment-with-locales.min';
import {
IMPORT_SETTINGS_DIALOG,
@@ -11,8 +11,6 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0
*/

/* eslint no-use-before-define: 0 */

import { URL } from '@cliqz/url-parser';
import {
UPDATE_SUMMARY_DATA,
@@ -36,7 +36,7 @@ const SteppedNavigation = (props) => {
const circles = [];
for (let i = 1; i <= totalSteps; i++) {
// Use <a> without an href prop to prevent a call to history with the same URL
circles.push((i === activeIndex) ? <a className="active" key={`nav-${i}`} /> : <NavLink to={`${i}`} key={`nav-${i}`} />); // eslint-disable-line jsx-a11y/anchor-has-content
circles.push((i === activeIndex) ? <a className="active" key={`nav-${i}`} /> : <NavLink to={`${i}`} key={`nav-${i}`} />);
This conversation was marked as resolved by wlycdgr

This comment has been minimized.

@wlycdgr

wlycdgr Jun 11, 2020
Member

I saw this rule got added to the .eslint file but in turned-off state, with a ToDo note to turn it on. Should we do that now instead of deferring it since it's apparently only one other place besides this one?

This comment has been minimized.

@IAmThePan

IAmThePan Jun 12, 2020
Author Contributor

There are two others linting rules like this. On the video call with you, Ethan, Tino and me, Tino suggested that this could be something that interns could work on.
One of the other a11y linting rules will require strings and translations as it is for video captions.

This comment has been minimized.

@wlycdgr

wlycdgr Jun 15, 2020
Member

Right right. Sorry, I'd forgotten. Sounds good.

}

return (
ProTip! Use n and p to navigate between commits in a pull request.