From 990347912bb6da209bbfa708be4b3ca47e7a945b Mon Sep 17 00:00:00 2001 From: Dario Gieselaar Date: Fri, 2 Aug 2019 17:33:29 +0200 Subject: [PATCH] =?UTF-8?q?[7.3]=20[APM]=20Add=20react-hooks=20lint=20rule?= =?UTF-8?q?s=20for=20APM=20folder,=20fix=20dep=E2=80=A6=20(#42182)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [APM] Add react-hooks lint rules for APM folder, fix dependencies (#42129) * [APM] Add react-hooks lint rules for APM folder, fix dependencies Closes #42128. * Validate useFetcher dependencies as well * Add useFetcher hook; move eslint config to kibana eslint config * Fix type errors --- .eslintrc.js | 8 ++++++++ .../Settings/AddSettings/AddSettingFlyout.tsx | 9 +++++---- .../app/TransactionOverview/List/index.tsx | 3 +-- .../app/TransactionOverview/index.tsx | 20 +++++++++---------- .../components/shared/KueryBar/index.tsx | 10 +++++++--- .../public/context/UrlParamsContext/index.tsx | 12 ++++++----- .../plugins/apm/public/hooks/useFetcher.tsx | 12 +++++++++-- .../public/hooks/useTransactionBreakdown.ts | 2 +- .../apm/public/hooks/useTransactionCharts.ts | 2 +- 9 files changed, 50 insertions(+), 28 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 6791d2f18d9c..5acd0f9952df 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -381,6 +381,14 @@ module.exports = { 'no-console': ['warn', { allow: ['error'] }], }, }, + { + plugins: ['react-hooks'], + files: ['x-pack/legacy/plugins/apm/**/*.{ts,tsx}'], + rules: { + 'react-hooks/rules-of-hooks': 'error', // Checks rules of Hooks + 'react-hooks/exhaustive-deps': ['error', { additionalHooks: '^useFetcher$' }], + }, + }, /** * GIS overrides diff --git a/x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyout.tsx b/x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyout.tsx index 368d70e83b76..1726c2233d40 100644 --- a/x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyout.tsx +++ b/x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyout.tsx @@ -47,10 +47,6 @@ export function AddSettingsFlyout({ onSubmit, selectedConfig }: Props) { - if (!isOpen) { - return null; - } - const [environment, setEnvironment] = useState( selectedConfig ? selectedConfig.service.environment || ENVIRONMENT_NOT_DEFINED @@ -88,6 +84,11 @@ export function AddSettingsFlyout({ const hasCorrectDecimals = Number.isInteger(sampleRateFloat * 1000); const isSampleRateValid = sampleRateFloat >= 0 && sampleRateFloat <= 1 && hasCorrectDecimals; + + if (!isOpen) { + return null; + } + return ( diff --git a/x-pack/legacy/plugins/apm/public/components/app/TransactionOverview/List/index.tsx b/x-pack/legacy/plugins/apm/public/components/app/TransactionOverview/List/index.tsx index d33ea648a539..8b9451129288 100644 --- a/x-pack/legacy/plugins/apm/public/components/app/TransactionOverview/List/index.tsx +++ b/x-pack/legacy/plugins/apm/public/components/app/TransactionOverview/List/index.tsx @@ -25,11 +25,10 @@ const TransactionNameLink = styled(TransactionLink)` interface Props { items: ITransactionGroup[]; - serviceName: string; isLoading: boolean; } -export function TransactionList({ items, serviceName, isLoading }: Props) { +export function TransactionList({ items, isLoading }: Props) { const columns: Array> = [ { field: 'name', diff --git a/x-pack/legacy/plugins/apm/public/components/app/TransactionOverview/index.tsx b/x-pack/legacy/plugins/apm/public/components/app/TransactionOverview/index.tsx index ab8455a9beca..066bad23e445 100644 --- a/x-pack/legacy/plugins/apm/public/components/app/TransactionOverview/index.tsx +++ b/x-pack/legacy/plugins/apm/public/components/app/TransactionOverview/index.tsx @@ -72,21 +72,22 @@ export function TransactionOverview({ const { data: transactionCharts } = useTransactionCharts(); + const { + data: transactionListData, + status: transactionListStatus + } = useTransactionList(urlParams); + const { data: hasMLJob = false } = useFetcher(() => { + return serviceName && transactionType + ? getHasMLJob({ serviceName, transactionType }) + : undefined; + }, [serviceName, transactionType]); + // TODO: improve urlParams typings. // `serviceName` or `transactionType` will never be undefined here, and this check should not be needed if (!serviceName || !transactionType) { return null; } - const { - data: transactionListData, - status: transactionListStatus - } = useTransactionList(urlParams); - const { data: hasMLJob = false } = useFetcher( - () => getHasMLJob({ serviceName, transactionType }), - [serviceName, transactionType] - ); - return ( {serviceTransactionTypes.length > 1 ? ( @@ -139,7 +140,6 @@ export function TransactionOverview({ diff --git a/x-pack/legacy/plugins/apm/public/components/shared/KueryBar/index.tsx b/x-pack/legacy/plugins/apm/public/components/shared/KueryBar/index.tsx index 0bef2e49a77e..6c46f1ecdb31 100644 --- a/x-pack/legacy/plugins/apm/public/components/shared/KueryBar/index.tsx +++ b/x-pack/legacy/plugins/apm/public/components/shared/KueryBar/index.tsx @@ -72,15 +72,19 @@ export function KueryBar() { let didCancel = false; async function loadIndexPattern() { - setState({ ...state, isLoadingIndexPattern: true }); + setState(value => ({ ...value, isLoadingIndexPattern: true })); const indexPattern = await getAPMIndexPatternForKuery(); if (didCancel) { return; } if (!indexPattern) { - setState({ ...state, isLoadingIndexPattern: false }); + setState(value => ({ ...value, isLoadingIndexPattern: false })); } else { - setState({ ...state, indexPattern, isLoadingIndexPattern: false }); + setState(value => ({ + ...value, + indexPattern, + isLoadingIndexPattern: false + })); } } loadIndexPattern(); diff --git a/x-pack/legacy/plugins/apm/public/context/UrlParamsContext/index.tsx b/x-pack/legacy/plugins/apm/public/context/UrlParamsContext/index.tsx index 383163e8e239..8cf326d6cfd6 100644 --- a/x-pack/legacy/plugins/apm/public/context/UrlParamsContext/index.tsx +++ b/x-pack/legacy/plugins/apm/public/context/UrlParamsContext/index.tsx @@ -39,17 +39,19 @@ const UrlParamsProvider: React.ComponentClass<{}> = withRouter( ({ location, children }) => { const refUrlParams = useRef(resolveUrlParams(location, {})); + const { start, end, rangeFrom, rangeTo } = refUrlParams.current; + const [, forceUpdate] = useState(''); const urlParams = useMemo( () => resolveUrlParams(location, { - start: refUrlParams.current.start, - end: refUrlParams.current.end, - rangeFrom: refUrlParams.current.rangeFrom, - rangeTo: refUrlParams.current.rangeTo + start, + end, + rangeFrom, + rangeTo }), - [location, refUrlParams.current] + [location, start, end, rangeFrom, rangeTo] ); refUrlParams.current = urlParams; diff --git a/x-pack/legacy/plugins/apm/public/hooks/useFetcher.tsx b/x-pack/legacy/plugins/apm/public/hooks/useFetcher.tsx index 0ff770e25707..a540c261def4 100644 --- a/x-pack/legacy/plugins/apm/public/hooks/useFetcher.tsx +++ b/x-pack/legacy/plugins/apm/public/hooks/useFetcher.tsx @@ -74,14 +74,22 @@ export function useFetcher( dispatchStatus({ id, isLoading: false }); didCancel = true; }; - }, [...effectKey, counter]); + /* eslint-disable react-hooks/exhaustive-deps */ + }, [ + counter, + id, + preservePreviousResponse, + dispatchStatus, + ...effectKey + /* eslint-enable react-hooks/exhaustive-deps */ + ]); return useMemo( () => ({ ...result, refresh: () => { // this will invalidate the effectKey and will result in a new request - setCounter(counter + 1); + setCounter(count => count + 1); } }), [result] diff --git a/x-pack/legacy/plugins/apm/public/hooks/useTransactionBreakdown.ts b/x-pack/legacy/plugins/apm/public/hooks/useTransactionBreakdown.ts index 9d6befabbead..22bfb1a1bc23 100644 --- a/x-pack/legacy/plugins/apm/public/hooks/useTransactionBreakdown.ts +++ b/x-pack/legacy/plugins/apm/public/hooks/useTransactionBreakdown.ts @@ -30,7 +30,7 @@ export function useTransactionBreakdown() { uiFilters }); } - }, [serviceName, start, end, uiFilters]); + }, [serviceName, start, end, transactionType, transactionName, uiFilters]); const receivedDataDuringLifetime = useRef(false); diff --git a/x-pack/legacy/plugins/apm/public/hooks/useTransactionCharts.ts b/x-pack/legacy/plugins/apm/public/hooks/useTransactionCharts.ts index c684d8d4c756..629f6bb60e1f 100644 --- a/x-pack/legacy/plugins/apm/public/hooks/useTransactionCharts.ts +++ b/x-pack/legacy/plugins/apm/public/hooks/useTransactionCharts.ts @@ -31,7 +31,7 @@ export function useTransactionCharts() { const memoizedData = useMemo( () => getTransactionCharts({ transactionType }, data), - [data] + [data, transactionType] ); return {