From 8b085b9eac95d3ebeedeee3b3963c7876a9e0e12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loix?= Date: Thu, 3 Sep 2020 18:45:32 +0200 Subject: [PATCH] [es_ui_shared] Fix eslint exhaustive deps rule (#76392) Co-authored-by: Elastic Machine --- .../global_flyout/global_flyout.tsx | 4 +- .../public/components/json_editor/index.ts | 2 +- .../components/json_editor/json_editor.tsx | 151 +++++++++--------- .../public/components/json_editor/use_json.ts | 67 +++++--- src/plugins/es_ui_shared/public/index.ts | 2 +- .../forms/components/fields/range_field.tsx | 7 +- .../load_mappings/load_mappings_provider.tsx | 6 +- .../load_from_json/modal_provider.tsx | 8 +- 8 files changed, 130 insertions(+), 117 deletions(-) diff --git a/src/plugins/es_ui_shared/__packages_do_not_import__/global_flyout/global_flyout.tsx b/src/plugins/es_ui_shared/__packages_do_not_import__/global_flyout/global_flyout.tsx index 548e477c7c411..4dd9cfcaff16b 100644 --- a/src/plugins/es_ui_shared/__packages_do_not_import__/global_flyout/global_flyout.tsx +++ b/src/plugins/es_ui_shared/__packages_do_not_import__/global_flyout/global_flyout.tsx @@ -160,9 +160,7 @@ export const useGlobalFlyout = () => { Array.from(getContents()).forEach(removeContent); } }; - // https://github.com/elastic/kibana/issues/73970 - /* eslint-disable-next-line react-hooks/exhaustive-deps */ - }, [removeContent]); + }, [removeContent, getContents]); return { ...ctx, addContent }; }; diff --git a/src/plugins/es_ui_shared/public/components/json_editor/index.ts b/src/plugins/es_ui_shared/public/components/json_editor/index.ts index 81476a65f4215..63319baa38f5c 100644 --- a/src/plugins/es_ui_shared/public/components/json_editor/index.ts +++ b/src/plugins/es_ui_shared/public/components/json_editor/index.ts @@ -19,4 +19,4 @@ export * from './json_editor'; -export { OnJsonEditorUpdateHandler } from './use_json'; +export { OnJsonEditorUpdateHandler, JsonEditorState } from './use_json'; diff --git a/src/plugins/es_ui_shared/public/components/json_editor/json_editor.tsx b/src/plugins/es_ui_shared/public/components/json_editor/json_editor.tsx index 7d21722781d60..206db5a285620 100644 --- a/src/plugins/es_ui_shared/public/components/json_editor/json_editor.tsx +++ b/src/plugins/es_ui_shared/public/components/json_editor/json_editor.tsx @@ -17,98 +17,97 @@ * under the License. */ -import React, { useCallback } from 'react'; +import React, { useCallback, useMemo } from 'react'; import { EuiFormRow, EuiCodeEditor } from '@elastic/eui'; import { debounce } from 'lodash'; -import { isJSON } from '../../../static/validators/string'; import { useJson, OnJsonEditorUpdateHandler } from './use_json'; -interface Props { - onUpdate: OnJsonEditorUpdateHandler; +interface Props { + onUpdate: OnJsonEditorUpdateHandler; label?: string; helpText?: React.ReactNode; value?: string; - defaultValue?: { [key: string]: any }; + defaultValue?: T; euiCodeEditorProps?: { [key: string]: any }; error?: string | null; } -export const JsonEditor = React.memo( - ({ - label, - helpText, +function JsonEditorComp({ + label, + helpText, + onUpdate, + value, + defaultValue, + euiCodeEditorProps, + error: propsError, +}: Props) { + const { content, setContent, error: internalError, isControlled } = useJson({ + defaultValue, onUpdate, value, - defaultValue, - euiCodeEditorProps, - error: propsError, - }: Props) => { - const isControlled = value !== undefined; + }); - const { content, setContent, error: internalError } = useJson({ - defaultValue, - onUpdate, - isControlled, - }); + const debouncedSetContent = useMemo(() => { + return debounce(setContent, 300); + }, [setContent]); - // https://github.com/elastic/kibana/issues/73971 - /* eslint-disable-next-line react-hooks/exhaustive-deps */ - const debouncedSetContent = useCallback(debounce(setContent, 300), [setContent]); + // We let the consumer control the validation and the error message. + const error = isControlled ? propsError : internalError; - // We let the consumer control the validation and the error message. - const error = isControlled ? propsError : internalError; + const onEuiCodeEditorChange = useCallback( + (updated: string) => { + if (isControlled) { + onUpdate({ + data: { + raw: updated, + format: () => JSON.parse(updated), + }, + validate: () => { + try { + JSON.parse(updated); + return true; + } catch (e) { + return false; + } + }, + isValid: undefined, + }); + } else { + debouncedSetContent(updated); + } + }, + [isControlled, debouncedSetContent, onUpdate] + ); - const onEuiCodeEditorChange = useCallback( - (updated: string) => { - if (isControlled) { - onUpdate({ - data: { - raw: updated, - format() { - return JSON.parse(updated); - }, - }, - validate() { - return isJSON(updated); - }, - isValid: undefined, - }); - } else { - debouncedSetContent(updated); - } - }, - /* eslint-disable-next-line react-hooks/exhaustive-deps */ - [isControlled] - ); + return ( + + + + ); +} - return ( - - - - ); - } -); +export const JsonEditor = React.memo(JsonEditorComp) as typeof JsonEditorComp; diff --git a/src/plugins/es_ui_shared/public/components/json_editor/use_json.ts b/src/plugins/es_ui_shared/public/components/json_editor/use_json.ts index 0ba39f5f05fe6..47d518e6814a4 100644 --- a/src/plugins/es_ui_shared/public/components/json_editor/use_json.ts +++ b/src/plugins/es_ui_shared/public/components/json_editor/use_json.ts @@ -17,24 +17,28 @@ * under the License. */ -import { useEffect, useState, useRef } from 'react'; +import { useEffect, useState, useRef, useCallback } from 'react'; import { i18n } from '@kbn/i18n'; import { isJSON } from '../../../static/validators/string'; -export type OnJsonEditorUpdateHandler = (arg: { +export interface JsonEditorState { data: { raw: string; format(): T; }; validate(): boolean; isValid: boolean | undefined; -}) => void; +} + +export type OnJsonEditorUpdateHandler = ( + arg: JsonEditorState +) => void; interface Parameters { onUpdate: OnJsonEditorUpdateHandler; defaultValue?: T; - isControlled?: boolean; + value?: string; } const stringifyJson = (json: { [key: string]: any }) => @@ -43,13 +47,16 @@ const stringifyJson = (json: { [key: string]: any }) => export const useJson = ({ defaultValue = {} as T, onUpdate, - isControlled = false, + value, }: Parameters) => { - const didMount = useRef(false); - const [content, setContent] = useState(stringifyJson(defaultValue)); + const isControlled = value !== undefined; + const isMounted = useRef(false); + const [content, setContent] = useState( + isControlled ? value! : stringifyJson(defaultValue) + ); const [error, setError] = useState(null); - const validate = () => { + const validate = useCallback(() => { // We allow empty string as it will be converted to "{}"" const isValid = content.trim() === '' ? true : isJSON(content); if (!isValid) { @@ -62,35 +69,43 @@ export const useJson = ({ setError(null); } return isValid; - }; + }, [content]); - const formatContent = () => { + const formatContent = useCallback(() => { const isValid = validate(); const data = isValid && content.trim() !== '' ? JSON.parse(content) : {}; return data as T; - }; + }, [validate, content]); useEffect(() => { - if (didMount.current) { - const isValid = isControlled ? undefined : validate(); - onUpdate({ - data: { - raw: content, - format: formatContent, - }, - validate, - isValid, - }); - } else { - didMount.current = true; + if (!isMounted.current || isControlled) { + return; } - // https://github.com/elastic/kibana/issues/73971 - /* eslint-disable-next-line react-hooks/exhaustive-deps */ - }, [content]); + + const isValid = validate(); + + onUpdate({ + data: { + raw: content, + format: formatContent, + }, + validate, + isValid, + }); + }, [onUpdate, content, formatContent, validate, isControlled]); + + useEffect(() => { + isMounted.current = true; + + return () => { + isMounted.current = false; + }; + }, []); return { content, setContent, error, + isControlled, }; }; diff --git a/src/plugins/es_ui_shared/public/index.ts b/src/plugins/es_ui_shared/public/index.ts index 995ae0ba42837..5a1c13658604a 100644 --- a/src/plugins/es_ui_shared/public/index.ts +++ b/src/plugins/es_ui_shared/public/index.ts @@ -26,7 +26,7 @@ import * as Monaco from './monaco'; import * as ace from './ace'; import * as GlobalFlyout from './global_flyout'; -export { JsonEditor, OnJsonEditorUpdateHandler } from './components/json_editor'; +export { JsonEditor, OnJsonEditorUpdateHandler, JsonEditorState } from './components/json_editor'; export { SectionLoading } from './components/section_loading'; diff --git a/src/plugins/es_ui_shared/static/forms/components/fields/range_field.tsx b/src/plugins/es_ui_shared/static/forms/components/fields/range_field.tsx index b83b0af5f97c6..9ffa7adace781 100644 --- a/src/plugins/es_ui_shared/static/forms/components/fields/range_field.tsx +++ b/src/plugins/es_ui_shared/static/forms/components/fields/range_field.tsx @@ -31,17 +31,16 @@ interface Props { export const RangeField = ({ field, euiFieldProps = {}, ...rest }: Props) => { const { isInvalid, errorMessage } = getFieldValidityAndErrorMessage(field); + const { onChange: onFieldChange } = field; const onChange = useCallback( (e: React.ChangeEvent | React.MouseEvent) => { const event = ({ ...e, value: `${e.currentTarget.value}` } as unknown) as React.ChangeEvent<{ value: string; }>; - field.onChange(event); + onFieldChange(event); }, - // https://github.com/elastic/kibana/issues/73972 - /* eslint-disable-next-line react-hooks/exhaustive-deps */ - [field.onChange] + [onFieldChange] ); return ( diff --git a/x-pack/plugins/index_management/public/application/components/mappings_editor/components/load_mappings/load_mappings_provider.tsx b/x-pack/plugins/index_management/public/application/components/mappings_editor/components/load_mappings/load_mappings_provider.tsx index a50572df9004e..af0590be5c941 100644 --- a/x-pack/plugins/index_management/public/application/components/mappings_editor/components/load_mappings/load_mappings_provider.tsx +++ b/x-pack/plugins/index_management/public/application/components/mappings_editor/components/load_mappings/load_mappings_provider.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { useState, useRef } from 'react'; +import React, { useState, useRef, useCallback } from 'react'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; import { @@ -134,9 +134,9 @@ export const LoadMappingsProvider = ({ onJson, children }: Props) => { state.json !== undefined && state.errors !== undefined ? 'validationResult' : 'json'; const i18nTexts = getTexts(view, state.errors?.length); - const onJsonUpdate: OnJsonEditorUpdateHandler = (jsonUpdateData) => { + const onJsonUpdate: OnJsonEditorUpdateHandler = useCallback((jsonUpdateData) => { jsonContent.current = jsonUpdateData; - }; + }, []); const openModal: OpenJsonModalFunc = () => { setState({ isModalOpen: true }); diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/load_from_json/modal_provider.tsx b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/load_from_json/modal_provider.tsx index f183386d5927d..9e777de0e2edf 100644 --- a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/load_from_json/modal_provider.tsx +++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/load_from_json/modal_provider.tsx @@ -5,7 +5,7 @@ */ import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; -import React, { FunctionComponent, useRef, useState } from 'react'; +import React, { FunctionComponent, useRef, useState, useCallback } from 'react'; import { EuiConfirmModal, EuiOverlayMask, EuiSpacer, EuiText, EuiCallOut } from '@elastic/eui'; import { JsonEditor, OnJsonEditorUpdateHandler } from '../../../../../shared_imports'; @@ -66,10 +66,12 @@ export const ModalProvider: FunctionComponent = ({ onDone, children }) => raw: defaultValueRaw, }, }); - const onJsonUpdate: OnJsonEditorUpdateHandler = (jsonUpdateData) => { + + const onJsonUpdate: OnJsonEditorUpdateHandler = useCallback((jsonUpdateData) => { setIsValidJson(jsonUpdateData.validate()); jsonContent.current = jsonUpdateData; - }; + }, []); + return ( <> {children(() => setIsModalVisible(true))}