Skip to content

Commit

Permalink
[Logs + Metrics UI] Remove eslint exceptions (#50979)
Browse files Browse the repository at this point in the history
This removes the two eslint exceptions specific to the `infra` plugin introduced in #49244.

fixes #49563
  • Loading branch information
weltenwort authored Dec 11, 2019
1 parent 9fcc934 commit 0cd62ca
Show file tree
Hide file tree
Showing 41 changed files with 269 additions and 231 deletions.
7 changes: 0 additions & 7 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,13 +170,6 @@ module.exports = {
'react-hooks/rules-of-hooks': 'off',
},
},
{
files: ['x-pack/legacy/plugins/infra/**/*.{js,ts,tsx}'],
rules: {
'react-hooks/exhaustive-deps': 'off',
'react-hooks/rules-of-hooks': 'off',
},
},
{
files: ['x-pack/legacy/plugins/lens/**/*.{js,ts,tsx}'],
rules: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ export const useFormattedTime = (

const dateFormat = formatMap[format];
const formattedTime = useMemo(() => getFormattedTime(time, dateFormat, fallbackFormat), [
getFormattedTime,
time,
dateFormat,
fallbackFormat,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const LogEntryActionsMenu: React.FunctionComponent<{
/>
</EuiContextMenuItem>,
],
[uptimeLink]
[apmLink, uptimeLink]
);

const hasMenuItems = useMemo(() => menuItems.length > 0, [menuItems]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';
import { debounce } from 'lodash';
import React, { useCallback, useEffect, useMemo, useState } from 'react';
import React, { useCallback, useMemo, useState } from 'react';

import euiStyled from '../../../../../common/eui_styled_components';
import { useVisibilityState } from '../../utils/use_visibility_state';
Expand Down Expand Up @@ -47,8 +47,25 @@ export const LogHighlightsMenu: React.FC<LogHighlightsMenuProps> = ({
} = useVisibilityState(false);

// Input field state
const [highlightTerm, setHighlightTerm] = useState('');
const [highlightTerm, _setHighlightTerm] = useState('');

const debouncedOnChange = useMemo(() => debounce(onChange, 275), [onChange]);
const setHighlightTerm = useCallback<typeof _setHighlightTerm>(
valueOrUpdater =>
_setHighlightTerm(previousHighlightTerm => {
const newHighlightTerm =
typeof valueOrUpdater === 'function'
? valueOrUpdater(previousHighlightTerm)
: valueOrUpdater;

if (newHighlightTerm !== previousHighlightTerm) {
debouncedOnChange([newHighlightTerm]);
}

return newHighlightTerm;
}),
[debouncedOnChange]
);
const changeHighlightTerm = useCallback(
e => {
const value = e.target.value;
Expand All @@ -57,9 +74,6 @@ export const LogHighlightsMenu: React.FC<LogHighlightsMenuProps> = ({
[setHighlightTerm]
);
const clearHighlightTerm = useCallback(() => setHighlightTerm(''), [setHighlightTerm]);
useEffect(() => {
debouncedOnChange([highlightTerm]);
}, [highlightTerm]);

const button = (
<EuiButtonEmpty color="text" size="xs" iconType="brush" onClick={togglePopover}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export const useMeasuredCharacterDimensions = (scale: TextScale) => {
X
</MonospaceCharacterDimensionsProbe>
),
[scale]
[measureElement, scale]
);

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import { EuiComboBox } from '@elastic/eui';
import { i18n } from '@kbn/i18n';

import React, { useCallback, useState, useEffect } from 'react';
import React, { useCallback, useState } from 'react';
import { FieldType } from 'ui/index_patterns';
import { colorTransformer, MetricsExplorerColor } from '../../../common/color_palette';
import {
Expand All @@ -31,24 +31,19 @@ interface SelectedOption {

export const MetricsExplorerMetrics = ({ options, onChange, fields, autoFocus = false }: Props) => {
const colors = Object.keys(MetricsExplorerColor) as MetricsExplorerColor[];
const [inputRef, setInputRef] = useState<HTMLInputElement | null>(null);
const [focusOnce, setFocusState] = useState<boolean>(false);
const [shouldFocus, setShouldFocus] = useState(autoFocus);

useEffect(() => {
if (inputRef && autoFocus && !focusOnce) {
inputRef.focus();
setFocusState(true);
}
}, [inputRef]);
// the EuiCombobox forwards the ref to an input element
const autoFocusInputElement = useCallback(
(inputElement: HTMLInputElement | null) => {
if (inputElement && shouldFocus) {
inputElement.focus();
setShouldFocus(false);
}
},
[shouldFocus]
);

// I tried to use useRef originally but the EUIComboBox component's type definition
// would only accept an actual input element or a callback function (with the same type).
// This effectivly does the same thing but is compatible with EuiComboBox.
const handleInputRef = (ref: HTMLInputElement) => {
if (ref) {
setInputRef(ref);
}
};
const handleChange = useCallback(
selectedOptions => {
onChange(
Expand All @@ -59,7 +54,7 @@ export const MetricsExplorerMetrics = ({ options, onChange, fields, autoFocus =
}))
);
},
[options, onChange]
[onChange, options.aggregation, colors]
);

const comboOptions = fields
Expand All @@ -86,7 +81,7 @@ export const MetricsExplorerMetrics = ({ options, onChange, fields, autoFocus =
selectedOptions={selectedOptions}
onChange={handleChange}
isClearable={true}
inputRef={handleInputRef}
inputRef={autoFocusInputElement}
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export const SavedViewCreateModal = ({ close, save, isInvalid }: Props) => {

const saveView = useCallback(() => {
save(viewName, includeTime);
}, [viewName, includeTime]);
}, [includeTime, save, viewName]);

return (
<EuiOverlayMask>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export const AddLogColumnButtonAndPopover: React.FunctionComponent<{

addLogColumn(selectedOption.columnConfiguration);
},
[addLogColumn, availableColumnOptions]
[addLogColumn, availableColumnOptions, closePopover]
);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export const useSourceConfigurationFormState = (configuration?: SourceConfigurat
const resetForm = useCallback(() => {
indicesConfigurationFormState.resetForm();
logColumnsConfigurationFormState.resetForm();
}, [indicesConfigurationFormState.resetForm, logColumnsConfigurationFormState.formState]);
}, [indicesConfigurationFormState, logColumnsConfigurationFormState]);

const isFormDirty = useMemo(
() => indicesConfigurationFormState.isFormDirty || logColumnsConfigurationFormState.isFormDirty,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,28 +17,33 @@ import {
} from '../../graphql/types';
import { findInventoryModel } from '../../../common/inventory_models';

interface Props {
interface WaffleInventorySwitcherProps {
nodeType: InfraNodeType;
changeNodeType: (nodeType: InfraNodeType) => void;
changeGroupBy: (groupBy: InfraSnapshotGroupbyInput[]) => void;
changeMetric: (metric: InfraSnapshotMetricInput) => void;
}

export const WaffleInventorySwitcher = (props: Props) => {
export const WaffleInventorySwitcher: React.FC<WaffleInventorySwitcherProps> = ({
changeNodeType,
changeGroupBy,
changeMetric,
nodeType,
}) => {
const [isOpen, setIsOpen] = useState(false);
const closePopover = useCallback(() => setIsOpen(false), []);
const openPopover = useCallback(() => setIsOpen(true), []);
const goToNodeType = useCallback(
(nodeType: InfraNodeType) => {
(targetNodeType: InfraNodeType) => {
closePopover();
props.changeNodeType(nodeType);
props.changeGroupBy([]);
const inventoryModel = findInventoryModel(nodeType);
props.changeMetric({
changeNodeType(targetNodeType);
changeGroupBy([]);
const inventoryModel = findInventoryModel(targetNodeType);
changeMetric({
type: inventoryModel.metrics.defaultSnapshot as InfraSnapshotMetricType,
});
},
[props.changeGroupBy, props.changeNodeType, props.changeMetric]
[closePopover, changeNodeType, changeGroupBy, changeMetric]
);
const goToHost = useCallback(() => goToNodeType('host' as InfraNodeType), [goToNodeType]);
const goToK8 = useCallback(() => goToNodeType('pod' as InfraNodeType), [goToNodeType]);
Expand Down Expand Up @@ -68,10 +73,10 @@ export const WaffleInventorySwitcher = (props: Props) => {
],
},
],
[]
[goToDocker, goToHost, goToK8]
);
const selectedText = useMemo(() => {
switch (props.nodeType) {
switch (nodeType) {
case InfraNodeType.host:
return i18n.translate('xpack.infra.waffle.nodeTypeSwitcher.hostsLabel', {
defaultMessage: 'Hosts',
Expand All @@ -81,7 +86,7 @@ export const WaffleInventorySwitcher = (props: Props) => {
case InfraNodeType.container:
return 'Docker';
}
}, [props.nodeType]);
}, [nodeType]);

return (
<EuiFilterGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export const useLogAnalysisCapabilities = () => {

useEffect(() => {
fetchMlCapabilities();
}, []);
}, [fetchMlCapabilities]);

const isLoading = useMemo(() => fetchMlCapabilitiesRequest.state === 'pending', [
fetchMlCapabilitiesRequest.state,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,23 +125,23 @@ export const useLogAnalysisModule = <JobType extends string>({
dispatchModuleStatus({ type: 'failedSetup' });
});
},
[cleanUpModule, setUpModule]
[cleanUpModule, dispatchModuleStatus, setUpModule]
);

const viewSetupForReconfiguration = useCallback(() => {
dispatchModuleStatus({ type: 'requestedJobConfigurationUpdate' });
}, []);
}, [dispatchModuleStatus]);

const viewSetupForUpdate = useCallback(() => {
dispatchModuleStatus({ type: 'requestedJobDefinitionUpdate' });
}, []);
}, [dispatchModuleStatus]);

const viewResults = useCallback(() => {
dispatchModuleStatus({ type: 'viewedResults' });
}, []);
}, [dispatchModuleStatus]);

const jobIds = useMemo(() => moduleDescriptor.getJobIds(spaceId, sourceId), [
moduleDescriptor.getJobIds,
moduleDescriptor,
spaceId,
sourceId,
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export const useAnalysisSetupState = <JobType extends string>({
? [...errors, ...index.errors]
: errors;
}, []);
}, [selectedIndexNames, validatedIndices, validateIndicesRequest.state]);
}, [isValidating, validateIndicesRequest.state, selectedIndexNames, validatedIndices]);

return {
cleanupAndSetup,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export const useLogEntryHighlights = (
} else {
setLogEntryHighlights([]);
}
}, [highlightTerms, startKey, endKey, filterQuery, sourceVersion]);
}, [endKey, filterQuery, highlightTerms, loadLogEntryHighlights, sourceVersion, startKey]);

const logEntryHighlightsById = useMemo(
() =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,15 @@ export const useLogSummaryHighlights = (
} else {
setLogSummaryHighlights([]);
}
}, [highlightTerms, start, end, bucketSize, filterQuery, sourceVersion]);
}, [
bucketSize,
debouncedLoadSummaryHighlights,
end,
filterQuery,
highlightTerms,
sourceVersion,
start,
]);

return {
logSummaryHighlights,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export const useNextAndPrevious = ({
const initialTimeKey = getUniqueLogEntryKey(entries[initialIndex]);
setCurrentTimeKey(initialTimeKey);
}
}, [currentTimeKey, entries, setCurrentTimeKey]);
}, [currentTimeKey, entries, setCurrentTimeKey, visibleMidpoint]);

