From 5981002b699be7dcfc7daa90a87c976dcd24515d Mon Sep 17 00:00:00 2001 From: Ashwin P Chandran Date: Mon, 17 Jul 2023 08:04:18 +0000 Subject: [PATCH] Working multi view state management Signed-off-by: Ashwin P Chandran --- .../public/components/app_container.tsx | 68 +++---------------- src/plugins/data_explorer/public/index.ts | 4 +- .../public/services/view_service/types.ts | 12 ++-- .../public/services/view_service/view.ts | 6 +- .../public/utils/state_management/hooks.ts | 7 +- .../public/utils/state_management/preload.ts | 3 +- .../utils/state_management/discover_slice.tsx | 21 ++---- .../utils/state_management/index.ts | 13 ++++ .../view_components/canvas/canvas.tsx | 13 ++-- .../view_components/canvas/index.tsx | 37 ++++------ .../view_components/panel/index.tsx | 18 +++-- .../view_components/panel/panel.tsx | 4 +- src/plugins/discover/public/build_services.ts | 4 +- src/plugins/discover/public/plugin.ts | 30 +++----- 14 files changed, 84 insertions(+), 156 deletions(-) diff --git a/src/plugins/data_explorer/public/components/app_container.tsx b/src/plugins/data_explorer/public/components/app_container.tsx index 8ecd045a2450..91b75a12423f 100644 --- a/src/plugins/data_explorer/public/components/app_container.tsx +++ b/src/plugins/data_explorer/public/components/app_container.tsx @@ -3,80 +3,31 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React, { useLayoutEffect, useRef, useState } from 'react'; +import React from 'react'; import { EuiPageTemplate } from '@elastic/eui'; +import { Suspense } from 'react'; import { AppMountParameters } from '../../../../core/public'; import { Sidebar } from './sidebar'; import { NoView } from './no_view'; import { View } from '../services/view_service/view'; -import { useOpenSearchDashboards } from '../../../opensearch_dashboards_react/public'; -import { DataExplorerServices } from '../types'; export const AppContainer = ({ view, params }: { view?: View; params: AppMountParameters }) => { - const [showSpinner, setShowSpinner] = useState(false); - const { - services: { store }, - } = useOpenSearchDashboards(); - const canvasRef = useRef(null); - const panelRef = useRef(null); - const unmountRef = useRef(null); - - useLayoutEffect(() => { - const unmount = () => { - if (unmountRef.current) { - unmountRef.current(); - unmountRef.current = null; - } - }; - - // Do nothing if the view is not defined or if the view is the same as the previous view - if (!view || (unmountRef.current && unmountRef.current.viewId === view.id)) { - return; - } - - // unmount the previous view - unmount(); - - const mount = async () => { - setShowSpinner(true); - try { - unmountRef.current = - (await view.mount({ - canvasElement: canvasRef.current!, - panelElement: panelRef.current!, - appParams: params, - // The provider is added to the services right after the store is created. so it is safe to assume its here. - store: store!, - })) || null; - } catch (e) { - // TODO: add error UI - // eslint-disable-next-line no-console - console.error(e); - } finally { - // if (canvasRef.current && panelRef.current) { - if (canvasRef.current) { - setShowSpinner(false); - } - } - }; - - mount(); - - return unmount; - }, [params, view, store]); - // TODO: Make this more robust. if (!view) { return ; } + const { Canvas, Panel } = view; + // Render the application DOM. // Note that `navigation.ui.TopNavMenu` is a stateful component exported on the `navigation` plugin's start contract. return ( -
+ Loading...
}> + + } className="dePageTemplate" @@ -85,8 +36,9 @@ export const AppContainer = ({ view, params }: { view?: View; params: AppMountPa paddingSize="none" > {/* TODO: improve loading state */} - {showSpinner &&
Loading...
} -
+ Loading...
}> + +
); }; diff --git a/src/plugins/data_explorer/public/index.ts b/src/plugins/data_explorer/public/index.ts index eb0db4303753..0a0575e339c1 100644 --- a/src/plugins/data_explorer/public/index.ts +++ b/src/plugins/data_explorer/public/index.ts @@ -13,5 +13,5 @@ export function plugin() { return new DataExplorerPlugin(); } export { DataExplorerPluginSetup, DataExplorerPluginStart, ViewRedirectParams } from './types'; -export { ViewMountParameters, ViewDefinition } from './services/view_service'; -export { RootState as DataExplorerRootState } from './utils/state_management'; +export { ViewProps, ViewDefinition } from './services/view_service'; +export { RootState, useTypedSelector, useTypedDispatch } from './utils/state_management'; diff --git a/src/plugins/data_explorer/public/services/view_service/types.ts b/src/plugins/data_explorer/public/services/view_service/types.ts index ed34f5d32479..2aa3915da468 100644 --- a/src/plugins/data_explorer/public/services/view_service/types.ts +++ b/src/plugins/data_explorer/public/services/view_service/types.ts @@ -4,8 +4,8 @@ */ import { Slice } from '@reduxjs/toolkit'; +import { LazyExoticComponent } from 'react'; import { AppMountParameters } from '../../../../../core/public'; -import { Store } from '../../utils/state_management'; // TODO: State management props @@ -14,12 +14,7 @@ interface ViewListItem { label: string; } -export interface ViewMountParameters { - canvasElement: HTMLDivElement; - panelElement: HTMLDivElement; - appParams: AppMountParameters; - store: any; -} +export type ViewProps = AppMountParameters; export interface ViewDefinition { readonly id: string; @@ -28,7 +23,8 @@ export interface ViewDefinition { defaults: T | (() => T) | (() => Promise); slice: Slice; }; - readonly mount: (params: ViewMountParameters) => Promise<() => void>; + readonly Canvas: LazyExoticComponent<(props: ViewProps) => React.ReactElement>; + readonly Panel: LazyExoticComponent<(props: ViewProps) => React.ReactElement>; readonly defaultPath: string; readonly appExtentions: { savedObject: { diff --git a/src/plugins/data_explorer/public/services/view_service/view.ts b/src/plugins/data_explorer/public/services/view_service/view.ts index 1e994a21b031..6268aa731497 100644 --- a/src/plugins/data_explorer/public/services/view_service/view.ts +++ b/src/plugins/data_explorer/public/services/view_service/view.ts @@ -13,7 +13,8 @@ export class View implements IView { public readonly defaultPath: string; public readonly appExtentions: IView['appExtentions']; readonly shouldShow?: (state: any) => boolean; - readonly mount: IView['mount']; + readonly Canvas: IView['Canvas']; + readonly Panel: IView['Panel']; constructor(options: ViewDefinition) { this.id = options.id; @@ -22,6 +23,7 @@ export class View implements IView { this.defaultPath = options.defaultPath; this.appExtentions = options.appExtentions; this.shouldShow = options.shouldShow; - this.mount = options.mount; + this.Canvas = options.Canvas; + this.Panel = options.Panel; } } diff --git a/src/plugins/data_explorer/public/utils/state_management/hooks.ts b/src/plugins/data_explorer/public/utils/state_management/hooks.ts index 607fe05b1623..d4194da3702f 100644 --- a/src/plugins/data_explorer/public/utils/state_management/hooks.ts +++ b/src/plugins/data_explorer/public/utils/state_management/hooks.ts @@ -3,9 +3,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { TypedUseSelectorHook, useDispatch, useSelector } from 'react-redux'; +import { useDispatch, useSelector } from 'react-redux'; import type { RootState, AppDispatch } from './store'; // Use throughout the app instead of plain `useDispatch` and `useSelector` export const useTypedDispatch = () => useDispatch(); -export const useTypedSelector: TypedUseSelectorHook = useSelector; +export const useTypedSelector: ( + selector: (state: TState) => TSelected, + equalityFn?: (left: TSelected, right: TSelected) => boolean +) => TSelected = useSelector; diff --git a/src/plugins/data_explorer/public/utils/state_management/preload.ts b/src/plugins/data_explorer/public/utils/state_management/preload.ts index 9cb85c3f4912..399f2d806c0a 100644 --- a/src/plugins/data_explorer/public/utils/state_management/preload.ts +++ b/src/plugins/data_explorer/public/utils/state_management/preload.ts @@ -17,7 +17,7 @@ export const getPreloadedState = async ( // initialize the default state for each view const views = services.viewRegistry.all(); - views.forEach(async (view) => { + const promises = views.map(async (view) => { if (!view.ui) { return; } @@ -31,6 +31,7 @@ export const getPreloadedState = async ( rootState[view.id] = defaults; } }); + await Promise.all(promises); return rootState; }; diff --git a/src/plugins/discover/public/application/utils/state_management/discover_slice.tsx b/src/plugins/discover/public/application/utils/state_management/discover_slice.tsx index cfac288cd3bc..d664c5e1d6d4 100644 --- a/src/plugins/discover/public/application/utils/state_management/discover_slice.tsx +++ b/src/plugins/discover/public/application/utils/state_management/discover_slice.tsx @@ -4,11 +4,9 @@ */ import { createSlice, PayloadAction } from '@reduxjs/toolkit'; -import { TypedUseSelectorHook, createDispatchHook, createSelectorHook } from 'react-redux'; -import { createContext } from 'react'; import { Filter, Query } from '../../../../../data/public'; import { DiscoverServices } from '../../../build_services'; -import { DataExplorerRootState } from '../../../../../data_explorer/public'; +import { RootState } from '../../../../../data_explorer/public'; export interface DiscoverState { /** @@ -19,10 +17,6 @@ export interface DiscoverState { * Array of applied filters */ filters?: Filter[]; - /** - * id of the used index pattern - */ - index?: string; /** * Used interval of the histogram */ @@ -41,14 +35,13 @@ export interface DiscoverState { savedQuery?: string; } -export interface RootState extends DataExplorerRootState { +export interface DiscoverRootState extends RootState { discover: DiscoverState; } const initialState = {} as DiscoverState; export const getPreloadedState = async ({ data }: DiscoverServices): Promise => { - // console.log(data.query.timefilter.timefilter.getRefreshInterval().value.toString()); return { ...initialState, interval: data.query.timefilter.timefilter.getRefreshInterval().value.toString(), @@ -67,6 +60,8 @@ export const discoverSlice = createSlice({ ...state, ...action.payload, }; + + return state; }, }, }); @@ -76,12 +71,4 @@ export const setState = discoverSlice.actions.setState as (payload: T) => Pay export const updateState = discoverSlice.actions.updateState as ( payload: Partial ) => PayloadAction>; - export const { reducer } = discoverSlice; -export const contextDiscover = createContext({}); - -export const useTypedSelector: TypedUseSelectorHook = createSelectorHook( - contextDiscover -); - -export const useDispatch = createDispatchHook(contextDiscover); diff --git a/src/plugins/discover/public/application/utils/state_management/index.ts b/src/plugins/discover/public/application/utils/state_management/index.ts index a850c1690e3b..d72cc772e6c4 100644 --- a/src/plugins/discover/public/application/utils/state_management/index.ts +++ b/src/plugins/discover/public/application/utils/state_management/index.ts @@ -2,3 +2,16 @@ * Copyright OpenSearch Contributors * SPDX-License-Identifier: Apache-2.0 */ + +import { TypedUseSelectorHook } from 'react-redux'; +import { RootState, useTypedDispatch, useTypedSelector } from '../../../../../data_explorer/public'; +import { DiscoverState } from './discover_slice'; + +export * from './discover_slice'; + +export interface DiscoverRootState extends RootState { + discover: DiscoverState; +} + +export const useSelector: TypedUseSelectorHook = useTypedSelector; +export const useDispatch = useTypedDispatch; diff --git a/src/plugins/discover/public/application/view_components/canvas/canvas.tsx b/src/plugins/discover/public/application/view_components/canvas/canvas.tsx index 7031c1e00f48..0246d97851ae 100644 --- a/src/plugins/discover/public/application/view_components/canvas/canvas.tsx +++ b/src/plugins/discover/public/application/view_components/canvas/canvas.tsx @@ -8,11 +8,7 @@ import { AppMountParameters } from '../../../../../../core/public'; import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_react/public'; import { DiscoverServices } from '../../../build_services'; import { TopNav } from './top_nav'; -import { - updateState, - useDispatch, - useTypedSelector, -} from '../../utils/state_management/discover_slice'; +import { updateState, useDispatch, useSelector } from '../../utils/state_management'; interface CanvasProps { opts: { @@ -22,15 +18,13 @@ interface CanvasProps { export const Canvas = ({ opts }: CanvasProps) => { const { services } = useOpenSearchDashboards(); - const { - discover: { interval }, - } = useTypedSelector((state) => state); + const interval = useSelector((state) => state.discover.interval); const dispatch = useDispatch(); return (
- Canvas + Interval: { dispatch(updateState({ interval: e.target.value })); }} /> +

