Open ananzh opened 1 year ago
Great proposal @ananzh! Definitely more inclined to option one. Here are a few suggestions to that.
Thanks for the detailed description of the migration!
To address the challenge of managing states in a centralized system with different applications like VisBuilder and DataExplorer, while avoiding naming conflicts and ensuring easy access to the states, we propose the following steps:
Prefixing state keys with the view.id to avoid naming conflicts. Then we can maintain the original state names within each view (like style, visualization, and ui in vis builder) but prepend the view's identifier when integrating them into the RootState. The only exception is metadata, because it is duplicate as the one in RootState.
The register slice id in Data Explorer store (src/plugins/data_explorer/public/utils/state_management/preload.ts):
const id = slice.name == view.id? slice.name: `${view.id}-${slice.name}`;
Therefore in plugin.ts, each view needs to register a proper ID. If there are multiple slices in one view, the slice id would be ${view.id}-${slice.name}
. For example, in the following vis_builder, the view.id is vis_builder
export const PLUGIN_ID = 'vis-builder';
dataExplorer.registerView<any>({
id: PLUGIN_ID,
title: PLUGIN_NAME,
defaultPath: '#/',
appExtentions: {
savedObject: {
docTypes: [VISBUILDER_SAVED_OBJECT],
toListItem: (obj) => ({
id: obj.id,
label: obj.title,
}),
},
},
…
There are four slices: ui, style, editor, visualization. The registered slice names in Data Explorer are vis-builder-ui, vis-builder-style, vis-builder-editor and vis-builder-visualization.
For view with single state, then the easiest naming is to have state name equal to PLUGIN_ID. For example, in Discover, both registered view ID and state is discover
. Then in Data Explorer, the registered slice name is discover
. Discover does not need further state access.
To access these namespaced states easily, we can create a selector function that dynamically constructs the state key based on the view's ID. For example, within VisBuilder, we use a generic selector that automatically appends vis-builder- to the state keys. Here are some reference helper functions.
// Function to access specific state with prefix
export const getViewState = (stateKey: 'editor' | 'ui' | 'visualization' | 'style') =>
(state: VisBuilderRootState) => state[`${PLUGIN_ID}-${stateKey}`]
To use
// Usage in VisBuilder
const styleState = useTypedSelector(getViewState('style'));
* access one specific state from the entire state object
type ValidStateKeys = 'editor' | 'ui' | 'visualization' | 'style'; type ValidStateTypes = StyleState | UiState | VisualizationState | EditorState;
const getDynamicState = ${PLUGIN_ID}-${stateKey}
;
return rootState[dynamicKey] as T;
};
To use:
const rootState = useSelector(state => state);
const editorState = getDynamicState
// TypeScript will ensure that editorState is of type EditorState if (editorState && editorState.status === 'loaded') { ... }
### Dispatch Mechanism in Centralized State Management
In the centralized state management system, where different slices of state are prefixed with their respective view IDs (like vis-builder-editor for the editor slice), we actually don't need to modify the actions:
* Action Type Prefixing: Redux Toolkit automatically namespaces action types with the slice name, making them unique. For example, the setStatus action from the editor slice has a type like "editor/setState".
* Reducer Handling: When `setStatus` is dispatched, Redux invokes the reducer associated with "editor/setStatus". The namespacing of slices in the global state, such as `vis-builder-editor`, does not require any additional handling when dispatching actions.
* No Need for Explicit State Specification: There is no need to manually specify which part of the state should be affected by an action. The linkage between the action and its target slice (the `editor` slice will point to `vis-builder-editor` reference in the global root state) is managed by the setup and registration of reducers and slices.
* Simple Dispatching: Actions are dispatched using their creators as usual. Redux handles routing these actions to the appropriate slice reducer based on their types.
How will this work with auto complete and typescript types and suggestions? today selecting using the selector is typesafe
@ashwin-pc the above is more general discussion on how to migrate/work with a centralized state management store. In vis-builder, we will add one helper function
export const getViewState = (stateKey: 'editor' | 'ui' | 'visualization' | 'style') =>
(state: VisBuilderRootState) => state[`${PLUGIN_ID}-${stateKey}`];
To use it:
const styleState = useTypedSelector(getViewState('style'));
The stateKey
is limited to specific strings ('editor', 'ui', 'visualization', 'style'), which correspond to the keys in VisBuilderRootState. This provides the following benefits:
Type Safety: Ensures that only valid keys for the VisBuilder's state are used, reducing the risk of runtime errors due to incorrect state keys.
Autocomplete: When using this selector function, developers will receive autocomplete suggestions for the stateKey parameter, which can speed up development and reduce errors.
Error Detection: If a developer tries to use an invalid key, TypeScript will flag it as an error, catching potential issues early in the development process.
The types defined in Data Explorer is more general to fit more views. But VisBuilder and other views could maintain type safety across the entire application.
@ashwin-pc I updated the previous comment. For access one specific state object directly
and access one specific state from the entire state object
, which one you like? Currently I am using the first one. Restrict the type to view specific slices. No need to load the entire state. What do you think?
Oh I like it if the helper exists. I just don't want to have to remember how to get to the state value without auto complete 😂
Overview
The Data Explorer project has taken a step forward in streamlining the data exploration experience in OpenSearch Dashboards (OSD) by addressing security concerns and offering a unified platform. As a strategic move, it's essential to integrate the vis-builder plugin with Data Explorer to provide a comprehensive data analysis framework for users.
Vis-builder has been instrumental in visual data exploration, offering capabilities like chart creation and advanced visualization settings. Its incorporation into Data Explorer would enhance the platform's overall data analytics and visualization potential.
There are two stages of the work:
Stage 1: Incorporate the vis-builder plugin as a view inside the Data Explorer.
Goals
Note: This proposal focuses primarily on Stage 1.
Stage 2: Enhancements (currently under design & discussion)
Goals
Note: Not the scope of current proposal.
Objective
To seamlessly incorporate the vis-builder plugin as a view inside the Data Explorer to offer a unified data exploration experience, thereby enriching the data analysis capabilities of OSD.
Tasks
[Task 1] Reconstruct and Reroute VisBuilder Plugin
[Task 2] State Management Migration
There are issues to migrate states. First, simplify metadata slice which should only ffocus on the editor's state.
Second migrate simplified metadata slice and other 3 slices, there are two proposals.
Proposal 1: Allowing Data Explorer to Register Multiple Slices for a Single View
Objective
Retain the modularity of individual slices for each component of the Vis Builder while simplifying integration with the Data Explorer.
Steps
Update the View Registration: Instead of registering a single slice, views will now register an array of slices.
Modify the ViewDefinition Interface: Replace the singular slice property with a plural slices property.
Handle side effects: In the Data Explorer store, we are dynamically adding slices (reducers) for each view. In the visBuilder, there are two side effects: handlerEditorState and handlerParentAggs. These side effects are listeners that perform specific actions when certain changes in the state occur. The current Data Explorer does not have these side effects integrated, which leads us to the main challenge. However, we see there is a TODO in Data Explore store config
Add Side effects here to apply after changes to the store are made. None for now.
, which indicates an initial design considerations on how integrate view’s side effect. Based on this, here are stepsDefine Side Effects in ViewDefinition.ui: Extend the ui object of each view to potentially include an array of side effects. This will allow each view to specify its set of side effects.
Modify handleChange to execute side Effects in getPreloadedStore:
handleChange
should iterate through each registered view and execute its side effects if they exist.Modify side effects functions based on new definition.
Register Side Effects for visBuilder: When defining the visBuilder UI, ensure to add the modified handlerEditorState and handlerParentAggs functions to the sideEffects array.
Some cleaning work: Clean out redux store from VisBuilder and utilize the one from Data Explorer.
Pros and Cons
Proposal 2: Combining VisBuilder Slices and register one slice to Data Explorer
This proposal defines a combined VisBuilderState which encompasses all the individual sub-states. Data Explorer current setting is one slice/view. A view, for example Discover, needs to specify a slice like below and data explorer register a slice via registerSlice.
Different from Discover, VisBuilder has multiple slices. Then for consistency and manage purpose, it might be ideal to keep the one slice for one view.
Steps
Combine slices: We would need to manually creating one with createSlice. We can’t use combineReducers here because this method returns a reducer function, not a slice object. Therefore this will bring incompatibility with registerSlice in Data Explorer. It would be convenient if we could directly utilize registerSlice and don’t have to make modifications in the upstream Data Explorer.
Combine preloaded state: create combined getPreloadedState .
Dispatch Actions: As mentioned above, dispatch needs to get updated. First take the current action with specific slice, for example setStyleState(newStyleValue) specific to the styleSlice. Then wrap that action with the setStyleState action creator to make it specific to the visBuilderSlice. Then dispatch the resulting action.
Understand and check extraReducers: Since we have combined the slices, it's crucial that extraReducers is moved in the combined slice (VisBuilderSlice) which should know which subsection of the state to modify.
Handle side effects: Similar to Proposal 1.
Pros and Cons
updateStyle: (state, action: PayloadAction<StyleState>) => { state.style = styleReducer(state.style, action); }
[Task 3] Context Provider Migration/Creation
[Task 4] UI modifications
The main part of this task is to divide src/plugins/vis_builder/public/application/app.tsx into a canvas area (VisBuilderCanvas) and a side panel (VisBuilderPanel). The canvas is where users interact with visualizations, and the side panel only provides field selector. Below is the brief re-construction:
VisBuilderPanel should constructed from cleaned LeftNav .
VisBuilderCanvas is the main canvas of VisBuilder. It should have the TopNav at the top, ConfigPanel on the left, Workspace in the middle, and RightNav on the right.
[Task 5] Comprehensive test for VisBuilder
Tentative Timeline
Based on 2-3 devs
Week 1-2
Week 3-4
Week 5-6
Additional Considerations