diff --git a/.eslintrc.js b/.eslintrc.js index ff4ac180c3774..9b75c36c95abd 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -94,12 +94,6 @@ module.exports = { 'jsx-a11y/no-onchange': 'off', }, }, - { - files: ['src/plugins/es_ui_shared/**/*.{js,mjs,ts,tsx}'], - rules: { - 'react-hooks/exhaustive-deps': 'off', - }, - }, { files: ['src/plugins/kibana_react/**/*.{js,mjs,ts,tsx}'], rules: { @@ -125,25 +119,12 @@ module.exports = { 'jsx-a11y/click-events-have-key-events': 'off', }, }, - { - files: ['x-pack/legacy/plugins/index_management/**/*.{js,mjs,ts,tsx}'], - rules: { - 'react-hooks/exhaustive-deps': 'off', - 'react-hooks/rules-of-hooks': 'off', - }, - }, { files: ['x-pack/plugins/ml/**/*.{js,mjs,ts,tsx}'], rules: { 'react-hooks/exhaustive-deps': 'off', }, }, - { - files: ['x-pack/legacy/plugins/snapshot_restore/**/*.{js,mjs,ts,tsx}'], - rules: { - 'react-hooks/exhaustive-deps': 'off', - }, - }, /** * Files that require Apache 2.0 headers, settings 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 aa575cd64944c..548e477c7c411 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,6 +160,8 @@ export const useGlobalFlyout = () => { Array.from(getContents()).forEach(removeContent); } }; + // https://github.com/elastic/kibana/issues/73970 + /* eslint-disable-next-line react-hooks/exhaustive-deps */ }, [removeContent]); return { ...ctx, addContent }; 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 8c63cc8494a8b..7d21722781d60 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 @@ -52,6 +52,8 @@ export const JsonEditor = React.memo( isControlled, }); + // 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. @@ -76,6 +78,7 @@ export const JsonEditor = React.memo( debouncedSetContent(updated); } }, + /* eslint-disable-next-line react-hooks/exhaustive-deps */ [isControlled] ); 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 1b5ca5d7f4384..0ba39f5f05fe6 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 @@ -84,6 +84,8 @@ export const useJson = ({ } else { didMount.current = true; } + // https://github.com/elastic/kibana/issues/73971 + /* eslint-disable-next-line react-hooks/exhaustive-deps */ }, [content]); return { diff --git a/src/plugins/es_ui_shared/public/index.ts b/src/plugins/es_ui_shared/public/index.ts index bdea5ccf5fe26..995ae0ba42837 100644 --- a/src/plugins/es_ui_shared/public/index.ts +++ b/src/plugins/es_ui_shared/public/index.ts @@ -39,7 +39,7 @@ export { UseRequestResponse, sendRequest, useRequest, -} from './request/np_ready_request'; +} from './request'; export { indices } from './indices'; diff --git a/src/plugins/es_ui_shared/public/request/index.ts b/src/plugins/es_ui_shared/public/request/index.ts index f942a9cc3932b..9e38bfe5ee16b 100644 --- a/src/plugins/es_ui_shared/public/request/index.ts +++ b/src/plugins/es_ui_shared/public/request/index.ts @@ -17,11 +17,5 @@ * under the License. */ -export { - SendRequestConfig, - SendRequestResponse, - UseRequestConfig, - UseRequestResponse, - sendRequest, - useRequest, -} from './request'; +export { SendRequestConfig, SendRequestResponse, sendRequest } from './send_request'; +export { UseRequestConfig, UseRequestResponse, useRequest } from './use_request'; diff --git a/src/plugins/es_ui_shared/public/request/np_ready_request.ts b/src/plugins/es_ui_shared/public/request/np_ready_request.ts deleted file mode 100644 index 06af698f2ce02..0000000000000 --- a/src/plugins/es_ui_shared/public/request/np_ready_request.ts +++ /dev/null @@ -1,184 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { useEffect, useState, useRef, useMemo } from 'react'; - -import { HttpSetup, HttpFetchQuery } from '../../../../../src/core/public'; - -export interface SendRequestConfig { - path: string; - method: 'get' | 'post' | 'put' | 'delete' | 'patch' | 'head'; - query?: HttpFetchQuery; - body?: any; -} - -export interface SendRequestResponse { - data: D | null; - error: E | null; -} - -export interface UseRequestConfig extends SendRequestConfig { - pollIntervalMs?: number; - initialData?: any; - deserializer?: (data: any) => any; -} - -export interface UseRequestResponse { - isInitialRequest: boolean; - isLoading: boolean; - error: E | null; - data?: D | null; - sendRequest: (...args: any[]) => Promise>; -} - -export const sendRequest = async ( - httpClient: HttpSetup, - { path, method, body, query }: SendRequestConfig -): Promise> => { - try { - const stringifiedBody = typeof body === 'string' ? body : JSON.stringify(body); - const response = await httpClient[method](path, { body: stringifiedBody, query }); - - return { - data: response.data ? response.data : response, - error: null, - }; - } catch (e) { - return { - data: null, - error: e.response && e.response.data ? e.response.data : e.body, - }; - } -}; - -export const useRequest = ( - httpClient: HttpSetup, - { - path, - method, - query, - body, - pollIntervalMs, - initialData, - deserializer = (data: any): any => data, - }: UseRequestConfig -): UseRequestResponse => { - const sendRequestRef = useRef<() => Promise>>(); - // Main states for tracking request status and data - const [error, setError] = useState(null); - const [isLoading, setIsLoading] = useState(true); - const [data, setData] = useState(initialData); - - // Consumers can use isInitialRequest to implement a polling UX. - const [isInitialRequest, setIsInitialRequest] = useState(true); - const pollInterval = useRef(null); - const pollIntervalId = useRef(null); - - // We always want to use the most recently-set interval in scheduleRequest. - pollInterval.current = pollIntervalMs; - - // Tied to every render and bound to each request. - let isOutdatedRequest = false; - - const scheduleRequest = () => { - // Clear current interval - if (pollIntervalId.current) { - clearTimeout(pollIntervalId.current); - } - - // Set new interval - if (pollInterval.current) { - pollIntervalId.current = setTimeout( - () => (sendRequestRef.current ?? _sendRequest)(), - pollInterval.current - ); - } - }; - - const _sendRequest = async () => { - // We don't clear error or data, so it's up to the consumer to decide whether to display the - // "old" error/data or loading state when a new request is in-flight. - setIsLoading(true); - - const requestBody = { - path, - method, - query, - body, - }; - - const response = await sendRequest(httpClient, requestBody); - const { data: serializedResponseData, error: responseError } = response; - - // If an outdated request has resolved, DON'T update state, but DO allow the processData handler - // to execute side effects like update telemetry. - if (isOutdatedRequest) { - return { data: null, error: null }; - } - - setError(responseError); - - if (!responseError) { - const responseData = deserializer(serializedResponseData); - setData(responseData); - } - - setIsLoading(false); - setIsInitialRequest(false); - - // If we're on an interval, we need to schedule the next request. This also allows us to reset - // the interval if the user has manually requested the data, to avoid doubled-up requests. - scheduleRequest(); - - return { data: serializedResponseData, error: responseError }; - }; - - useEffect(() => { - sendRequestRef.current = _sendRequest; - }, [_sendRequest]); - - const stringifiedQuery = useMemo(() => JSON.stringify(query), [query]); - - useEffect(() => { - (sendRequestRef.current ?? _sendRequest)(); - // To be functionally correct we'd send a new request if the method, path, query or body changes. - // But it doesn't seem likely that the method will change and body is likely to be a new - // object even if its shape hasn't changed, so for now we're just watching the path and the query. - }, [path, stringifiedQuery]); - - useEffect(() => { - scheduleRequest(); - - // Clean up intervals and inflight requests and corresponding state changes - return () => { - isOutdatedRequest = true; - if (pollIntervalId.current) { - clearTimeout(pollIntervalId.current); - } - }; - }, [pollIntervalMs]); - - return { - isInitialRequest, - isLoading, - error, - data, - sendRequest: sendRequestRef.current ?? _sendRequest, // Gives the user the ability to manually request data - }; -}; diff --git a/src/plugins/es_ui_shared/public/request/request.test.js b/src/plugins/es_ui_shared/public/request/request.test.js deleted file mode 100644 index 190c32517eefe..0000000000000 --- a/src/plugins/es_ui_shared/public/request/request.test.js +++ /dev/null @@ -1,262 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import sinon from 'sinon'; -// import { sendRequest as sendRequestUnbound, useRequest as useRequestUnbound } from './request'; - -import React from 'react'; -import { act } from 'react-dom/test-utils'; -import { mount } from 'enzyme'; - -const TestHook = ({ callback }) => { - callback(); - return null; -}; - -let element; - -const testHook = (callback) => { - element = mount(); -}; - -const wait = async (wait) => new Promise((resolve) => setTimeout(resolve, wait || 1)); - -// FLAKY: -// - https://github.com/elastic/kibana/issues/42561 -// - https://github.com/elastic/kibana/issues/42562 -// - https://github.com/elastic/kibana/issues/42563 -// - https://github.com/elastic/kibana/issues/42225 -describe.skip('request lib', () => { - const successRequest = { path: '/success', method: 'post', body: {} }; - const errorRequest = { path: '/error', method: 'post', body: {} }; - const successResponse = { statusCode: 200, data: { message: 'Success message' } }; - const errorResponse = { statusCode: 400, statusText: 'Error message' }; - - let sendPost; - let sendRequest; - let useRequest; - - /** - * - * commented out due to hooks being called regardless of skip - * https://github.com/facebook/jest/issues/8379 - - beforeEach(() => { - sendPost = sinon.stub(); - sendPost.withArgs(successRequest.path, successRequest.body).returns(successResponse); - sendPost.withArgs(errorRequest.path, errorRequest.body).throws(errorResponse); - - const httpClient = { - post: (...args) => { - return sendPost(...args); - }, - }; - - sendRequest = sendRequestUnbound.bind(null, httpClient); - useRequest = useRequestUnbound.bind(null, httpClient); - }); - - */ - - describe('sendRequest function', () => { - it('uses the provided path, method, and body to send the request', async () => { - const response = await sendRequest({ ...successRequest }); - sinon.assert.calledOnce(sendPost); - expect(response).toEqual({ data: successResponse.data, error: null }); - }); - - it('surfaces errors', async () => { - try { - await sendRequest({ ...errorRequest }); - } catch (e) { - sinon.assert.calledOnce(sendPost); - expect(e).toBe(errorResponse.error); - } - }); - }); - - describe('useRequest hook', () => { - let hook; - - function initUseRequest(config) { - act(() => { - testHook(() => { - hook = useRequest(config); - }); - }); - } - - describe('parameters', () => { - describe('path, method, body', () => { - it('is used to send the request', async () => { - initUseRequest({ ...successRequest }); - await wait(50); - expect(hook.data).toBe(successResponse.data); - }); - }); - - describe('pollIntervalMs', () => { - it('sends another request after the specified time has elapsed', async () => { - initUseRequest({ ...successRequest, pollIntervalMs: 10 }); - await wait(50); - // We just care that multiple requests have been sent out. We don't check the specific - // timing because that risks introducing flakiness into the tests, and it's unlikely - // we could break the implementation by getting the exact timing wrong. - expect(sendPost.callCount).toBeGreaterThan(1); - - // We have to manually clean up or else the interval will continue to fire requests, - // interfering with other tests. - element.unmount(); - }); - }); - - describe('initialData', () => { - it('sets the initial data value', () => { - initUseRequest({ ...successRequest, initialData: 'initialData' }); - expect(hook.data).toBe('initialData'); - }); - }); - - describe('deserializer', () => { - it('is called once the request resolves', async () => { - const deserializer = sinon.stub(); - initUseRequest({ ...successRequest, deserializer }); - sinon.assert.notCalled(deserializer); - - await wait(50); - sinon.assert.calledOnce(deserializer); - sinon.assert.calledWith(deserializer, successResponse.data); - }); - - it('processes data', async () => { - initUseRequest({ ...successRequest, deserializer: () => 'intercepted' }); - await wait(50); - expect(hook.data).toBe('intercepted'); - }); - }); - }); - - describe('state', () => { - describe('isInitialRequest', () => { - it('is true for the first request and false for subsequent requests', async () => { - initUseRequest({ ...successRequest }); - expect(hook.isInitialRequest).toBe(true); - - hook.sendRequest(); - await wait(50); - expect(hook.isInitialRequest).toBe(false); - }); - }); - - describe('isLoading', () => { - it('represents in-flight request status', async () => { - initUseRequest({ ...successRequest }); - expect(hook.isLoading).toBe(true); - - await wait(50); - expect(hook.isLoading).toBe(false); - }); - }); - - describe('error', () => { - it('surfaces errors from requests', async () => { - initUseRequest({ ...errorRequest }); - await wait(50); - expect(hook.error).toBe(errorResponse); - }); - - it('persists while a request is in-flight', async () => { - initUseRequest({ ...errorRequest }); - await wait(50); - hook.sendRequest(); - expect(hook.isLoading).toBe(true); - expect(hook.error).toBe(errorResponse); - }); - - it('is null when the request is successful', async () => { - initUseRequest({ ...successRequest }); - await wait(50); - expect(hook.isLoading).toBe(false); - expect(hook.error).toBeNull(); - }); - }); - - describe('data', () => { - it('surfaces payloads from requests', async () => { - initUseRequest({ ...successRequest }); - await wait(50); - expect(hook.data).toBe(successResponse.data); - }); - - it('persists while a request is in-flight', async () => { - initUseRequest({ ...successRequest }); - await wait(50); - hook.sendRequest(); - expect(hook.isLoading).toBe(true); - expect(hook.data).toBe(successResponse.data); - }); - - it('is null when the request fails', async () => { - initUseRequest({ ...errorRequest }); - await wait(50); - expect(hook.isLoading).toBe(false); - expect(hook.data).toBeNull(); - }); - }); - }); - - describe('callbacks', () => { - describe('sendRequest', () => { - it('sends the request', () => { - initUseRequest({ ...successRequest }); - sinon.assert.calledOnce(sendPost); - hook.sendRequest(); - sinon.assert.calledTwice(sendPost); - }); - - it('resets the pollIntervalMs', async () => { - initUseRequest({ ...successRequest, pollIntervalMs: 800 }); - await wait(200); // 200ms - hook.sendRequest(); - expect(sendPost.callCount).toBe(2); - - await wait(200); // 400ms - hook.sendRequest(); - - await wait(200); // 600ms - hook.sendRequest(); - - await wait(200); // 800ms - hook.sendRequest(); - - await wait(200); // 1000ms - hook.sendRequest(); - - // If sendRequest didn't reset the interval, the interval would have triggered another - // request by now, and the callCount would be 7. - expect(sendPost.callCount).toBe(6); - - // We have to manually clean up or else the interval will continue to fire requests, - // interfering with other tests. - element.unmount(); - }); - }); - }); - }); -}); diff --git a/src/plugins/es_ui_shared/public/request/request.ts b/src/plugins/es_ui_shared/public/request/request.ts deleted file mode 100644 index fd6980367136e..0000000000000 --- a/src/plugins/es_ui_shared/public/request/request.ts +++ /dev/null @@ -1,165 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { useEffect, useState, useRef } from 'react'; - -export interface SendRequestConfig { - path: string; - method: 'get' | 'post' | 'put' | 'delete' | 'patch' | 'head'; - body?: any; -} - -export interface SendRequestResponse { - data: any; - error: Error | null; -} - -export interface UseRequestConfig extends SendRequestConfig { - pollIntervalMs?: number; - initialData?: any; - deserializer?: (data: any) => any; -} - -export interface UseRequestResponse { - isInitialRequest: boolean; - isLoading: boolean; - error: null | unknown; - data: any; - sendRequest: (...args: any[]) => Promise; -} - -export const sendRequest = async ( - httpClient: ng.IHttpService, - { path, method, body }: SendRequestConfig -): Promise => { - try { - const response = await (httpClient as any)[method](path, body); - - if (typeof response.data === 'undefined') { - throw new Error(response.statusText); - } - - return { data: response.data, error: null }; - } catch (e) { - return { - data: null, - error: e.response ? e.response : e, - }; - } -}; - -export const useRequest = ( - httpClient: ng.IHttpService, - { - path, - method, - body, - pollIntervalMs, - initialData, - deserializer = (data: any): any => data, - }: UseRequestConfig -): UseRequestResponse => { - // Main states for tracking request status and data - const [error, setError] = useState(null); - const [isLoading, setIsLoading] = useState(true); - const [data, setData] = useState(initialData); - - // Consumers can use isInitialRequest to implement a polling UX. - const [isInitialRequest, setIsInitialRequest] = useState(true); - const pollInterval = useRef(null); - const pollIntervalId = useRef(null); - - // We always want to use the most recently-set interval in scheduleRequest. - pollInterval.current = pollIntervalMs; - - // Tied to every render and bound to each request. - let isOutdatedRequest = false; - - const scheduleRequest = () => { - // Clear current interval - if (pollIntervalId.current) { - clearTimeout(pollIntervalId.current); - } - - // Set new interval - if (pollInterval.current) { - pollIntervalId.current = setTimeout(_sendRequest, pollInterval.current); - } - }; - - const _sendRequest = async () => { - // We don't clear error or data, so it's up to the consumer to decide whether to display the - // "old" error/data or loading state when a new request is in-flight. - setIsLoading(true); - - const requestBody = { - path, - method, - body, - }; - - const response = await sendRequest(httpClient, requestBody); - const { data: serializedResponseData, error: responseError } = response; - const responseData = deserializer(serializedResponseData); - - // If an outdated request has resolved, DON'T update state, but DO allow the processData handler - // to execute side effects like update telemetry. - if (isOutdatedRequest) { - return { data: null, error: null }; - } - - setError(responseError); - setData(responseData); - setIsLoading(false); - setIsInitialRequest(false); - - // If we're on an interval, we need to schedule the next request. This also allows us to reset - // the interval if the user has manually requested the data, to avoid doubled-up requests. - scheduleRequest(); - - return { data: serializedResponseData, error: responseError }; - }; - - useEffect(() => { - _sendRequest(); - // To be functionally correct we'd send a new request if the method, path, or body changes. - // But it doesn't seem likely that the method will change and body is likely to be a new - // object even if its shape hasn't changed, so for now we're just watching the path. - }, [path]); - - useEffect(() => { - scheduleRequest(); - - // Clean up intervals and inflight requests and corresponding state changes - return () => { - isOutdatedRequest = true; - if (pollIntervalId.current) { - clearTimeout(pollIntervalId.current); - } - }; - }, [pollIntervalMs]); - - return { - isInitialRequest, - isLoading, - error, - data, - sendRequest: _sendRequest, // Gives the user the ability to manually request data - }; -}; diff --git a/src/plugins/es_ui_shared/public/request/send_request.test.helpers.ts b/src/plugins/es_ui_shared/public/request/send_request.test.helpers.ts new file mode 100644 index 0000000000000..4312780e74c0f --- /dev/null +++ b/src/plugins/es_ui_shared/public/request/send_request.test.helpers.ts @@ -0,0 +1,83 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import sinon from 'sinon'; + +import { HttpSetup, HttpFetchOptions } from '../../../../../src/core/public'; +import { + SendRequestConfig, + SendRequestResponse, + sendRequest as originalSendRequest, +} from './send_request'; + +export interface SendRequestHelpers { + getSendRequestSpy: () => sinon.SinonStub; + sendSuccessRequest: () => Promise; + getSuccessResponse: () => SendRequestResponse; + sendErrorRequest: () => Promise; + getErrorResponse: () => SendRequestResponse; +} + +const successRequest: SendRequestConfig = { method: 'post', path: '/success', body: {} }; +const successResponse = { statusCode: 200, data: { message: 'Success message' } }; + +const errorValue = { statusCode: 400, statusText: 'Error message' }; +const errorRequest: SendRequestConfig = { method: 'post', path: '/error', body: {} }; +const errorResponse = { response: { data: errorValue } }; + +export const createSendRequestHelpers = (): SendRequestHelpers => { + const sendRequestSpy = sinon.stub(); + const httpClient = { + post: (path: string, options: HttpFetchOptions) => sendRequestSpy(path, options), + }; + const sendRequest = originalSendRequest.bind(null, httpClient as HttpSetup) as ( + config: SendRequestConfig + ) => Promise>; + + // Set up successful request helpers. + sendRequestSpy + .withArgs(successRequest.path, { + body: JSON.stringify(successRequest.body), + query: undefined, + }) + .resolves(successResponse); + const sendSuccessRequest = () => sendRequest({ ...successRequest }); + const getSuccessResponse = () => ({ data: successResponse.data, error: null }); + + // Set up failed request helpers. + sendRequestSpy + .withArgs(errorRequest.path, { + body: JSON.stringify(errorRequest.body), + query: undefined, + }) + .rejects(errorResponse); + const sendErrorRequest = () => sendRequest({ ...errorRequest }); + const getErrorResponse = () => ({ + data: null, + error: errorResponse.response.data, + }); + + return { + getSendRequestSpy: () => sendRequestSpy, + sendSuccessRequest, + getSuccessResponse, + sendErrorRequest, + getErrorResponse, + }; +}; diff --git a/src/plugins/es_ui_shared/public/request/send_request.test.ts b/src/plugins/es_ui_shared/public/request/send_request.test.ts new file mode 100644 index 0000000000000..e4deaeaba817e --- /dev/null +++ b/src/plugins/es_ui_shared/public/request/send_request.test.ts @@ -0,0 +1,47 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import sinon from 'sinon'; + +import { SendRequestHelpers, createSendRequestHelpers } from './send_request.test.helpers'; + +describe('sendRequest function', () => { + let helpers: SendRequestHelpers; + + beforeEach(() => { + helpers = createSendRequestHelpers(); + }); + + it('uses the provided path, method, and body to send the request', async () => { + const { sendSuccessRequest, getSendRequestSpy, getSuccessResponse } = helpers; + + const response = await sendSuccessRequest(); + sinon.assert.calledOnce(getSendRequestSpy()); + expect(response).toEqual(getSuccessResponse()); + }); + + it('surfaces errors', async () => { + const { sendErrorRequest, getSendRequestSpy, getErrorResponse } = helpers; + + // For some reason sinon isn't throwing an error on rejection, as an awaited Promise normally would. + const error = await sendErrorRequest(); + sinon.assert.calledOnce(getSendRequestSpy()); + expect(error).toEqual(getErrorResponse()); + }); +}); diff --git a/src/plugins/es_ui_shared/public/request/send_request.ts b/src/plugins/es_ui_shared/public/request/send_request.ts new file mode 100644 index 0000000000000..453e91570cd85 --- /dev/null +++ b/src/plugins/es_ui_shared/public/request/send_request.ts @@ -0,0 +1,52 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { HttpSetup, HttpFetchQuery } from '../../../../../src/core/public'; + +export interface SendRequestConfig { + path: string; + method: 'get' | 'post' | 'put' | 'delete' | 'patch' | 'head'; + query?: HttpFetchQuery; + body?: any; +} + +export interface SendRequestResponse { + data: D | null; + error: E | null; +} + +export const sendRequest = async ( + httpClient: HttpSetup, + { path, method, body, query }: SendRequestConfig +): Promise> => { + try { + const stringifiedBody = typeof body === 'string' ? body : JSON.stringify(body); + const response = await httpClient[method](path, { body: stringifiedBody, query }); + + return { + data: response.data ? response.data : response, + error: null, + }; + } catch (e) { + return { + data: null, + error: e.response?.data ?? e.body, + }; + } +}; diff --git a/src/plugins/es_ui_shared/public/request/use_request.test.helpers.tsx b/src/plugins/es_ui_shared/public/request/use_request.test.helpers.tsx new file mode 100644 index 0000000000000..0d6fd122ad22c --- /dev/null +++ b/src/plugins/es_ui_shared/public/request/use_request.test.helpers.tsx @@ -0,0 +1,184 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React from 'react'; +import { act } from 'react-dom/test-utils'; +import { mount, ReactWrapper } from 'enzyme'; +import sinon from 'sinon'; + +import { HttpSetup, HttpFetchOptions } from '../../../../../src/core/public'; +import { SendRequestConfig, SendRequestResponse } from './send_request'; +import { useRequest, UseRequestResponse, UseRequestConfig } from './use_request'; + +export interface UseRequestHelpers { + advanceTime: (ms: number) => Promise; + completeRequest: () => Promise; + hookResult: UseRequestResponse; + getSendRequestSpy: () => sinon.SinonStub; + setupSuccessRequest: (overrides?: {}, requestTimings?: number[]) => void; + getSuccessResponse: () => SendRequestResponse; + setupErrorRequest: (overrides?: {}, requestTimings?: number[]) => void; + getErrorResponse: () => SendRequestResponse; + setErrorResponse: (overrides?: {}) => void; + setupErrorWithBodyRequest: (overrides?: {}) => void; + getErrorWithBodyResponse: () => SendRequestResponse; +} + +// Each request will take 1s to resolve. +export const REQUEST_TIME = 1000; + +const successRequest: SendRequestConfig = { method: 'post', path: '/success', body: {} }; +const successResponse = { statusCode: 200, data: { message: 'Success message' } }; + +const errorValue = { statusCode: 400, statusText: 'Error message' }; +const errorRequest: SendRequestConfig = { method: 'post', path: '/error', body: {} }; +const errorResponse = { response: { data: errorValue } }; + +const errorWithBodyRequest: SendRequestConfig = { + method: 'post', + path: '/errorWithBody', + body: {}, +}; +const errorWithBodyResponse = { body: errorValue }; + +export const createUseRequestHelpers = (): UseRequestHelpers => { + // The behavior we're testing involves state changes over time, so we need finer control over + // timing. + jest.useFakeTimers(); + + const flushPromiseJobQueue = async () => { + // See https://stackoverflow.com/questions/52177631/jest-timer-and-promise-dont-work-well-settimeout-and-async-function + await Promise.resolve(); + }; + + const completeRequest = async () => { + await act(async () => { + jest.runAllTimers(); + await flushPromiseJobQueue(); + }); + }; + + const advanceTime = async (ms: number) => { + await act(async () => { + jest.advanceTimersByTime(ms); + await flushPromiseJobQueue(); + }); + }; + + let element: ReactWrapper; + // We'll use this object to observe the state of the hook and access its callback(s). + const hookResult = {} as UseRequestResponse; + const sendRequestSpy = sinon.stub(); + + const setupUseRequest = (config: UseRequestConfig, requestTimings?: number[]) => { + let requestCount = 0; + + const httpClient = { + post: (path: string, options: HttpFetchOptions) => { + return new Promise((resolve, reject) => { + // Increase the time it takes to resolve a request so we have time to inspect the hook + // as it goes through various states. + setTimeout(() => { + try { + resolve(sendRequestSpy(path, options)); + } catch (e) { + reject(e); + } + }, (requestTimings && requestTimings[requestCount++]) || REQUEST_TIME); + }); + }, + }; + + const TestComponent = ({ requestConfig }: { requestConfig: UseRequestConfig }) => { + const { isInitialRequest, isLoading, error, data, sendRequest } = useRequest( + httpClient as HttpSetup, + requestConfig + ); + + hookResult.isInitialRequest = isInitialRequest; + hookResult.isLoading = isLoading; + hookResult.error = error; + hookResult.data = data; + hookResult.sendRequest = sendRequest; + + return null; + }; + + act(() => { + element = mount(); + }); + }; + + // Set up successful request helpers. + sendRequestSpy + .withArgs(successRequest.path, { + body: JSON.stringify(successRequest.body), + query: undefined, + }) + .resolves(successResponse); + const setupSuccessRequest = (overrides = {}, requestTimings?: number[]) => + setupUseRequest({ ...successRequest, ...overrides }, requestTimings); + const getSuccessResponse = () => ({ data: successResponse.data, error: null }); + + // Set up failed request helpers. + sendRequestSpy + .withArgs(errorRequest.path, { + body: JSON.stringify(errorRequest.body), + query: undefined, + }) + .rejects(errorResponse); + const setupErrorRequest = (overrides = {}, requestTimings?: number[]) => + setupUseRequest({ ...errorRequest, ...overrides }, requestTimings); + const getErrorResponse = () => ({ + data: null, + error: errorResponse.response.data, + }); + // We'll use this to change a success response to an error response, to test how the state changes. + const setErrorResponse = (overrides = {}) => { + element.setProps({ requestConfig: { ...errorRequest, ...overrides } }); + }; + + // Set up failed request helpers with the alternative error shape. + sendRequestSpy + .withArgs(errorWithBodyRequest.path, { + body: JSON.stringify(errorWithBodyRequest.body), + query: undefined, + }) + .rejects(errorWithBodyResponse); + const setupErrorWithBodyRequest = (overrides = {}) => + setupUseRequest({ ...errorWithBodyRequest, ...overrides }); + const getErrorWithBodyResponse = () => ({ + data: null, + error: errorWithBodyResponse.body, + }); + + return { + advanceTime, + completeRequest, + hookResult, + getSendRequestSpy: () => sendRequestSpy, + setupSuccessRequest, + getSuccessResponse, + setupErrorRequest, + getErrorResponse, + setErrorResponse, + setupErrorWithBodyRequest, + getErrorWithBodyResponse, + }; +}; diff --git a/src/plugins/es_ui_shared/public/request/use_request.test.ts b/src/plugins/es_ui_shared/public/request/use_request.test.ts new file mode 100644 index 0000000000000..f7902218d9314 --- /dev/null +++ b/src/plugins/es_ui_shared/public/request/use_request.test.ts @@ -0,0 +1,353 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { act } from 'react-dom/test-utils'; +import sinon from 'sinon'; + +import { + UseRequestHelpers, + REQUEST_TIME, + createUseRequestHelpers, +} from './use_request.test.helpers'; + +describe('useRequest hook', () => { + let helpers: UseRequestHelpers; + + beforeEach(() => { + helpers = createUseRequestHelpers(); + }); + + describe('parameters', () => { + describe('path, method, body', () => { + it('is used to send the request', async () => { + const { setupSuccessRequest, completeRequest, hookResult, getSuccessResponse } = helpers; + setupSuccessRequest(); + await completeRequest(); + expect(hookResult.data).toBe(getSuccessResponse().data); + }); + }); + + describe('pollIntervalMs', () => { + it('sends another request after the specified time has elapsed', async () => { + const { setupSuccessRequest, advanceTime, getSendRequestSpy } = helpers; + setupSuccessRequest({ pollIntervalMs: REQUEST_TIME }); + + await advanceTime(REQUEST_TIME); + expect(getSendRequestSpy().callCount).toBe(1); + + // We need to advance (1) the pollIntervalMs and (2) the request time. + await advanceTime(REQUEST_TIME * 2); + expect(getSendRequestSpy().callCount).toBe(2); + + // We need to advance (1) the pollIntervalMs and (2) the request time. + await advanceTime(REQUEST_TIME * 2); + expect(getSendRequestSpy().callCount).toBe(3); + }); + }); + + describe('initialData', () => { + it('sets the initial data value', async () => { + const { setupSuccessRequest, completeRequest, hookResult, getSuccessResponse } = helpers; + setupSuccessRequest({ initialData: 'initialData' }); + expect(hookResult.data).toBe('initialData'); + + // The initial data value will be overwritten once the request resolves. + await completeRequest(); + expect(hookResult.data).toBe(getSuccessResponse().data); + }); + }); + + describe('deserializer', () => { + it('is called with the response once the request resolves', async () => { + const { setupSuccessRequest, completeRequest, getSuccessResponse } = helpers; + + const deserializer = sinon.stub(); + setupSuccessRequest({ deserializer }); + sinon.assert.notCalled(deserializer); + await completeRequest(); + + sinon.assert.calledOnce(deserializer); + sinon.assert.calledWith(deserializer, getSuccessResponse().data); + }); + + it('provides the data return value', async () => { + const { setupSuccessRequest, completeRequest, hookResult } = helpers; + setupSuccessRequest({ deserializer: () => 'intercepted' }); + await completeRequest(); + expect(hookResult.data).toBe('intercepted'); + }); + }); + }); + + describe('state', () => { + describe('isInitialRequest', () => { + it('is true for the first request and false for subsequent requests', async () => { + const { setupSuccessRequest, completeRequest, hookResult } = helpers; + setupSuccessRequest(); + expect(hookResult.isInitialRequest).toBe(true); + + hookResult.sendRequest(); + await completeRequest(); + expect(hookResult.isInitialRequest).toBe(false); + }); + }); + + describe('isLoading', () => { + it('represents in-flight request status', async () => { + const { setupSuccessRequest, completeRequest, hookResult } = helpers; + setupSuccessRequest(); + expect(hookResult.isLoading).toBe(true); + + await completeRequest(); + expect(hookResult.isLoading).toBe(false); + }); + }); + + describe('error', () => { + it('surfaces errors from requests', async () => { + const { setupErrorRequest, completeRequest, hookResult, getErrorResponse } = helpers; + setupErrorRequest(); + await completeRequest(); + expect(hookResult.error).toBe(getErrorResponse().error); + }); + + it('surfaces body-shaped errors from requests', async () => { + const { + setupErrorWithBodyRequest, + completeRequest, + hookResult, + getErrorWithBodyResponse, + } = helpers; + + setupErrorWithBodyRequest(); + await completeRequest(); + expect(hookResult.error).toBe(getErrorWithBodyResponse().error); + }); + + it('persists while a request is in-flight', async () => { + const { setupErrorRequest, completeRequest, hookResult, getErrorResponse } = helpers; + setupErrorRequest(); + await completeRequest(); + expect(hookResult.isLoading).toBe(false); + expect(hookResult.error).toBe(getErrorResponse().error); + + act(() => { + hookResult.sendRequest(); + }); + expect(hookResult.isLoading).toBe(true); + expect(hookResult.error).toBe(getErrorResponse().error); + }); + + it('is null when the request is successful', async () => { + const { setupSuccessRequest, completeRequest, hookResult } = helpers; + setupSuccessRequest(); + expect(hookResult.error).toBeNull(); + + await completeRequest(); + expect(hookResult.isLoading).toBe(false); + expect(hookResult.error).toBeNull(); + }); + }); + + describe('data', () => { + it('surfaces payloads from requests', async () => { + const { setupSuccessRequest, completeRequest, hookResult, getSuccessResponse } = helpers; + setupSuccessRequest(); + expect(hookResult.data).toBeUndefined(); + + await completeRequest(); + expect(hookResult.data).toBe(getSuccessResponse().data); + }); + + it('persists while a request is in-flight', async () => { + const { setupSuccessRequest, completeRequest, hookResult, getSuccessResponse } = helpers; + setupSuccessRequest(); + await completeRequest(); + expect(hookResult.isLoading).toBe(false); + expect(hookResult.data).toBe(getSuccessResponse().data); + + act(() => { + hookResult.sendRequest(); + }); + expect(hookResult.isLoading).toBe(true); + expect(hookResult.data).toBe(getSuccessResponse().data); + }); + + it('persists from last successful request when the next request fails', async () => { + const { + setupSuccessRequest, + completeRequest, + hookResult, + getErrorResponse, + setErrorResponse, + getSuccessResponse, + } = helpers; + + setupSuccessRequest(); + await completeRequest(); + expect(hookResult.isLoading).toBe(false); + expect(hookResult.error).toBeNull(); + expect(hookResult.data).toBe(getSuccessResponse().data); + + setErrorResponse(); + await completeRequest(); + expect(hookResult.isLoading).toBe(false); + expect(hookResult.error).toBe(getErrorResponse().error); + expect(hookResult.data).toBe(getSuccessResponse().data); + }); + }); + }); + + describe('callbacks', () => { + describe('sendRequest', () => { + it('sends the request', async () => { + const { setupSuccessRequest, completeRequest, hookResult, getSendRequestSpy } = helpers; + setupSuccessRequest(); + + await completeRequest(); + expect(getSendRequestSpy().callCount).toBe(1); + + await act(async () => { + hookResult.sendRequest(); + await completeRequest(); + }); + expect(getSendRequestSpy().callCount).toBe(2); + }); + + it('resets the pollIntervalMs', async () => { + const { setupSuccessRequest, advanceTime, hookResult, getSendRequestSpy } = helpers; + const DOUBLE_REQUEST_TIME = REQUEST_TIME * 2; + setupSuccessRequest({ pollIntervalMs: DOUBLE_REQUEST_TIME }); + + // The initial request resolves, and then we'll immediately send a new one manually... + await advanceTime(REQUEST_TIME); + expect(getSendRequestSpy().callCount).toBe(1); + act(() => { + hookResult.sendRequest(); + }); + + // The manual request resolves, and we'll send yet another one... + await advanceTime(REQUEST_TIME); + expect(getSendRequestSpy().callCount).toBe(2); + act(() => { + hookResult.sendRequest(); + }); + + // At this point, we've moved forward 3s. The poll is set at 2s. If sendRequest didn't + // reset the poll, the request call count would be 4, not 3. + await advanceTime(REQUEST_TIME); + expect(getSendRequestSpy().callCount).toBe(3); + }); + }); + }); + + describe('request behavior', () => { + it('outdated responses are ignored by poll requests', async () => { + const { + setupSuccessRequest, + setErrorResponse, + completeRequest, + hookResult, + getErrorResponse, + getSendRequestSpy, + } = helpers; + const DOUBLE_REQUEST_TIME = REQUEST_TIME * 2; + // Send initial request, which will have a longer round-trip time. + setupSuccessRequest({}, [DOUBLE_REQUEST_TIME]); + + // Send a new request, which will have a shorter round-trip time. + setErrorResponse(); + + // Complete both requests. + await completeRequest(); + + // Two requests were sent... + expect(getSendRequestSpy().callCount).toBe(2); + // ...but the error response is the one that takes precedence because it was *sent* more + // recently, despite the success response *returning* more recently. + expect(hookResult.error).toBe(getErrorResponse().error); + expect(hookResult.data).toBeUndefined(); + }); + + it(`outdated responses are ignored if there's a more recently-sent manual request`, async () => { + const { setupSuccessRequest, advanceTime, hookResult, getSendRequestSpy } = helpers; + + const HALF_REQUEST_TIME = REQUEST_TIME * 0.5; + setupSuccessRequest({ pollIntervalMs: REQUEST_TIME }); + + // Before the original request resolves, we make a manual sendRequest call. + await advanceTime(HALF_REQUEST_TIME); + expect(getSendRequestSpy().callCount).toBe(0); + act(() => { + hookResult.sendRequest(); + }); + + // The original quest resolves but it's been marked as outdated by the the manual sendRequest + // call "interrupts", so data is left undefined. + await advanceTime(HALF_REQUEST_TIME); + expect(getSendRequestSpy().callCount).toBe(1); + expect(hookResult.data).toBeUndefined(); + }); + + it(`changing pollIntervalMs doesn't trigger a new request`, async () => { + const { setupErrorRequest, setErrorResponse, completeRequest, getSendRequestSpy } = helpers; + const DOUBLE_REQUEST_TIME = REQUEST_TIME * 2; + // Send initial request. + setupErrorRequest({ pollIntervalMs: REQUEST_TIME }); + + // Setting a new poll will schedule a second request, but not send one immediately. + setErrorResponse({ pollIntervalMs: DOUBLE_REQUEST_TIME }); + + // Complete initial request. + await completeRequest(); + + // Complete scheduled poll request. + await completeRequest(); + expect(getSendRequestSpy().callCount).toBe(2); + }); + + it('when the path changes after a request is scheduled, the scheduled request is sent with that path', async () => { + const { + setupSuccessRequest, + completeRequest, + hookResult, + getErrorResponse, + setErrorResponse, + getSendRequestSpy, + } = helpers; + const DOUBLE_REQUEST_TIME = REQUEST_TIME * 2; + + // Sned first request and schedule a request, both with the success path. + setupSuccessRequest({ pollIntervalMs: DOUBLE_REQUEST_TIME }); + + // Change the path to the error path, sending a second request. pollIntervalMs is the same + // so the originally scheduled poll remains cheduled. + setErrorResponse({ pollIntervalMs: DOUBLE_REQUEST_TIME }); + + // Complete the initial request, the requests by the path change, and the scheduled poll request. + await completeRequest(); + await completeRequest(); + + // If the scheduled poll request was sent to the success path, we wouldn't have an error result. + // But we do, because it was sent to the error path. + expect(getSendRequestSpy().callCount).toBe(3); + expect(hookResult.error).toBe(getErrorResponse().error); + }); + }); +}); diff --git a/src/plugins/es_ui_shared/public/request/use_request.ts b/src/plugins/es_ui_shared/public/request/use_request.ts new file mode 100644 index 0000000000000..481843bf40e88 --- /dev/null +++ b/src/plugins/es_ui_shared/public/request/use_request.ts @@ -0,0 +1,161 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { useEffect, useCallback, useState, useRef, useMemo } from 'react'; + +import { HttpSetup } from '../../../../../src/core/public'; +import { + sendRequest as sendStatelessRequest, + SendRequestConfig, + SendRequestResponse, +} from './send_request'; + +export interface UseRequestConfig extends SendRequestConfig { + pollIntervalMs?: number; + initialData?: any; + deserializer?: (data: any) => any; +} + +export interface UseRequestResponse { + isInitialRequest: boolean; + isLoading: boolean; + error: E | null; + data?: D | null; + sendRequest: () => Promise>; +} + +export const useRequest = ( + httpClient: HttpSetup, + { path, method, query, body, pollIntervalMs, initialData, deserializer }: UseRequestConfig +): UseRequestResponse => { + const isMounted = useRef(false); + + // Main states for tracking request status and data + const [error, setError] = useState(null); + const [isLoading, setIsLoading] = useState(true); + const [data, setData] = useState(initialData); + + // Consumers can use isInitialRequest to implement a polling UX. + const requestCountRef = useRef(0); + const isInitialRequest = requestCountRef.current === 0; + const pollIntervalIdRef = useRef(null); + + const clearPollInterval = useCallback(() => { + if (pollIntervalIdRef.current) { + clearTimeout(pollIntervalIdRef.current); + pollIntervalIdRef.current = null; + } + }, []); + + // Convert our object to string to be able to compare them in our useMemo, + // allowing the consumer to freely passed new objects to the hook on each + // render without requiring them to be memoized. + const queryStringified = query ? JSON.stringify(query) : undefined; + const bodyStringified = body ? JSON.stringify(body) : undefined; + + const requestBody = useMemo(() => { + return { + path, + method, + query: queryStringified ? query : undefined, + body: bodyStringified ? body : undefined, + }; + // queryStringified and bodyStringified stand in for query and body as dependencies. + /* eslint-disable-next-line react-hooks/exhaustive-deps */ + }, [path, method, queryStringified, bodyStringified]); + + const sendRequest = useCallback(async () => { + // If we're on an interval, this allows us to reset it if the user has manually requested the + // data, to avoid doubled-up requests. + clearPollInterval(); + + const requestId = ++requestCountRef.current; + + // We don't clear error or data, so it's up to the consumer to decide whether to display the + // "old" error/data or loading state when a new request is in-flight. + setIsLoading(true); + + const response = await sendStatelessRequest(httpClient, requestBody); + const { data: serializedResponseData, error: responseError } = response; + + const isOutdatedRequest = requestId !== requestCountRef.current; + const isUnmounted = isMounted.current === false; + + // Ignore outdated or irrelevant data. + if (isOutdatedRequest || isUnmounted) { + return { data: null, error: null }; + } + + setError(responseError); + // If there's an error, keep the data from the last request in case it's still useful to the user. + if (!responseError) { + const responseData = deserializer + ? deserializer(serializedResponseData) + : serializedResponseData; + setData(responseData); + } + // Setting isLoading to false also acts as a signal for scheduling the next poll request. + setIsLoading(false); + + return { data: serializedResponseData, error: responseError }; + }, [requestBody, httpClient, deserializer, clearPollInterval]); + + const scheduleRequest = useCallback(() => { + // If there's a scheduled poll request, this new one will supersede it. + clearPollInterval(); + + if (pollIntervalMs) { + pollIntervalIdRef.current = setTimeout(sendRequest, pollIntervalMs); + } + }, [pollIntervalMs, sendRequest, clearPollInterval]); + + // Send the request on component mount and whenever the dependencies of sendRequest() change. + useEffect(() => { + sendRequest(); + }, [sendRequest]); + + // Schedule the next poll request when the previous one completes. + useEffect(() => { + // When a request completes, attempt to schedule the next one. Note that we aren't re-scheduling + // a request whenever sendRequest's dependencies change. isLoading isn't set to false until the + // initial request has completed, so we won't schedule a request on mount. + if (!isLoading) { + scheduleRequest(); + } + }, [isLoading, scheduleRequest]); + + useEffect(() => { + isMounted.current = true; + + return () => { + isMounted.current = false; + + // Clean up on unmount. + clearPollInterval(); + }; + }, [clearPollInterval]); + + return { + isInitialRequest, + isLoading, + error, + data, + sendRequest, // Gives the user the ability to manually request data + }; +}; 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 e02c9b6c18615..b83b0af5f97c6 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 @@ -39,6 +39,8 @@ export const RangeField = ({ field, euiFieldProps = {}, ...rest }: Props) => { }>; field.onChange(event); }, + // https://github.com/elastic/kibana/issues/73972 + /* eslint-disable-next-line react-hooks/exhaustive-deps */ [field.onChange] ); diff --git a/x-pack/plugins/index_management/public/shared_imports.ts b/x-pack/plugins/index_management/public/shared_imports.ts index 16dcab18c3caf..2ba2a5c493c49 100644 --- a/x-pack/plugins/index_management/public/shared_imports.ts +++ b/x-pack/plugins/index_management/public/shared_imports.ts @@ -13,7 +13,7 @@ export { Forms, extractQueryParams, GlobalFlyout, -} from '../../../../src/plugins/es_ui_shared/public/'; +} from '../../../../src/plugins/es_ui_shared/public'; export { FormSchema, diff --git a/x-pack/plugins/watcher/public/application/lib/api.ts b/x-pack/plugins/watcher/public/application/lib/api.ts index 82ec2925ba6dc..4f42b26d51c46 100644 --- a/x-pack/plugins/watcher/public/application/lib/api.ts +++ b/x-pack/plugins/watcher/public/application/lib/api.ts @@ -35,44 +35,53 @@ export const getSavedObjectsClient = () => savedObjectsClient; const basePath = ROUTES.API_ROOT; +const loadWatchesDeserializer = ({ watches = [] }: { watches: any[] }) => { + return watches.map((watch: any) => Watch.fromUpstreamJson(watch)); +}; + export const useLoadWatches = (pollIntervalMs: number) => { return useRequest({ path: `${basePath}/watches`, method: 'get', pollIntervalMs, - deserializer: ({ watches = [] }: { watches: any[] }) => { - return watches.map((watch: any) => Watch.fromUpstreamJson(watch)); - }, + deserializer: loadWatchesDeserializer, }); }; +const loadWatchDetailDeserializer = ({ watch = {} }: { watch: any }) => + Watch.fromUpstreamJson(watch); + export const useLoadWatchDetail = (id: string) => { return useRequest({ path: `${basePath}/watch/${id}`, method: 'get', - deserializer: ({ watch = {} }: { watch: any }) => Watch.fromUpstreamJson(watch), + deserializer: loadWatchDetailDeserializer, }); }; +const loadWatchHistoryDeserializer = ({ watchHistoryItems = [] }: { watchHistoryItems: any }) => { + return watchHistoryItems.map((historyItem: any) => + WatchHistoryItem.fromUpstreamJson(historyItem) + ); +}; + export const useLoadWatchHistory = (id: string, startTime: string) => { return useRequest({ query: startTime ? { startTime } : undefined, path: `${basePath}/watch/${id}/history`, method: 'get', - deserializer: ({ watchHistoryItems = [] }: { watchHistoryItems: any }) => { - return watchHistoryItems.map((historyItem: any) => - WatchHistoryItem.fromUpstreamJson(historyItem) - ); - }, + deserializer: loadWatchHistoryDeserializer, }); }; +const loadWatchHistoryDetailDeserializer = ({ watchHistoryItem }: { watchHistoryItem: any }) => + WatchHistoryItem.fromUpstreamJson(watchHistoryItem); + export const useLoadWatchHistoryDetail = (id: string | undefined) => { return useRequest({ path: !id ? '' : `${basePath}/history/${id}`, method: 'get', - deserializer: ({ watchHistoryItem }: { watchHistoryItem: any }) => - WatchHistoryItem.fromUpstreamJson(watchHistoryItem), + deserializer: loadWatchHistoryDetailDeserializer, }); }; @@ -148,6 +157,8 @@ export const loadIndexPatterns = async () => { return savedObjects; }; +const getWatchVisualizationDataDeserializer = (data: { visualizeData: any }) => data?.visualizeData; + export const useGetWatchVisualizationData = (watchModel: BaseWatch, visualizeOptions: any) => { return useRequest({ path: `${basePath}/watch/visualize`, @@ -156,21 +167,23 @@ export const useGetWatchVisualizationData = (watchModel: BaseWatch, visualizeOpt watch: watchModel.upstreamJson, options: visualizeOptions.upstreamJson, }), - deserializer: (data: { visualizeData: any }) => data?.visualizeData, + deserializer: getWatchVisualizationDataDeserializer, }); }; +const loadSettingsDeserializer = (data: { + action_types: { + [key: string]: { + enabled: boolean; + }; + }; +}) => Settings.fromUpstreamJson(data); + export const useLoadSettings = () => { return useRequest({ path: `${basePath}/settings`, method: 'get', - deserializer: (data: { - action_types: { - [key: string]: { - enabled: boolean; - }; - }; - }) => Settings.fromUpstreamJson(data), + deserializer: loadSettingsDeserializer, }); }; diff --git a/x-pack/plugins/watcher/public/application/sections/watch_edit/components/threshold_watch_edit/watch_visualization.tsx b/x-pack/plugins/watcher/public/application/sections/watch_edit/components/threshold_watch_edit/watch_visualization.tsx index c0d906114277e..2ff0f53d07e91 100644 --- a/x-pack/plugins/watcher/public/application/sections/watch_edit/components/threshold_watch_edit/watch_visualization.tsx +++ b/x-pack/plugins/watcher/public/application/sections/watch_edit/components/threshold_watch_edit/watch_visualization.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { Fragment, useContext, useEffect } from 'react'; +import React, { Fragment, useContext, useEffect, useMemo } from 'react'; import { AnnotationDomainTypes, Axis, @@ -105,7 +105,9 @@ export const WatchVisualization = () => { threshold, } = watch; - const domain = getDomain(watch); + // Only recalculate the domain if the watch configuration changes. This prevents the visualization + // request's resolution from re-triggering itself in an infinite loop. + const domain = useMemo(() => getDomain(watch), [watch]); const timeBuckets = createTimeBuckets(); timeBuckets.setBounds(domain); const interval = timeBuckets.getInterval().expression; diff --git a/x-pack/plugins/watcher/public/application/shared_imports.ts b/x-pack/plugins/watcher/public/application/shared_imports.ts index a9e07b80a9b22..766e8e659c8ae 100644 --- a/x-pack/plugins/watcher/public/application/shared_imports.ts +++ b/x-pack/plugins/watcher/public/application/shared_imports.ts @@ -10,6 +10,6 @@ export { UseRequestConfig, sendRequest, useRequest, -} from '../../../../../src/plugins/es_ui_shared/public/'; +} from '../../../../../src/plugins/es_ui_shared/public'; export { useXJsonMode } from '../../../../../src/plugins/es_ui_shared/static/ace_x_json/hooks';