const indexOfCurrentTimeKey = useMemo(() => {
if (currentTimeKey && entries.length > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ export const LogHighlightsPositionBridge = withLogPosition(
const { setJumpToTarget, setVisibleMidpoint } = useContext(LogHighlightsState.Context);
useEffect(() => {
setVisibleMidpoint(visibleMidpoint);
}, [visibleMidpoint]);
}, [setVisibleMidpoint, visibleMidpoint]);

useEffect(() => {
setJumpToTarget(() => jumpToTargetPosition);
}, [jumpToTargetPosition]);
}, [jumpToTargetPosition, setJumpToTarget]);

return null;
}
Expand All @@ -41,7 +41,7 @@ export const LogHighlightsFilterQueryBridge = withLogFilter(

useEffect(() => {
setFilterQuery(serializedFilterQuery);
}, [serializedFilterQuery]);
}, [serializedFilterQuery, setFilterQuery]);

return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export const WithStreamItems: React.FunctionComponent<{
createLogEntryStreamItem(logEntry, logEntryHighlightsById[logEntry.gid] || [])
),

[logEntries.entries, logEntryHighlightsById]
[isAutoReloading, logEntries.entries, logEntries.isReloading, logEntryHighlightsById]
);

return children({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ export function useMetricsExplorerData(
}
setLoading(false);
})();

// TODO: fix this dependency list while preserving the semantics
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [options, source, timerange, signal, afterKey]);
return { error, loading, data };
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ function useStateWithLocalStorage<State>(
const [state, setState] = useState<State>(parseJsonOrDefault<State>(storageState, defaultState));
useEffect(() => {
localStorage.setItem(key, JSON.stringify(state));
}, [state]);
}, [key, state]);
return [state, setState];
}

Expand Down
Loading

0 comments on commit 0cd62ca

Please sign in to comment.