From 2851dfeed634b75ed888e7afc948663f7143406f Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Tue, 26 Oct 2021 17:14:56 -0400 Subject: [PATCH] use consistent IDs for control group. Clear redux state when embeddable is destroyed. Do not combine reducers when the reducer already exists. Destroy and unmount control group --- .../dashboard_container_factory.tsx | 3 +- .../lib/dashboard_control_group.ts | 7 +- .../component/control_frame_component.tsx | 4 +- .../embeddable/control_group_container.tsx | 6 ++ .../generic_embeddable_store.ts | 10 +-- .../redux_embeddable_wrapper.tsx | 68 ++++++++++++------- 6 files changed, 64 insertions(+), 34 deletions(-) diff --git a/src/plugins/dashboard/public/application/embeddable/dashboard_container_factory.tsx b/src/plugins/dashboard/public/application/embeddable/dashboard_container_factory.tsx index b13741d6762e2..1cb8d4ae269ef 100644 --- a/src/plugins/dashboard/public/application/embeddable/dashboard_container_factory.tsx +++ b/src/plugins/dashboard/public/application/embeddable/dashboard_container_factory.tsx @@ -9,7 +9,6 @@ import { i18n } from '@kbn/i18n'; import { EmbeddablePersistableStateService } from 'src/plugins/embeddable/common'; -import uuid from 'uuid'; import { DashboardContainerInput } from '../..'; import { DASHBOARD_CONTAINER_TYPE } from './dashboard_constants'; import type { DashboardContainer, DashboardContainerServices } from './dashboard_container'; @@ -84,7 +83,7 @@ export class DashboardContainerFactoryDefinition ...getDefaultDashboardControlGroupInput(), ...(initialInput.controlGroupInput ?? {}), viewMode: initialInput.viewMode, - id: uuid.v4(), + id: `control_group_${initialInput.id ?? 'new_dashboard'}`, }); const { DashboardContainer: DashboardContainerEmbeddable } = await import( './dashboard_container' diff --git a/src/plugins/dashboard/public/application/lib/dashboard_control_group.ts b/src/plugins/dashboard/public/application/lib/dashboard_control_group.ts index 83bd3bac6f7a0..aaf6c5f0af4fc 100644 --- a/src/plugins/dashboard/public/application/lib/dashboard_control_group.ts +++ b/src/plugins/dashboard/public/application/lib/dashboard_control_group.ts @@ -133,7 +133,12 @@ export const syncDashboardControlGroup = async ({ .subscribe(() => dashboardContainer.updateInput({ lastReloadRequestTime: Date.now() })) ); - return { onDestroyControlGroup: () => subscriptions.unsubscribe() }; + return { + onDestroyControlGroup: () => { + subscriptions.unsubscribe(); + controlGroup.destroy(); + }, + }; }; export const controlGroupInputIsEqual = ( diff --git a/src/plugins/presentation_util/public/components/controls/control_group/component/control_frame_component.tsx b/src/plugins/presentation_util/public/components/controls/control_group/component/control_frame_component.tsx index e14122ebb41d8..f94d2f8fee0dc 100644 --- a/src/plugins/presentation_util/public/components/controls/control_group/component/control_frame_component.tsx +++ b/src/plugins/presentation_util/public/components/controls/control_group/component/control_frame_component.tsx @@ -53,7 +53,9 @@ export const ControlFrame = ({ customPrepend, enableActions, embeddableId }: Con embeddable.render(embeddableRoot.current); } const subscription = embeddable?.getInput$().subscribe((newInput) => setTitle(newInput.title)); - return () => subscription?.unsubscribe(); + return () => { + subscription?.unsubscribe(); + }; }, [embeddable, embeddableRoot]); const floatingActions = ( diff --git a/src/plugins/presentation_util/public/components/controls/control_group/embeddable/control_group_container.tsx b/src/plugins/presentation_util/public/components/controls/control_group/embeddable/control_group_container.tsx index 06fb86f022253..ff25286a75211 100644 --- a/src/plugins/presentation_util/public/components/controls/control_group/embeddable/control_group_container.tsx +++ b/src/plugins/presentation_util/public/components/controls/control_group/embeddable/control_group_container.tsx @@ -43,6 +43,7 @@ export class ControlGroupContainer extends Container< > { public readonly type = CONTROL_GROUP_TYPE; private subscriptions: Subscription = new Subscription(); + private domNode?: HTMLElement; public untilReady = () => { const panelsLoading = () => @@ -133,9 +134,14 @@ export class ControlGroupContainer extends Container< public destroy() { super.destroy(); this.subscriptions.unsubscribe(); + if (this.domNode) ReactDOM.unmountComponentAtNode(this.domNode); } public render(dom: HTMLElement) { + if (this.domNode) { + ReactDOM.unmountComponentAtNode(this.domNode); + } + this.domNode = dom; const PresentationUtilProvider = pluginServices.getContextProvider(); ReactDOM.render( diff --git a/src/plugins/presentation_util/public/components/redux_embeddables/generic_embeddable_store.ts b/src/plugins/presentation_util/public/components/redux_embeddables/generic_embeddable_store.ts index 22883b8f0c813..fe5a647e7e327 100644 --- a/src/plugins/presentation_util/public/components/redux_embeddables/generic_embeddable_store.ts +++ b/src/plugins/presentation_util/public/components/redux_embeddables/generic_embeddable_store.ts @@ -27,10 +27,12 @@ managedEmbeddablesStore.injectReducer = ({ key, asyncReducer, }: InjectReducerProps) => { - managedEmbeddablesStore.asyncReducers[key] = asyncReducer as Reducer; - managedEmbeddablesStore.replaceReducer( - combineReducers({ ...managedEmbeddablesStore.asyncReducers }) - ); + if (!managedEmbeddablesStore.asyncReducers[key]) { + managedEmbeddablesStore.asyncReducers[key] = asyncReducer as Reducer; + managedEmbeddablesStore.replaceReducer( + combineReducers({ ...managedEmbeddablesStore.asyncReducers }) + ); + } }; /** diff --git a/src/plugins/presentation_util/public/components/redux_embeddables/redux_embeddable_wrapper.tsx b/src/plugins/presentation_util/public/components/redux_embeddables/redux_embeddable_wrapper.tsx index 59f29545078d4..9e7b53fb21c3b 100644 --- a/src/plugins/presentation_util/public/components/redux_embeddables/redux_embeddable_wrapper.tsx +++ b/src/plugins/presentation_util/public/components/redux_embeddables/redux_embeddable_wrapper.tsx @@ -10,7 +10,7 @@ import { Provider, TypedUseSelectorHook, useDispatch, useSelector } from 'react- import { SliceCaseReducers, PayloadAction, createSlice } from '@reduxjs/toolkit'; import React, { PropsWithChildren, useEffect, useMemo, useRef } from 'react'; import { Draft } from 'immer/dist/types/types-external'; -import { debounceTime } from 'rxjs/operators'; +import { debounceTime, finalize } from 'rxjs/operators'; import { Filter } from '@kbn/es-query'; import { isEqual } from 'lodash'; @@ -101,6 +101,12 @@ export const ReduxEmbeddableWrapper = { const key = `${embeddable.type}_${embeddable.id}`; + const store = getManagedEmbeddablesStore(); + + const initialState = getExplicitInput(embeddable); + if (stateContainsFilters(initialState)) { + initialState.filters = cleanFiltersForSerialize(initialState.filters); + } // A generic reducer used to update redux state when the embeddable input changes const updateEmbeddableReduxState = ( @@ -110,21 +116,28 @@ export const ReduxEmbeddableWrapper = (embeddable); - if (stateContainsFilters(initialState)) { - initialState.filters = cleanFiltersForSerialize(initialState.filters); - } + // A generic reducer used to clear redux state when the embeddable is destroyed + const clearEmbeddableReduxState = () => { + return undefined; + }; + const slice = createSlice>({ initialState, name: key, - reducers: { ...reducers, updateEmbeddableReduxState }, + reducers: { ...reducers, updateEmbeddableReduxState, clearEmbeddableReduxState }, }); - const store = getManagedEmbeddablesStore(); - store.injectReducer({ - key, - asyncReducer: slice.reducer, - }); + if (store.asyncReducers[key]) { + // if the store already has reducers set up for this embeddable type & id, update the existing state. + const updateExistingState = (slice.actions as ReduxEmbeddableContextServices['actions']) + .updateEmbeddableReduxState; + store.dispatch(updateExistingState(initialState)); + } else { + store.injectReducer({ + key, + asyncReducer: slice.reducer, + }); + } const useEmbeddableSelector: TypedUseSelectorHook = () => useSelector((state: ReturnType) => state[key]); @@ -165,7 +178,7 @@ const ReduxEmbeddableSync = (); const dispatch = useEmbeddableDispatch(); @@ -177,22 +190,25 @@ const ReduxEmbeddableSync = { - const differences = diffInput(getExplicitInput(embeddable), stateRef.current); - if (differences && Object.keys(differences).length > 0) { - if (stateContainsFilters(differences)) { - differences.filters = cleanFiltersForSerialize(differences.filters); - } - dispatch(updateEmbeddableReduxState(differences)); + .pipe( + finalize(() => { + // empty redux store, when embeddable is destroyed. + destroyedRef.current = true; + dispatch(clearEmbeddableReduxState(undefined)); + }), + debounceTime(0) + ) // debounce input changes to ensure that when many updates are made in one render the latest wins out + .subscribe(() => { + const differences = diffInput(getExplicitInput(embeddable), stateRef.current); + if (differences && Object.keys(differences).length > 0) { + if (stateContainsFilters(differences)) { + differences.filters = cleanFiltersForSerialize(differences.filters); } - }, - undefined, - () => (destroyedRef.current = true) // when input observer is complete, embeddable is destroyed - ); + dispatch(updateEmbeddableReduxState(differences)); + } + }); return () => inputSubscription.unsubscribe(); - }, [diffInput, dispatch, embeddable, updateEmbeddableReduxState]); + }, [diffInput, dispatch, embeddable, updateEmbeddableReduxState, clearEmbeddableReduxState]); useEffect(() => { if (isErrorEmbeddable(embeddable) || destroyedRef.current) return;