Services: {services.docLinks.DOC_LINK_VERSION}

); }; diff --git a/src/plugins/discover/public/application/view_components/canvas/index.tsx b/src/plugins/discover/public/application/view_components/canvas/index.tsx index 66140a696ad6..34fd6a0bf103 100644 --- a/src/plugins/discover/public/application/view_components/canvas/index.tsx +++ b/src/plugins/discover/public/application/view_components/canvas/index.tsx @@ -4,32 +4,21 @@ */ import React from 'react'; -import ReactDOM from 'react-dom'; -import { Provider } from 'react-redux'; -import { ViewMountParameters } from '../../../../../data_explorer/public'; +import { ViewProps } from '../../../../../data_explorer/public'; import { OpenSearchDashboardsContextProvider } from '../../../../../opensearch_dashboards_react/public'; -import { DiscoverServices } from '../../../build_services'; import { Canvas } from './canvas'; -import { contextDiscover } from '../../utils/state_management/discover_slice'; +import { getServices } from '../../../opensearch_dashboards_services'; -export const renderCanvas = ( - { canvasElement, appParams, store }: ViewMountParameters, - services: DiscoverServices -) => { - const { setHeaderActionMenu } = appParams; - - ReactDOM.render( +// eslint-disable-next-line import/no-default-export +export default function CanvasApp({ setHeaderActionMenu }: ViewProps) { + const services = getServices(); + return ( - - - - , - canvasElement + + ); - - return () => ReactDOM.unmountComponentAtNode(canvasElement); -}; +} diff --git a/src/plugins/discover/public/application/view_components/panel/index.tsx b/src/plugins/discover/public/application/view_components/panel/index.tsx index f92e5af0bfdf..c05807d3a63a 100644 --- a/src/plugins/discover/public/application/view_components/panel/index.tsx +++ b/src/plugins/discover/public/application/view_components/panel/index.tsx @@ -4,19 +4,17 @@ */ import React from 'react'; -import ReactDOM from 'react-dom'; -import { ViewMountParameters } from '../../../../../data_explorer/public'; +import { ViewProps } from '../../../../../data_explorer/public'; import { OpenSearchDashboardsContextProvider } from '../../../../../opensearch_dashboards_react/public'; -import { DiscoverServices } from '../../../build_services'; import { Panel } from './panel'; +import { getServices } from '../../../opensearch_dashboards_services'; -export const renderPanel = ({ panelElement }: ViewMountParameters, services: DiscoverServices) => { - ReactDOM.render( +// eslint-disable-next-line import/no-default-export +export default function PanelApp(props: ViewProps) { + const services = getServices(); + return ( - , - panelElement + ); - - return () => ReactDOM.unmountComponentAtNode(panelElement); -}; +} diff --git a/src/plugins/discover/public/application/view_components/panel/panel.tsx b/src/plugins/discover/public/application/view_components/panel/panel.tsx index fe3b36f6e87b..fda7f8a44318 100644 --- a/src/plugins/discover/public/application/view_components/panel/panel.tsx +++ b/src/plugins/discover/public/application/view_components/panel/panel.tsx @@ -4,7 +4,9 @@ */ import React from 'react'; +import { useSelector } from '../../utils/state_management'; export const Panel = () => { - return
Side Panel
; + const interval = useSelector((state) => state.discover.interval); + return
{interval}
; }; diff --git a/src/plugins/discover/public/build_services.ts b/src/plugins/discover/public/build_services.ts index 0d7fa433cee1..a2b70f5c5099 100644 --- a/src/plugins/discover/public/build_services.ts +++ b/src/plugins/discover/public/build_services.ts @@ -83,11 +83,11 @@ export interface DiscoverServices { visualizations: VisualizationsStart; } -export async function buildServices( +export function buildServices( core: CoreStart, plugins: DiscoverStartPlugins, context: PluginInitializerContext -): Promise { +): DiscoverServices { const services: SavedObjectOpenSearchDashboardsServices = { savedObjectsClient: core.savedObjects.client, indexPatterns: plugins.data.indexPatterns, diff --git a/src/plugins/discover/public/plugin.ts b/src/plugins/discover/public/plugin.ts index 590d95c13c67..3fd5acfa7404 100644 --- a/src/plugins/discover/public/plugin.ts +++ b/src/plugins/discover/public/plugin.ts @@ -30,6 +30,7 @@ import { HomePublicPluginSetup } from 'src/plugins/home/public'; import { Start as InspectorPublicPluginStart } from 'src/plugins/inspector/public'; import { stringify } from 'query-string'; import rison from 'rison-node'; +import { lazy } from 'react'; import { DataPublicPluginStart, DataPublicPluginSetup, opensearchFilters } from '../../data/public'; import { SavedObjectLoader } from '../../saved_objects/public'; import { createOsdUrlTracker, url } from '../../opensearch_dashboards_utils/public'; @@ -163,7 +164,7 @@ export class DiscoverPlugin private stopUrlTracking: (() => void) | undefined = undefined; private servicesInitialized: boolean = false; private urlGenerator?: DiscoverStart['urlGenerator']; - private initializeServices?: () => Promise<{ core: CoreStart; plugins: DiscoverStartPlugins }>; + private initializeServices?: () => { core: CoreStart; plugins: DiscoverStartPlugins }; setup(core: CoreSetup, plugins: DiscoverSetupPlugins) { // TODO: Remove this before merge to main @@ -323,9 +324,6 @@ export class DiscoverPlugin }); } - // TODO: Carry this over to the view - // make sure the index pattern list is up to date - // await dataStart.indexPatterns.clearCache(); return () => { appUnMounted(); }; @@ -374,26 +372,16 @@ export class DiscoverPlugin }, ui: { defaults: async () => { - await this.initializeServices?.(); + this.initializeServices?.(); const services = getServices(); return await getPreloadedState(services); }, slice: discoverSlice, }, shouldShow: () => true, - mount: async (params) => { - // TODO: Remove this before merge to main - // eslint-disable-next-line no-console - console.log('DiscoverPlugin.dataExplorer.mount()'); - const { renderCanvas, renderPanel } = await import('./application/view_components'); - const [coreStart, pluginsStart] = await core.getStartServices(); - const services = await buildServices(coreStart, pluginsStart, this.initializerContext); - - renderCanvas(params, services); - renderPanel(params, services); - - return () => {}; - }, + // ViewCompon + Canvas: lazy(() => import('./application/view_components/canvas')), + Panel: lazy(() => import('./application/view_components/panel')), }); // this.registerEmbeddable(core, plugins); @@ -414,17 +402,19 @@ export class DiscoverPlugin console.log('DiscoverPlugin.start()'); setUiActions(plugins.uiActions); - this.initializeServices = async () => { + this.initializeServices = () => { if (this.servicesInitialized) { return { core, plugins }; } - const services = await buildServices(core, plugins, this.initializerContext); + const services = buildServices(core, plugins, this.initializerContext); setServices(services); this.servicesInitialized = true; return { core, plugins }; }; + this.initializeServices(); + return { urlGenerator: this.urlGenerator, savedSearchLoader: createSavedSearchesLoader({