-
Notifications
You must be signed in to change notification settings - Fork 8.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DISCUSS] State management plugin API #55977
Comments
Pinging @elastic/kibana-app-arch (Team:AppArch) |
So here is how it could look with our current state containers and state syncing utils, which are already used in management and dashboard. I also did some minor tweaks to state syncing, which made a lot of sense to me, while working on this demo. Goal
So first of all let's define our state shape.To make this work in current setup, we have to define 2 different state containers to store our state in, because technically we have different states which want to sync separately. import { QueryAppState, QueryGlobalState } from '../../../../../src/plugins/data/public';
interface AppState extends QueryAppState {
name: string;
}
const defaultAppState: AppState = {
name: '',
};
interface GlobalState extends QueryGlobalState {
globalData: string;
}
const defaultGlobalState: GlobalState = {
globalData: '',
};
Create state containersNext we have to create state containers to our state. import { createStateContainer } from '../../../../../src/plugins/kibana_utils/public';
const appStateContainer = createStateContainer(defaultAppState);
const globalStateContainer = createStateContainer(defaultGlobalState); But, if we don't want to worry about resetting the state when app gets unmounted, we should create it inside react component. And we don't want to recreate it on each render call. So it can be achieved by something like this: function useCreateStateContainer<State extends BaseState>(
defaultState: State
): ReduxLikeStateContainer<State> {
const stateContainerRef = useRef<ReduxLikeStateContainer<State> | null>(null);
if (!stateContainerRef.current) {
stateContainerRef.current = createStateContainer(defaultState);
}
return stateContainerRef.current;
}
export const StateDemoApp = (props: StateDemoAppDeps) => {
const appStateContainer = useCreateStateContainer(defaultAppState);
const globalStateContainer = useCreateStateContainer(defaultGlobalState);
return (
<App {...props} />
);
}; I think we should have this Attach state containers to React.There are state management helpers for connecting state containers to react components (look at it as on react-redux for redux). import {
createStateContainerReactHelpers,
ReduxLikeStateContainer,
} from '../../../../../src/plugins/kibana_utils/public';
const {
Provider: AppStateContainerProvider,
useState: useAppState,
useContainer: useAppStateContainer,
} = createStateContainerReactHelpers<ReduxLikeStateContainer<AppState>>();
const {
Provider: GlobalStateContainerProvider,
useState: useGlobalState,
useContainer: useGlobalStateContainer,
} = createStateContainerReactHelpers<ReduxLikeStateContainer<GlobalState>>();
export const StateDemoApp = (props: StateDemoAppDeps) => {
const appStateContainer = useCreateStateContainer(defaultAppState);
const globalStateContainer = useCreateStateContainer(defaultGlobalState);
return (
<AppStateContainerProvider value={appStateContainer}>
<GlobalStateContainerProvider value={globalStateContainer}>
<App {...props} />
</GlobalStateContainerProvider>
</AppStateContainerProvider>
);
}; Now we have hook, we can use in child component to get access to underlying state containers. Build the app container component using state containersuse state containers instead of React.useState for storing the state. const App = (props: StateDemoAppDeps) => {
const appStateContainer = useAppStateContainer();
const appState = useAppState();
const globalStateContainer = useGlobalStateContainer();
const globalState = useGlobalState();
const indexPattern = useIndexPattern(data);
if (!indexPattern) return <div>Loading...</div>;
return (
<>
<navigation.ui.TopNavMenu
appName={PLUGIN_ID}
showSearchBar={true}
indexPatterns={[indexPattern]}
/>
<EuiFieldText
value={appState.name}
onChange={e => appStateContainer.set({ ...appState, name: e.target.value })}
/>
<EuiFieldText
value={globalState.globalData}
onChange={e =>
globalStateContainer.set({ ...globalState, globalData: e.target.value })
}
/>
</>
);
}; Setup syncing URL <-> State Containers <-> Data Services
function useGlobalStateSyncing<GlobalState extends QueryGlobalState>(
globalStateContainer: BaseStateContainer<GlobalState>,
query: QueryStart,
kbnUrlStateStorage: IKbnUrlStateStorage
) {
// setup sync state utils
useEffect(() => {
// sync global filters, time filters, refresh interval from data.query to state container
const stopSyncingQueryGlobalStateWithStateContainer = connectToQueryGlobalState(
query,
globalStateContainer
);
// sets up syncing global state container with url
const {
start: startSyncingGlobalStateWithUrl,
stop: stopSyncingGlobalStateWithUrl,
} = syncState({
storageKey: '_g',
stateStorage: kbnUrlStateStorage,
stateContainer: {
...globalStateContainer,
// stateSync utils requires explicit handling of default state ("null")
set: state => state && globalStateContainer.set(state),
},
});
// skipped here!
// Manually take care of initial syncing. This is app specific!
// e.g. some initial state could come from saved object and it should take precedence
// e.g. or there could be custom options like "Save time with Dashboard" which will change this behaviour per app
// finally start syncing state containers with url
startSyncingGlobalStateWithUrl();
return () => {
stopSyncingQueryGlobalStateWithStateContainer();
stopSyncingGlobalStateWithUrl();
};
}, [query, kbnUrlStateStorage, globalStateContainer]);
}
function useAppStateSyncing<AppState extends QueryAppState>(
appStateContainer: BaseStateContainer<AppState>,
query: QueryStart,
kbnUrlStateStorage: IKbnUrlStateStorage
) {
// setup sync state utils
useEffect(() => {
// sync app filters with app state container from data.query to state container
const stopSyncingQueryAppStateWithStateContainer = connectToQueryAppState(
query,
appStateContainer
);
// sets up syncing app state container with url
const { start: startSyncingAppStateWithUrl, stop: stopSyncingAppStateWithUrl } = syncState({
storageKey: '_a',
stateStorage: kbnUrlStateStorage,
stateContainer: {
...appStateContainer,
// stateSync utils requires explicit handling of default state ("null")
set: state => state && appStateContainer.set(state),
},
});
// skipped here!
// Manually take care of initial syncing. This is app specific!
// e.g. some initial state could come from saved object and it should take precedence
// e.g. or there could be custom options like "Save time with Dashboard" which will change this behaviour per app
// finally start syncing state containers with url
startSyncingAppStateWithUrl();
return () => {
stopSyncingQueryAppStateWithStateContainer();
stopSyncingAppStateWithUrl();
};
}, [query, kbnUrlStateStorage, appStateContainer]);
} And then in the app: const App = ({
data,
kbnUrlStateStorage,
}: StateDemoAppDeps) => {
const appStateContainer = useAppStateContainer();
const appState = useAppState();
const globalStateContainer = useGlobalStateContainer();
const globalState = useGlobalState();
useGlobalStateSyncing(globalStateContainer, data.query, kbnUrlStateStorage);
useAppStateSyncing(appStateContainer, data.query, kbnUrlStateStorage);
/// render ui
} Complete example:https://github.com/elastic/kibana/pull/56128/files#diff-a0e48df3e83e86c617c169f43d50072e Some points to note:
(this is used by dashboard to sync all
e.g in dashboard app filters are synced with custom state manager by just:
|
This seems like a good idea to me, assuming we document the specific reasons why you might want it.
IMO all of the extra work to get After all, there is no more truly "global" state anymore, just state shared between apps. Just because some state is pushed into a
This is what I think we should steer folks away from -- the way URLs are structured in the dashboard is not how IMO they should look long term. So if someone wants to sync filters, query, timefilter to their app URL, plus some other stuff specific to their app, I think the minimal implementation would be even simpler if they had filters, query, timefilter, myName, and myGlobalData all as separate URL params. (@Dosant does this sound right to you?) All this is to say, in the example in this thread, the requirements for having a dashboard-like URL is where a lot of the complexity comes from. |
@lukeelmers, Yes, this was also my understanding. Also for more context with new utilities, please review - #56479 . Maybe some new thoughts will appear from there. |
@streamich, Could you take a look please link. Interested in your thoughts since you've been working a lot on state containers. While I was working on the example, had couple thoughts on improvements:
const [state, setState] = useState() Wonder what do you think about those 2? |
We should also consider this issue: #56503 In the #55818 we have to "watch" for any update coming from either global filters, time range or refresh interval and then update In legacy it was done in one place in chrome and chrome updated all the urls in navbar. But in np, it was decided that apps "watch" for those updates themselves and update their own url in navbar. With current setup as in #55818, an app creates a state container which orchestrates changes from "global filters, time range or refresh interval". And each app have to orchestrate it. These orchestration is happening when apps are not mounted (see the setup phase of dashboard plugin). It looks like we need to make sure we have observables on data.query like:
which could be used for cases like in #55818 |
I'm not sure I fully understand details here, but if I was writing an app I would try to use a single state container, similar how you have a single Redux store per app, as state containers are essentially Redux stores. Something like this: interface DashboardState {
globalStuff: any;
isEditMode: boolean;
// etc.
}
const dashboardContainer = createStateContainer<DashboardState>();
const {useSelector, Provider} = createReactStateContainerHelpers(dashboardContainer);
<Provider>
<DashboardApp />
</Provider>
const DashboardApp = () => {
const { isEditMode } = useSelector(({isEditMode}) => ({isEditMode}));
return <div>this is dashboard</div>;
}; |
Short term resolved by #56128 |
Assuming one is implementing a plugin and wants to use our state containers \ syncing utilities, what is the most minimal solution he could implement, in the demo plugin code below?
Assuming he wants filters, query and timefilter to be stored as in any other app (i.e. like dashboard) and also adding
myName
to app state andmyGlobalData
to global stateThe text was updated successfully, but these errors were encountered: