Skip to content

Commit

Permalink
Change eslint configuration and fix resulting issues (#4423)
Browse files Browse the repository at this point in the history
* Remove app/service/query-string (unused) and its dependency.

* Fix usage of mixed operators.

* eslint --fix fixes for missing dependencies for react hooks

* Fix: useCallback dependency passed to $http's .catch.

* Satisfy react/no-direct-mutation-state.

* Fix no-mixed-operators violations.

* Move the decision of whether to render Custom chart one level up to make sure hooks are called in the same order.

* Fix: name was undefined. It wasn't detected before because there is such global.

* Simplify eslint config and switch to creat-react-app's eslint base.

* Add prettier config.

* Make sure eslint doesn't conflict with prettier

* A few updates post eslint (#4425)

* Prettier command in package.json
  • Loading branch information
arikfr authored Dec 11, 2019
1 parent 0385b6f commit 1b9b303
Show file tree
Hide file tree
Showing 31 changed files with 844 additions and 618 deletions.
1 change: 1 addition & 0 deletions client/.eslintignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
build/*.js
dist
config/*.js
client/dist
51 changes: 1 addition & 50 deletions client/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,66 +1,17 @@
module.exports = {
root: true,
extends: ["airbnb", "plugin:compat/recommended"],
extends: ["react-app", "plugin:compat/recommended", "prettier"],
plugins: ["jest", "compat", "no-only-tests"],
settings: {
"import/resolver": "webpack"
},
parser: "babel-eslint",
env: {
browser: true,
node: true
},
rules: {
// allow debugger during development
"no-debugger": process.env.NODE_ENV === "production" ? 2 : 0,
"no-param-reassign": 0,
"no-mixed-operators": 0,
"no-underscore-dangle": 0,
"no-use-before-define": ["error", "nofunc"],
"prefer-destructuring": "off",
"prefer-template": "off",
"no-restricted-properties": "off",
"no-restricted-globals": "off",
"no-multi-assign": "off",
"no-lonely-if": "off",
"consistent-return": "off",
"no-control-regex": "off",
"no-multiple-empty-lines": "warn",
"no-only-tests/no-only-tests": "error",
"operator-linebreak": "off",
"react/destructuring-assignment": "off",
"react/jsx-filename-extension": "off",
"react/jsx-one-expression-per-line": "off",
"react/jsx-uses-react": "error",
"react/jsx-uses-vars": "error",
"react/jsx-wrap-multilines": "warn",
"react/no-access-state-in-setstate": "warn",
"react/prefer-stateless-function": "warn",
"react/forbid-prop-types": "warn",
"react/prop-types": "warn",
"jsx-a11y/anchor-is-valid": "off",
"jsx-a11y/click-events-have-key-events": "off",
"jsx-a11y/label-has-associated-control": [
"warn",
{
controlComponents: true
}
],
"jsx-a11y/label-has-for": "off",
"jsx-a11y/no-static-element-interactions": "off",
"max-len": [
"error",
120,
2,
{
ignoreUrls: true,
ignoreComments: false,
ignoreRegExpLiterals: true,
ignoreStrings: true,
ignoreTemplateLiterals: true
}
],
"no-else-return": ["error", { allowElseIf: true }],
"object-curly-newline": ["error", { consistent: true }]
}
};
4 changes: 2 additions & 2 deletions client/app/components/EditParameterSettingsDialog.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,13 @@ function EditParameterSettingsDialog(props) {

// fetch query by id
useEffect(() => {
const { queryId } = props.parameter;
const queryId = props.parameter.queryId;
if (queryId) {
Query.get({ id: queryId }, (query) => {
setInitialQuery(query);
});
}
}, []);
}, [props.parameter.queryId]);

function isFulfilled() {
// name
Expand Down
57 changes: 6 additions & 51 deletions client/app/components/QuerySelector.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,15 @@ import React, { useState, useEffect } from 'react';
import PropTypes from 'prop-types';
import cx from 'classnames';
import { react2angular } from 'react2angular';
import { debounce, find } from 'lodash';
import { find } from 'lodash';
import Input from 'antd/lib/input';
import Select from 'antd/lib/select';
import { Query } from '@/services/query';
import notification from '@/services/notification';
import { QueryTagsControl } from '@/components/tags-control/TagsControl';
import useSearchResults from '@/lib/hooks/useSearchResults';

const SEARCH_DEBOUNCE_DURATION = 200;
const { Option } = Select;

class StaleSearchError extends Error {
constructor() {
super('stale search');
}
}

function search(term) {
// get recent
if (!term) {
Expand All @@ -34,61 +27,23 @@ function search(term) {
}

export function QuerySelector(props) {
const [searchTerm, setSearchTerm] = useState();
const [searching, setSearching] = useState();
const [searchResults, setSearchResults] = useState([]);
const [searchTerm, setSearchTerm] = useState('');
const [selectedQuery, setSelectedQuery] = useState();
const [doSearch, searchResults, searching] = useSearchResults(search, { initialResults: [] });

let isStaleSearch = false;
const debouncedSearch = debounce(_search, SEARCH_DEBOUNCE_DURATION);
const placeholder = 'Search a query by name';
const clearIcon = <i className="fa fa-times hide-in-percy" onClick={() => selectQuery(null)} />;
const spinIcon = <i className={cx('fa fa-spinner fa-pulse hide-in-percy', { hidden: !searching })} />;

useEffect(() => { doSearch(searchTerm); }, [doSearch, searchTerm]);

// set selected from prop
useEffect(() => {
if (props.selectedQuery) {
setSelectedQuery(props.selectedQuery);
}
}, [props.selectedQuery]);

// on search term changed, debounced
useEffect(() => {
// clear results, no search
if (searchTerm === null) {
setSearchResults(null);
return () => {};
}

// search
debouncedSearch(searchTerm);
return () => {
debouncedSearch.cancel();
isStaleSearch = true;
};
}, [searchTerm]);

function _search(term) {
setSearching(true);
search(term)
.then(rejectStale)
.then((results) => {
setSearchResults(results);
setSearching(false);
})
.catch((err) => {
if (!(err instanceof StaleSearchError)) {
setSearching(false);
}
});
}

function rejectStale(results) {
return isStaleSearch
? Promise.reject(new StaleSearchError())
: Promise.resolve(results);
}

function selectQuery(queryId) {
let query = null;
if (queryId) {
Expand Down
2 changes: 1 addition & 1 deletion client/app/components/TimeAgo.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export function TimeAgo({ date, placeholder, autoUpdate }) {
const timer = setInterval(forceUpdate, 30 * 1000);
return () => clearInterval(timer);
}
}, [autoUpdate]);
}, [autoUpdate, forceUpdate]);

return (
<Tooltip title={title}>
Expand Down
2 changes: 1 addition & 1 deletion client/app/components/Timer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export function Timer({ from }) {
useEffect(() => {
const timer = setInterval(forceUpdate, 1000);
return () => clearInterval(timer);
}, []);
}, [forceUpdate]);

const diff = moment.now() - startTime;
const format = diff > 1000 * 60 * 60 ? 'HH:mm:ss' : 'mm:ss'; // no HH under an hour
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ export default function FavoritesDropdown({ fetch, urlTemplate }) {
}, [fetch]);

// fetch items on init
useEffect(() => fetchItems(false), []);
useEffect(() => {
fetchItems(false);
}, [fetchItems]);

// fetch items on click
const onVisibleChange = visible => visible && fetchItems();
Expand Down
12 changes: 6 additions & 6 deletions client/app/components/dynamic-form/DynamicForm.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,22 +58,22 @@ class DynamicForm extends React.Component {

const hasFilledExtraField = some(props.fields, (field) => {
const { extra, initialValue } = field;
return extra && (!isEmpty(initialValue) || isNumber(initialValue) || isBoolean(initialValue) && initialValue);
return extra && (!isEmpty(initialValue) || isNumber(initialValue) || (isBoolean(initialValue) && initialValue));
});

const inProgressActions = {};
props.actions.forEach(action => inProgressActions[action.name] = false);

this.state = {
isSubmitting: false,
inProgressActions: [],
showExtraFields: hasFilledExtraField,
inProgressActions
};

this.actionCallbacks = this.props.actions.reduce((acc, cur) => ({
...acc,
[cur.name]: cur.callback,
}), null);

props.actions.forEach((action) => {
this.state.inProgressActions[action.name] = false;
});
}

setActionInProgress = (actionName, inProgress) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ function useGrantees(url) {

const addPermission = useCallback((userId, accessType = 'modify') => $http.post(
url, { access_type: accessType, user_id: userId },
).catch(() => notification.error('Could not grant permission to the user'), [url]));
).catch(() => notification.error('Could not grant permission to the user')), [url]);

const removePermission = useCallback((userId, accessType = 'modify') => $http.delete(
url, { data: { access_type: accessType, user_id: userId } },
Expand Down Expand Up @@ -77,7 +77,7 @@ function UserSelect({ onSelect, shouldShowUser }) {
useEffect(() => {
setLoadingUsers(true);
debouncedSearchUsers(searchTerm);
}, [searchTerm]);
}, [debouncedSearchUsers, searchTerm]);

return (
<Select
Expand Down Expand Up @@ -117,16 +117,16 @@ function PermissionsEditorDialog({ dialog, author, context, aclUrl }) {
.then(setGrantees)
.catch(() => notification.error('Failed to load grantees list'))
.finally(() => setLoadingGrantees(false));
}, []);
}, [loadGrantees]);

const userHasPermission = useCallback(
user => (user.id === author.id || !!get(find(grantees, { id: user.id }), 'accessType')),
[grantees],
[author.id, grantees],
);

useEffect(() => {
loadUsersWithPermissions();
}, [aclUrl]);
}, [aclUrl, loadUsersWithPermissions]);

return (
<Modal
Expand Down
2 changes: 1 addition & 1 deletion client/app/components/users/ChangePasswordDialog.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class ChangePasswordDialog extends React.Component {
notification.success('Saved.');
this.props.dialog.close({ success: true });
}, (error = {}) => {
notification.error(error.data && error.data.message || 'Failed saving.');
notification.error((error.data && error.data.message) || 'Failed saving.');
this.setState({ updatingPassword: false });
});
} else {
Expand Down
2 changes: 1 addition & 1 deletion client/app/components/users/UserEdit.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export default class UserEdit extends React.Component {
successCallback('Saved.');
this.setState({ user: User.convertUserInfo(user) });
}, (error = {}) => {
errorCallback(error.data && error.data.message || 'Failed saving.');
errorCallback((error.data && error.data.message) || 'Failed saving.');
});
};

Expand Down
2 changes: 1 addition & 1 deletion client/app/lib/hooks/useQueryResult.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ function getQueryResultData(queryResult) {

export default function useQueryResult(queryResult) {
const [data, setData] = useState(getQueryResultData(queryResult));
let isCancelled = false;
useEffect(() => {
let isCancelled = false;
if (queryResult) {
queryResult.toPromise()
.then(() => {
Expand Down
13 changes: 6 additions & 7 deletions client/app/lib/hooks/useSearchResults.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useState, useEffect } from 'react';
import { useState, useEffect, useRef } from 'react';
import { useDebouncedCallback } from 'use-debounce';

export default function useSearchResults(
Expand All @@ -7,17 +7,16 @@ export default function useSearchResults(
) {
const [result, setResult] = useState(initialResults);
const [isLoading, setIsLoading] = useState(false);

let currentSearchTerm = null;
let isDestroyed = false;
const currentSearchTerm = useRef(null);
const isDestroyed = useRef(false);

const [doSearch] = useDebouncedCallback((searchTerm) => {
setIsLoading(true);
currentSearchTerm = searchTerm;
currentSearchTerm.current = searchTerm;
fetch(searchTerm)
.catch(() => null)
.then((data) => {
if ((searchTerm === currentSearchTerm) && !isDestroyed) {
if ((searchTerm === currentSearchTerm.current) && !isDestroyed.current) {
setResult(data);
setIsLoading(false);
}
Expand All @@ -26,7 +25,7 @@ export default function useSearchResults(

useEffect(() => (
// ignore all requests after component destruction
() => { isDestroyed = true; }
() => { isDestroyed.current = true; }
), []);

return [doSearch, result, isLoading];
Expand Down
2 changes: 1 addition & 1 deletion client/app/pages/alert/components/AlertDestinations.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export default class AlertDestinations extends React.Component {
return {
content: (
<div className="destination-wrapper">
<img src={`${IMG_ROOT}/${item.type}.png`} className="destination-icon" alt={name} />
<img src={`${IMG_ROOT}/${item.type}.png`} className="destination-icon" alt={item.name} />
<span className="flex-fill">{item.name}</span>
<ListItemAddon isSelected={isSelected} alreadyInGroup={alreadyInGroup} deselectedIcon="fa-plus" />
</div>
Expand Down
2 changes: 1 addition & 1 deletion client/app/pages/alert/components/MenuButton.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export default function MenuButton({ doDelete, canEdit, mute, unmute, muted }) {
maskClosable: true,
autoFocusButton: null,
});
}, []);
}, [doDelete]);

return (
<Dropdown
Expand Down
2 changes: 1 addition & 1 deletion client/app/pages/alert/components/Rearm.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function RearmByDuration({ value, onChange, editMode }) {
break;
}
}
}, []);
}, [value]);

if (!isNumber(count) || !isNumber(durationIdx)) {
return null;
Expand Down
2 changes: 1 addition & 1 deletion client/app/pages/home/Home.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ function FavoriteList({ title, resource, itemUrl, emptyState }) {
resource.favorites().$promise
.then(({ results }) => setItems(results))
.finally(() => setLoading(false));
}, []);
}, [resource]);

return (
<>
Expand Down
11 changes: 0 additions & 11 deletions client/app/services/query-string.js

This file was deleted.

2 changes: 1 addition & 1 deletion client/app/visualizations/EditVisualizationDialog.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ function EditVisualizationDialog({ dialog, visualization, query, queryResult })
originalOptions: options,
};
},
[visualization],
[data, isNew, visualization],
);

const [type, setType] = useState(defaultState.type);
Expand Down
Loading

0 comments on commit 1b9b303

Please sign in to comment.