Shared loading states #144
Comments
|
Offtopic: Thinking about it, we could also implement some kind of link middle-/afterware like Ismay brought up a very good argument for using redux over context for this (which currently is the proper solution for global state in app-shell apps not using redux):
apollo has this baked in as well, but I think for us, using redux is significantly easier that writing it ourselves. |
This is only the case if you store the state as the context. We do not do this, nor should we ever do it. Each Query or Mutation (eventually subscription) currently has its own state and only rerenders itself, nothing else, when necessary. Eventually this might come from an event emitter from a centralized store (we'll need to do this when we add caching anyway) I think providing an "out of the box" global state option might be worth considering, but I consider it distinct from loading states which are baked into the data engine runtime |
I'm not talking about what the data provider does, but what apps have to do if they need global state. EDIT: What I'm trying to say is: The data provider should provide a way to leverage on its state mechanism so the app doesn't have to use context
Which is why I labelled it as "offtopic", I could create another issue, but think it's even less relevant than the current one |
|
Hey @Mohammer5 - raising this again so it doesn't get lost. As a quick question, would an implementation of optimistic updates solve your use-case? Basically, when mutating from somewhere else in the application any other queries for that object would get a "placeholder" value even while the HTTP request is in-flight. It can then be rolled back if the request fails, and replaced with the real thing if it succeeds. I think this is a simpler API than a more generic state system or namespaced loading/error states, but obviously comes with some additional challenges to ensure we have a properly-normalized and indexed local cache. I implemented a very rudimentary version of this with an EventEmitter here (emit) and here (subscribe) which works well but for a quite limited use-case. Would optimistic updates be a possible way to address your use-case or do you think there would be requirements either for more "ad-hoc" namespaced loading states or for true propagation of "loading" rather than "stubbed" optimistic values? |
|
I had some time to think about this. I think it wasn't so much about loading states themselves, rather about having shared client state between routes and/or components. I can give you a simple example: Let's say one route displays an input field (e. g. for search) and the other route wants to display the same field, just somewhere different in the ui and the app wants to retain the value while changing routes. Here's a second example: The import export app polls the current status of an import job and we want it to keep the import result (once the polling is done) so you could go back there after navigating to a different route. That's how the new v35 version works right now (with context) Here are my current options with application state, each with good and bad sides:
React contextThis is obviously the most primitive choice but still easiest and pure-react-way choice. Client side resolversThis is not a bad idea in terms of pure & maintainable client side data. I think it won't be hard to implement either (looping through a set of entries when a query or mutation should be executed and let the client side resolver handle everything when it should be used). It would require some form of client side caching (like apollo's This would require the app to either initialize the data engine itself (apollo-like) or the engine offers an api to add client side resolvers during the runtime ( I guess this would be a MUCH better implementation than what I've proposed above and one of the main ideas why I think this could be a very good way of handling client side state is because there's only a single source of truth (the engine) for dynamic data. What I don't like about the two solutions is that they're both quite imperative. I have to tell the state how to change instead of what to do, which is where redux shines. It's more very declarative and - once you understand the concept - very easy to extend or maintain. The I still like the idea of the engine being the single source of truth though, so I'm wondering if we couldn't combine those two concepts:
Obviously we couldn't leverage on the |
|
This could live side by side with optimistic updates, I assume that it could be a very complicated implementation though because you want every I was wondering about if we couldn't just use apollo directly and have a set of client side resolvers that do their "magic", basically an apollo server on the client side. But I don't know if there's a way to "catch" unknown query types because we obviously don't want to write the whole dhis2 schema as graphql schema. |
|
I'm currently tackling something similar for the scheduler. My personal preference would be to stick to the primitive approach (React context) with a form of caching, if possible and maintainable. I'll see if I can make that work with the scheduler, will report back on how that goes. |
|
The reason I'm advocating for redux instead of context is that redux's paradigm is declarative and intention-driven while context is imperative and state-driven. As developer I find it much easier and safer to express my intention as code and then being able to reason about code quite comfortably instead of having to understand / know where state is going to be mutated. Also using context in performance critical situations needs more adjustments on the app's side, which either complicates the app or leads the developer to using a different piece of technology. And I'd prefer consistence yet simple solutions. |
|
I keep thinking about this and I want to add two more points to this discussion: React contextAlthough we can solve everything with context in terms of handling ui state / local state, I still favor the idea of a single source of truth (the data engine in this case). ReduxImperatively/directly manipulating state can be seen as an action inside our business logic, while in reality there are two actions:
This distinction is not codified though, so as a developer you have to make assumptions why state is being manipulated. Redux does make us codifying that distinction, which opens up some possibilities we didn't have before. Not code-wise but of cognitive nature as the developer has to make fewer assumptions, has to keep less code in mind and can therefore create more complex scenarios while the extra complexity is "hidden"/"covered" by redux. I'm not trying to advocate for redux specifically, we could achieve the same with React's context (even with the same performance), but it'd require us to implement some kind of library (or workspace in this repo), increase the maintenance burden and on-boarding time. Many developers are familiar with redux on the other hand. I still think this should be baked into to data engine though. |
|
@ismay had a good remark:
My response to that is: if we build state management into the engine, we could think about simpler patterns for now until we see that it doesn't scale Actually I agree with @ismay there. I'd be fine with starting more low level and see how it scales. If we don't need complex ui state, then it's probably even better not using redux. Still would like to have a single source of truth. But we could defer this discussion and see how it plays out just using context for now. Maybe I'm trying to optimize something that doesn't need optimization here. |
|
Hey guys! Thanks for the great discussion, lots of good points! I’ll respond in more detail shortly, in a marathon meeting today. I’m definitely in favor of a simple, lightweight solution first and designing for declarative single-source-of-truth access. One detail thing I’ll add quickly is that I think I’d advocate for a separate “state” service within app-runtime, rather than commingling with data engine - basically an implementation detail, but I think keeping them separate is clean |
|
I think the important things to define here are:
My initial thought these are not fully fleshed out, definitely need to discuss as a team so feel free to disagree! In terms of requirements and simplifying assumptions the ones I see are :
In terms of API, I think I'd be in favor of something simple like the below but without reducers, action creators, dispatch, etc. - instead we could just expose two hooks :
The first version of each hook makes the assumption that state is just a dictionary of key-value pairs, the second is a bit more robust (Redux-like) but without the requirement of centralizing all action creators and reducers. (In the example below I've gone the full redux clone route, but it would be easy to implement the above API in the same way). I planned to just sketch this out and include it in-line, but ended up coding it as a sample app. Note that here (as in the data service) I'm using React Context purely as a deep-vdom distribution mechanism - the value of the context itself never actually changes, so none of the context consumers re-render (avoiding the problems often associated with context for global state). Instead it just delivers a pointer to the current state, a dispatch function, and a couple helper functions for subscribing to changes to somewhere possibly very far down the React tree. This could technically also be done by passing Definitely not saying this is the way to go, but wanted to show a proof-of-concept React Context-based approach with reducers and selectors. Here's the POC app I ended up with and below is the simple library. It doesn't support middleware or thunks, but does the right thing in terms of only re-rendering when a relevant slice of the state changes. It's basically a limited 60-line Redux clone, so at this point we might just want to use Redux (though no-deps has some appeal). import React, { useEffect, useCallback, useContext, useState } from 'react'
const identity = state => state
export const createStore = (reducer = identity) => {
const subscriptions = new Set()
let state = reducer(undefined, undefined)
return {
getState: () => state,
subscribe: callback => {
subscriptions.add(callback)
},
unsubscribe: callback => {
subscriptions.delete(callback)
},
dispatch: action => {
state = reducer(state, action)
for (let callback of subscriptions) {
callback(state)
}
}
}
}
const GlobalStateContext = React.createContext(createStore())
const useGlobalStateStore = () => useContext(GlobalStateContext)
export const GlobalStateProvider = ({ store, children }) =>
<GlobalStateContext.Provider value={store}>
{children}
</GlobalStateContext.Provider>
export const useGlobalState = (selector = identity) => {
const store = useGlobalStateStore()
const [selectedState, setSelectedState] = useState(selector(store.getState()))
useEffect(() => {
// TODO: deep equality check, this only triggers an update on referential inequality
const callback = state => setSelectedState(selector(state))
store.subscribe(callback)
return () => store.unsubscribe(callback)
}, [store, /* ignore selector */]) /* eslint-disable-line react-hooks/exhaustive-deps */
return [selectedState, { dispatch: store.dispatch }]
}
export const useGlobalDispatch = () => {
return useGlobalStateStore().dispatch
}
export const useGlobalAction = actionCreator => {
const dispatch = useGlobalDispatch()
return useCallback((...args) => {
dispatch(actionCreator(...args))
}, [actionCreator, dispatch])
}
|
|
@ismay are you using React Context in the Scheduler for some global state or just local React state? One thing I'm pretty sure about is that we tend to over-use Redux when we have it, when we should use local state for a LOT more things (particularly relating to UI). I actually think most of the examples @Mohammer5 used above should be accomplished without adding global new explicit state to the application. Instead, things like synchronizing refreshed data should be automatic when using the Data Engine, and things like shared loading/error state (in the Org Unit Tree, for example) should probably be handled with Suspense for Data Fetching and Error Boundaries, rather than adding something to the global state. |
@Mohammer5 I like where you're going here 🎉 I would love to use Apollo Client and GraphQL directly, but unfortunately until we get a GraphQL server into Core (I just presented a proposal to try for this in 2.36 but no promises...) we won't be able to. Handling GraphQL queries (Apollo Server) is quite heavy, and doesn't work in the javascript event loop. I once saw an implementation which ran the server in a Web Worker to process queries and translate them into REST calls, but I still don't think it's super performant and not worth the performance hit just for the developer experience improvement (nice as that would be) The other (and probably bigger) major advantage of using GraphQL is the ability to combine queries into one network roundtrip (since GraphQL is sent in a POST body), so the plan is to push for that and get the Apollo Client experience as a huge bonus. EDIT: Found one 3-year-old reference to client-side GraphQL, could be worth exploring but I'm very skeptical about the benefit-performance trade-off |
|
Sorry for the super long comments! To reiterate these are the things I think we should iron out, then implement the simplest solution possible:
@Mohammer5 @ismay if it would be easier (or more fun) to brainstorm ideas and try to break things on a call let's set one up! |
|
Unrelated to the suggestion, I'd like to mention this middleware: I've used it a few times already and it's quite nice. Not sure if this is a requirement for our apps, it does help with offline support, but I guess that we don't need to because we don't support mobile devices |
|
@amcgee Your solution looks pretty good already! I think I'd prefer a Components that need data from a 3rd party api could just use |
|
Ah, just another thing, I think actions should always look like this: interface Action {
type: string,
payload: object,
} |
It's a start!
To use context we need a Provider component unfortunately, because
Yeah, that's a good call, 100% agree. I'm a bit hesitant to go full-redux here because I think it ends up with tons of boilerplate when we are trying to stay simple. I realize the benefits of centralized state transition logic, but I don't know that we necessarily need the action-reducer pattern. (see : the usage example for my redux clone has more lines than the implementation 😉 ) I think typescript safety on centralized state would have a lot of benefits, but I'm not going to open that can of worms again here 😃 we'd implement this in typescript anyway here in |
I can see the simplicity by not using a redux-like pattern in the beginning. // change a single value
const { change } = useGlobalState()
change(state => /* return updated state */)
// multiple changes
const { change, batch } = useGlobalState()
batch(() => {
change(state => /* return updated state 1 */)
change(state => /* return updated state 2 */)
change(state => /* return updated state 3 */)
})
// alternative to batch
change(
state => /* return updated state 1 */,
state => /* return updated state 2 */,
state => /* return updated state 3 */,
)We could still use selectors const { state } = useGlobalState()
const foo = getFoo(state) // getFoo = state => state.foo
const bar = getBar(state) // getBar = state => state.barThe above example is imperative though, but adding reducer/dispatch functionality wouldn't be a breaking change
That can be done by the the
PS: No worries! Sometimes I'm bombarding you guys with walls of texts |
Hmm interstesting... I'm not familiar with final-form's Here's the API I was thinking about for a "simpler" approach - under the hood it could actually even create actions and reducers, but I find this much simpler to reason about (and easier to code) than creating a reducer (with a switch case), action creator function, and action type constant for any state change: Basically this takes advantage of first-class function primitives to do the same thing but without the repetition - the drawback is that you don't necessarily know the reducer from the get-go, but in practice if all the actions are colocated you can reason about them well enough. I'm using the term // Action-reducers (could be all located centrally)
// These basically combine actionCreator and reducer
// We could enforce that these always global static consts (as we do with queries)
const arSetAnswer = answer => state => ({ ...state, answer })
const arDefineQuestion = question => state => ({ ...state, question })
// Consumer Components
const questionSelector = state => state.question
const Question = () => {
const [question] = useGlobalState(questionSelector)
return <span>
<strong>{question}</strong>
</span>
}
const Answer = () => {
const [answer] = useGlobalState(state => state.answer) // inline selectors work too
return <span>
<strong>{answer}</strong>
</span>
}
const SolveButton = () => {
const defineQuestion = useGlobalAction(arDefineQuestion)
return <button onClick={() => defineQuestion('What is the meaning of life, the universe, and everything?')}>What is the question?</button>
}
const RandomizeButton = () => {
const setAnswer = useGlobalAction(arSetAnswer)
return <button onClick={() => setAnswer(Math.floor(Math.random() * 10000))}>Randomize answer</button>
}
// App
// In this case, <GlobalStateProvider> is rendered by the App Shell, so the App doesn't need it.
export const App = () => <>
<Question />
<Answer />
<SolveButton />
<RandomizeButton />
</>The entire state transition schema is those first two lines, compared to the same thing in Redux style which is 30+ lines and would usually be split across multiple files. Obviously this is somewhat limiting, but it would actually be not so difficult to move to an action / reducer / dispatch pattern later (consumers shouldn't care if they're passing an action-creator or an action-reducer)
I think we want to keep the concerns of state and data management separate - each with their own context - at least for now. This still follows the principle of single source of truth (apparently I'm just retweeting Dan these days...). When we have a I think if we're providing a centralized reducer my preference would be actually to expose a separate Application state shouldn't be shared by different library components by design, so we'd have to be careful about never accidentally including |
That's exactly the purpose (probably to work with other technologies as well). const arSetAnswer = answer => state => ({ ...state, answer })
const setAnswer = useGlobalAction(arSetAnswer)const answerReducer = (state, answer) => ({ ...state, answer })
const setAnswer = useGlobalAction(answerReducer)I think style-wise I'd prefer the second one, but that's probably just because I'm use to that "syntax". I'm not a big fan of currying / higher order functions anymore (there are few cases where it absolutely makes sense though), I prefer partial application when necessary and just pass in everything else at the same time when possible (which is possible most of the time)
Fair enough! |
That sounds good, though at the moment I don't have the use cases clear yet. Personally I'd prefer to get the scheduler refactor close to done first. At that point I should have a clear idea of any use cases that aren't supported yet (if any). Having the chat before that point would become too much of a philosophical exercise I think (at least from my perspective). |
|
The point is to address the relevant use-cases, so that sounds good to me |
|
Hi! Due to a lack of activity on this issue over time (180 days) it seems to be stale. If still relevant, please provide information that moves it forward, e.g. additional information, a pull request with suggested changes, or a reason to keep it open. Any activity will keep it open, otherwise it will be closed automatically in 30 days. Thanks! 🤖 |
This is currently not an issue, just creating for reference
Type
Feature request
Problem
Given there are two independent components. One for rendering a list of items (q-comp), the other one for mutating said list (m-comp; either by adding, changing or deleting items).
When the m-comp runs its mutation process, there is no way to know about this in the q-comp.
What I think the solution should cover
By this I mean that there should a global state that can be used to know whether a certain process is happening or has errored
How I would do this in Apollo
Apollo offers the
apollo-link-statemiddleware. It can be used to define client-side resolvers.I then define a
useMutation/useQueryhooks which leverages on the one provided by apollo, but additionally sends mutations for the loading/error state. The following code is more or less pseudo code and doesn't work exactly like that, but it conveys the message.One important note:
Inside that hook, I define 3 queries for mutating the loading state, they all expect a
name.I then use these to set the loading states for the provided name.
This way a component can add a namespace to the process it's executing
And a list file can explicitly state when it wants to access loading states.
I guess this way we could also implement a more explicit
refetchQueries(a property on theuseMutationconfig object). This doesn't really solve how to do explicitupdates ("optimistic update") though, I don't think we want to store all responses in the app-runtime's cache :The text was updated successfully, but these errors were encountered: