From 4acad8fae23b772b72ef48db44574c120fd9b0f8 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Tue, 7 May 2024 12:00:06 +0200 Subject: [PATCH] fix to reduce rerenders --- .../api/stream_reducer.ts | 6 +- .../response_stream/client/string_reducer.ts | 12 +--- .../client/use_fetch_stream.ts | 69 ++++++++++++++----- 3 files changed, 57 insertions(+), 30 deletions(-) diff --git a/x-pack/packages/ml/aiops_log_rate_analysis/api/stream_reducer.ts b/x-pack/packages/ml/aiops_log_rate_analysis/api/stream_reducer.ts index ca6148c133ccad..c4bde0b90c0fd9 100644 --- a/x-pack/packages/ml/aiops_log_rate_analysis/api/stream_reducer.ts +++ b/x-pack/packages/ml/aiops_log_rate_analysis/api/stream_reducer.ts @@ -34,12 +34,8 @@ export const initialState: StreamState = { export function streamReducer( state: StreamState, - action: AiopsLogRateAnalysisApiAction | AiopsLogRateAnalysisApiAction[] + action: AiopsLogRateAnalysisApiAction ): StreamState { - if (Array.isArray(action)) { - return action.reduce(streamReducer, state); - } - switch (action.type) { case API_ACTION_NAME.ADD_SIGNIFICANT_ITEMS: return { ...state, significantItems: [...state.significantItems, ...action.payload] }; diff --git a/x-pack/packages/ml/response_stream/client/string_reducer.ts b/x-pack/packages/ml/response_stream/client/string_reducer.ts index f77b31e1fed1e0..d14990947fd059 100644 --- a/x-pack/packages/ml/response_stream/client/string_reducer.ts +++ b/x-pack/packages/ml/response_stream/client/string_reducer.ts @@ -7,20 +7,14 @@ import type { Reducer, ReducerAction, ReducerState } from 'react'; -type StringReducerPayload = string | string[] | undefined; +type StringReducerPayload = string | undefined; export type StringReducer = Reducer; /** * The `stringReducer` is provided to handle plain string based streams with `streamFactory()`. * * @param state - The current state, being the string fetched so far. - * @param payload — The state update can be a plain string, an array of strings or `undefined`. - * * An array of strings will be joined without a delimiter and added to the current string. - * In combination with `useFetchStream`'s buffering this allows to do bulk updates - * within the reducer without triggering a React/DOM update on every stream chunk. - * * `undefined` can be used to reset the state to an empty string, for example, when a - * UI has the option to trigger a refetch of a stream. - * + * @param payload — The state update can be a plain string to be added or `undefined` to reset the state. * @returns The updated state, a string that combines the previous string and the payload. */ export function stringReducer( @@ -31,5 +25,5 @@ export function stringReducer( return ''; } - return `${state}${Array.isArray(payload) ? payload.join('') : payload}`; + return `${state}${payload}`; } diff --git a/x-pack/packages/ml/response_stream/client/use_fetch_stream.ts b/x-pack/packages/ml/response_stream/client/use_fetch_stream.ts index be8e6cb4d9a33e..ff51af87f30986 100644 --- a/x-pack/packages/ml/response_stream/client/use_fetch_stream.ts +++ b/x-pack/packages/ml/response_stream/client/use_fetch_stream.ts @@ -7,14 +7,12 @@ import { useEffect, - useReducer, useRef, useState, type Reducer, - type ReducerAction, type ReducerState, + type ReducerAction, } from 'react'; -import useThrottle from 'react-use/lib/useThrottle'; import type { HttpSetup, HttpFetchOptions } from '@kbn/core/public'; import { isPopulatedObject } from '@kbn/ml-is-populated-object'; @@ -22,6 +20,8 @@ import { isPopulatedObject } from '@kbn/ml-is-populated-object'; import { fetchStream } from './fetch_stream'; import { stringReducer, type StringReducer } from './string_reducer'; +const DATA_THROTTLE_MS = 100; + // This pattern with a dual ternary allows us to default to StringReducer // and if a custom reducer is supplied fall back to that one instead. // The complexity in here allows us to create a simpler API surface where @@ -57,6 +57,7 @@ function isReducerOptions(arg: unknown): arg is CustomReducer { * @param apiVersion Optional API version. * @param body Optional API request body. * @param customReducer Optional custom reducer and initial state. + * @param headers Optional headers. * @returns An object with streaming data and methods to act on the stream. */ export function useFetchStream>( @@ -75,11 +76,41 @@ export function useFetchStream>( ? customReducer : ({ reducer: stringReducer, initialState: '' } as FetchStreamCustomReducer); - const [data, dispatch] = useReducer( - reducerWithFallback.reducer, - reducerWithFallback.initialState - ); - const dataThrottled = useThrottle(data, 100); + // We used `useReducer` in previous iterations of this hook, but it caused + // a lot of unnecessary re-renders even in combination with `useThrottle`. + // We're now using `dataRef` to allow updates outside of the render cycle. + // When the stream is running, we'll update `data` with the `dataRef` value + // periodically. + const [data, setData] = useState(reducerWithFallback.initialState); + const dataRef = useRef(reducerWithFallback.initialState); + + // This effect is used to throttle the data updates while the stream is running. + // It will update the `data` state with the current `dataRef` value every 100ms. + useEffect(() => { + // We cannot check against `isRunning` in the `setTimeout` callback, because + // we would check against a stale value. Instead, we use a mutable + // object to keep track of the current state of the effect. + const effectState = { isActive: true }; + + if (isRunning) { + setData(dataRef.current); + + function updateData() { + setTimeout(() => { + setData(dataRef.current); + if (effectState.isActive) { + updateData(); + } + }, DATA_THROTTLE_MS); + } + + updateData(); + } + + return () => { + effectState.isActive = false; + }; + }, [isRunning]); const abortCtrl = useRef(new AbortController()); @@ -99,7 +130,7 @@ export function useFetchStream>( abortCtrl.current = new AbortController(); - for await (const [fetchStreamError, actions] of fetchStream>( + for await (const [fetchStreamError, action] of fetchStream>( http, endpoint, apiVersion, @@ -110,16 +141,22 @@ export function useFetchStream>( )) { if (fetchStreamError !== null) { addError(fetchStreamError); - } else if (Array.isArray(actions) && actions.length > 0) { - for (const action of actions) { - dispatch(action as ReducerAction>); - } + } else if (action) { + dataRef.current = reducerWithFallback.reducer(dataRef.current, action) as ReducerState< + CustomReducer + >; } } setIsRunning(false); }; + const dispatch = (action: ReducerAction['reducer']>) => { + dataRef.current = reducerWithFallback.reducer(dataRef.current, action) as ReducerState< + CustomReducer + >; + }; + const cancel = () => { abortCtrl.current.abort(); setIsCancelled(true); @@ -133,10 +170,10 @@ export function useFetchStream>( return { cancel, - // To avoid a race condition where the stream already ended but `useThrottle` would - // yet have to trigger another update within the throttling interval, we'll return + // To avoid a race condition where the stream already ended but the throttling would + // yet have to trigger another update within the interval, we'll return // the unthrottled data once the stream is complete. - data: isRunning ? dataThrottled : data, + data: isRunning ? data : dataRef.current, dispatch, errors, isCancelled,