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

finish linting errors for no-restricted-syntax. 1 remains: couldn't r…

…esolve removing iterator loops for urlSearchParams
  • Loading branch information
IAmThePan committed May 12, 2020
commit 3d50aff04aeeef662eaad1d68504e2d044144f79
@@ -66,7 +66,7 @@ module.exports = {
}],
'no-plusplus': [0],
'no-prototype-builtins': [1],
'no-restricted-syntax': [0], // TODO: enable this check
'no-restricted-syntax': [1],
This conversation was marked as resolved by wlycdgr

This comment has been minimized.

@christophertino

christophertino Jun 4, 2020
Member

Remove 'no-restricted-syntax'

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

'no-tabs': [0],
'no-underscore-dangle': [0],
'no-unused-vars': [1],
@@ -34,7 +34,6 @@ import {
updateTrackerBlocked, updateCategoryBlocked, updateBlockAllTrackers, toggleExpandAll
} from '../utils/blocking';
import { sendMessage } from '../utils/msg';
import { objectEntries } from '../../../src/utils/common';

const initialState = {
expand_all_trackers: false,
@@ -195,8 +194,10 @@ const _importSettingsDialog = (state, action) => {
const _importSettingsNative = (state, action) => {
const { settings } = action;
const updated_state = {};
// eslint-disable-next-line prefer-const
for (let [key, value] of objectEntries(settings)) {
const settingsKeys = Object.keys(settings);
for (let i = 0; i < settingsKeys.length; i++) {
const key = settingsKeys[i];
let value = settings[key];
This conversation was marked as resolved by wlycdgr
Comment on lines +198 to +200

This comment has been minimized.

@wlycdgr

wlycdgr Jun 11, 2020
Member

We could get rid of the loop altogether, no?

const updated_state = { ...settings };
updated_state['alert_bubble_timeout'] = Math.min(settings['alert_bubble_timeout'], 30);

This comment has been minimized.

@IAmThePan

IAmThePan Jun 12, 2020
Author Contributor

Much more elegant. Updated on feature/cleanup-cleanup.

This comment has been minimized.

@wlycdgr

wlycdgr Jun 15, 2020
Member

lgtm on feature/cleanup-cleanup

if (key === 'alert_bubble_timeout') {
value = (value > 30) ? 30 : value;
}
@@ -40,7 +40,8 @@ class Account {
const opts = {
errorHandler: errors => (
new Promise((resolve, reject) => {
for (const err of errors) {
for (let i = 0; i < errors.length; i++) {
const err = errors[i];
This conversation was marked as resolved by wlycdgr
Comment on lines +43 to +44

This comment has been minimized.

@wlycdgr

wlycdgr Jun 11, 2020
Member

As above, how come we are not using forEach here? Or is some Airbnb rule other than 11.1 (which suggests this) behind this change?

This comment has been minimized.

@IAmThePan

IAmThePan Jun 12, 2020
Author Contributor

I can't update this loop to forEach because you can't break out of a forEach and return without doing something unnatural with errors. I could do .some but it's not as readable. Keeping as is for now.

switch (err.code) {
case '10020': // token is not valid
case '10060': // user id does not match
@@ -346,10 +347,12 @@ class Account {

// check scopes
if (userScopes.indexOf('god') >= 0) { return true; }
for (const sArr of required) {
for (let i = 0; i < required.length; i++) {
const sArr = required[i];
This conversation was marked as resolved by wlycdgr
Comment on lines +350 to +351

This comment has been minimized.

@wlycdgr

wlycdgr Jun 11, 2020
Member

Same forEach question

This comment has been minimized.

@IAmThePan

IAmThePan Jun 12, 2020
Author Contributor

Same as above. This loop uses a break which you can't elegantly cause with a forEach loop. Keeping it as is for now.

let matches = true;
if (sArr.length > 0) {
for (const s of sArr) {
for (let j = 0; j < sArr.length; j++) {
const s = sArr[j];
This conversation was marked as resolved by wlycdgr
Comment on lines +354 to +355

This comment has been minimized.

@wlycdgr

wlycdgr Jun 11, 2020
Member

And here

This comment has been minimized.

@IAmThePan

IAmThePan Jun 12, 2020
Author Contributor

Same reasoning as above.

if (userScopes.indexOf(s) === -1) {
matches = false;
break;
@@ -84,7 +84,9 @@ class BugDb extends Updatable {
const categoryArray = [];
const categories = {};

for (appId in db.apps) {
const appIds = Object.keys(db.apps);
for (let i = 0; i < appIds.length; i++) {
This conversation was marked as resolved by wlycdgr
Comment on lines +87 to +88

This comment has been minimized.

@wlycdgr

wlycdgr Jun 11, 2020
Member

Same question re: forEach. I won't mention it again so as not to clutter up the PR more - if it does make sense to change the other instances should be easy to find.

This comment has been minimized.

@IAmThePan

IAmThePan Jun 12, 2020
Author Contributor

Same as the others. I didn't update this one because the loop uses a continue.

appId = appIds[i];
if (Object.prototype.hasOwnProperty.call(db.apps, appId)) {
category = db.apps[appId].cat;
if (t(`category_${category}`) === `category_${category}`) {
@@ -181,7 +183,9 @@ class BugDb extends Updatable {

log('initializing bugdb regexes...');

for (const id in regexes) {
const regexesKeys = Object.keys(regexes);
for (let i = 0; i < regexesKeys.length; i++) {
const id = regexesKeys[i];
if (Object.prototype.hasOwnProperty.call(regexes, id)) {
db.patterns.regex[id] = new RegExp(regexes[id], 'i');
}
@@ -68,9 +68,11 @@ class Click2PlayDb extends Updatable {
reset(tab_id) {
if (!Object.prototype.hasOwnProperty.call(this.allowOnceList, tab_id)) { return; }

const entries = Object.entries(this.allowOnceList[tab_id]);
let keep = false;
for (const [appID, count] of entries) {
const allowKeys = Object.keys(this.allowOnceList[tab_id]);
for (let i = 0; i < allowKeys.length; i++) {
const appID = allowKeys[i];
const count = this.allowOnceList[tab_id][appID];
const newCount = count - 1;
this.allowOnceList[tab_id][appID] = newCount;
if (newCount > 0) {
@@ -29,7 +29,7 @@ import dispatcher from './Dispatcher';
import promoModals from './PromoModals';
import { getCliqzGhosteryBugs, sendCliqzModuleCounts } from '../utils/cliqzModulesData';
import { getActiveTab, flushChromeMemoryCache, processUrl } from '../utils/utils';
import { objectEntries, log } from '../utils/common';
import { log } from '../utils/common';

const SYNC_SET = new Set(globals.SYNC_ARRAY);
const { IS_CLIQZ } = globals;
@@ -586,8 +586,10 @@ class PanelData {
}

// Set the conf from data
// TODO can this now be replaced by Object.entries?
for (const [key, value] of objectEntries(data)) {
const dataKeys = Object.keys(data);
for (let i = 0; i < dataKeys.length; i++) {
const key = dataKeys[i];
const value = data[key];
if (Object.prototype.hasOwnProperty.call(conf, key) && !isEqual(conf[key], value)) {
conf[key] = value;
syncSetDataChanged = SYNC_SET.has(key) ? true : syncSetDataChanged;
@@ -58,15 +58,17 @@ export function getCliqzData(tabId, tabHostUrl, antiTracking) {
return tracker.ads;
};

for (const bug of bugsValues) {
for (let i = 0; i < bugsValues.length; i++) {
const bug = bugsValues[i];
const dataPoints = getDataPoints(bug);
if (dataPoints) {
totalUnsafeCount += dataPoints;
trackerCount++;
}
}

for (const other of othersValues) {
for (let i = 0; i < othersValues.length; i++) {
const other = othersValues[i];
let whitelisted = false;
const dataPoints = getDataPoints(other);

@@ -157,25 +157,6 @@ export function hashCode(str) {
return hash;
}

/**
* Generator which makes object iterable with for...of loop
* @memberOf BackgroundUtils
*
* @param {Object} object over which own enumerable properties we want to iterate
* @return {Object} Generator object
*/

export function* objectEntries(obj) {
const propKeys = Object.keys(obj);

for (const propKey of propKeys) {
// `yield` returns a value and then pauses
// the generator. Later, execution continues
// where it was previously paused.
yield [propKey, obj[propKey]];
}
}

/**
* Unescape base64-encoded string.
* @private
@@ -22,7 +22,7 @@ import { debounce } from 'underscore';
import { URL } from '@cliqz/url-parser';
import tabInfo from '../classes/TabInfo';
import globals from '../classes/Globals';
import { log, objectEntries } from './common';
import { log } from './common';

const { BROWSER_INFO } = globals;
const IS_FIREFOX = (BROWSER_INFO.name === 'firefox');
@@ -355,7 +355,10 @@ function _fetchJson(method, url, query, extraHeaders, referrer = 'no-referrer',
Accept: 'application/json'
});
if (extraHeaders) {
for (const [key, value] of objectEntries(extraHeaders)) {
const extraHeadersKeys = Object.keys(extraHeaders);
for (let i = 0; i < extraHeadersKeys.length; i++) {
const key = extraHeadersKeys[i];
const value = extraHeaders[key];
headers.append(key, value);
}
}
@@ -448,7 +451,10 @@ function _fetchJson(method, url, query, extraHeaders, referrer = 'no-referrer',
xhr.setRequestHeader('Content-Type', 'application/json');
xhr.setRequestHeader('Accept', 'application/json');
if (extraHeaders) {
for (const [key, value] of objectEntries(extraHeaders)) {
const extraHeadersKeys = Object.keys(extraHeaders);
for (let i = 0; i < extraHeadersKeys.length; i++) {
const key = extraHeadersKeys[i];
const value = extraHeaders[key];
xhr.setRequestHeader(key, value);
}
}
@@ -47,7 +47,7 @@ describe('src/classes/FoundBugs.js', () => {
"614": {"name": "New Relic","cat": "site_analytics","tags": [48]}},
"bugs": {"2": {"aid": 13},"935": {"aid": 13},"1982": {"aid": 13},"719": {"aid": 464},"1009": {"aid": 614}},
"firstPartyExceptions": {'something': true},
"patterns": {'something': true},
"patterns": { regex: { 'something': true} },
"version":416
});
// Mock bugDb fetch response
@@ -19,6 +19,7 @@ describe('src/utils/matcher.js', () => {
beforeAll(done => {
// Fake the XMLHttpRequest for fetchJson(/daabases/bugs.json)
const bugsJson = JSON.stringify({
"apps": {},
"firstPartyExceptions": {
"101": [
"google.com/ig"
@@ -32,6 +33,7 @@ describe('src/utils/matcher.js', () => {
]
},
"patterns": {
"regex": {},
"host": {
"com": {
"gmodules": {
@@ -62,11 +62,12 @@ const leet_convert = function(string) {
// 'z': 'z',
};

let letter;
let output = string || '';
output = output.replace(/cks/g, 'x');

for (letter in characterMap) {
const characterKeys = Object.keys(characterMap);
for (let i = 0; i < characterKeys.length; i++) {
const letter = characterKeys[i];
if (Object.prototype.hasOwnProperty.call(characterMap, letter)) {
output = output.replace(new RegExp(letter, 'g'), characterMap[letter]);
}
@@ -82,11 +83,12 @@ if (!fs.existsSync('./tools/leet/messages.en.copy.json')) {

// Import the copied messages file
const leet = {};
let key;
const en = jsonfile.readFileSync('./tools/leet/messages.en.copy.json');

// Create a LEETed version of the messages.json file
for (key in en) {
const enKeys = Object.keys(en);
for (let i = 0; i < enKeys.length; i++) {
const key = enKeys[i];
if (Object.prototype.hasOwnProperty.call(en[key], 'message')) {
const message = leet_convert(en[key].message);
const { placeholders } = en[key];
ProTip! Use n and p to navigate between commits in a pull request.