Skip to content

Commit

Permalink
use consistent IDs for control group. Clear redux state when embeddab…
Browse files Browse the repository at this point in the history
…le is destroyed. Do not combine reducers when the reducer already exists. Destroy and unmount control group
  • Loading branch information
ThomThomson committed Oct 26, 2021
1 parent 497c057 commit 2851dfe
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () =>
Expand Down Expand Up @@ -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(
<PresentationUtilProvider>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@ managedEmbeddablesStore.injectReducer = <StateShape>({
key,
asyncReducer,
}: InjectReducerProps<StateShape>) => {
managedEmbeddablesStore.asyncReducers[key] = asyncReducer as Reducer<unknown>;
managedEmbeddablesStore.replaceReducer(
combineReducers({ ...managedEmbeddablesStore.asyncReducers })
);
if (!managedEmbeddablesStore.asyncReducers[key]) {
managedEmbeddablesStore.asyncReducers[key] = asyncReducer as Reducer<unknown>;
managedEmbeddablesStore.replaceReducer(
combineReducers({ ...managedEmbeddablesStore.asyncReducers })
);
}
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -101,6 +101,12 @@ export const ReduxEmbeddableWrapper = <InputType extends EmbeddableInput = Embed
const reduxEmbeddableContext: ReduxEmbeddableContextServices | ReduxContainerContextServices =
useMemo(() => {
const key = `${embeddable.type}_${embeddable.id}`;
const store = getManagedEmbeddablesStore();

const initialState = getExplicitInput<InputType>(embeddable);
if (stateContainsFilters(initialState)) {
initialState.filters = cleanFiltersForSerialize(initialState.filters);
}

// A generic reducer used to update redux state when the embeddable input changes
const updateEmbeddableReduxState = (
Expand All @@ -110,21 +116,28 @@ export const ReduxEmbeddableWrapper = <InputType extends EmbeddableInput = Embed
return { ...state, ...action.payload };
};

const initialState = getExplicitInput<InputType>(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<InputType, SliceCaseReducers<InputType>>({
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<InputType> = () =>
useSelector((state: ReturnType<typeof store.getState>) => state[key]);
Expand Down Expand Up @@ -165,7 +178,7 @@ const ReduxEmbeddableSync = <InputType extends EmbeddableInput = EmbeddableInput
const {
useEmbeddableSelector,
useEmbeddableDispatch,
actions: { updateEmbeddableReduxState },
actions: { updateEmbeddableReduxState, clearEmbeddableReduxState },
} = useReduxEmbeddableContext<InputType>();

const dispatch = useEmbeddableDispatch();
Expand All @@ -177,22 +190,25 @@ const ReduxEmbeddableSync = <InputType extends EmbeddableInput = EmbeddableInput
// When Embeddable Input changes, push differences to redux.
const inputSubscription = embeddable
.getInput$()
.pipe(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<InputType>(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<InputType>(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;
Expand Down

0 comments on commit 2851dfe

Please sign in to comment.