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

A few updates post eslint #4425

Merged
merged 4 commits into from
Dec 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change needed for eslint to detect the correct value for the dependencies array?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, using it like this it automatically corrects to [props.parameter.queryId]

if (queryId) {
Query.get({ id: queryId }, (query) => {
setInitialQuery(query);
});
}
}, [props.parameter]);
}, [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
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), [fetchItems]);
useEffect(() => {
fetchItems(false);
}, [fetchItems]);

// fetch items on click
const onVisibleChange = visible => visible && fetchItems();
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/visualizations/choropleth/Renderer/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export default function Renderer({ data, options, onOptionsChange }) {
options, // detect changes for all options except bounds, but pass them all!
);
}
}, [map, geoJson, data, optionsWithoutBounds, options]);
}, [map, geoJson, data, optionsWithoutBounds]); // eslint-disable-line react-hooks/exhaustive-deps

useEffect(() => {
if (map) {
Expand Down
3 changes: 2 additions & 1 deletion client/app/visualizations/funnel/Renderer/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ function generateRowKeyPrefix() {

export default function Renderer({ data, options }) {
const funnelData = useMemo(() => prepareData(data.rows, options), [data, options]);
const rowKeyPrefix = useMemo(() => generateRowKeyPrefix(), []);
// eslint-disable-next-line react-hooks/exhaustive-deps
const rowKeyPrefix = useMemo(() => generateRowKeyPrefix(), [funnelData]);

const formatValue = useMemo(() => createNumberFormatter(options.numberFormat), [options.numberFormat]);

Expand Down