-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Discussion: declarative side effects and data fetching approaches #349
Comments
If you had asked developers what they wanted, they would have said they want to easily mutate state and call side-effects whenever and wherever they want.
EDIT: I'm looking over this and will provide my thoughts soon. Here's another source of inspiration: https://redux-saga.js.org/docs/basics/DeclarativeEffects.html |
I have tried Declarative Side-Effects many times and always fail to scale them. Whole system is become too fragile to accept changes. I have found that Data-Driven Side-Effects works instead. For the short:
In some sense UI is just another side effect. |
@AlexeyDemedetskiy : I think that's literally what David means by "declarative" here: // reducer:
case 'FETCH':
return {
...state,
// finite state
status: 'loading',
// actions (effects) to execute
actions: [
{ type: 'fetchUser', id: 42 }
]
}
// UI:
const { actions } = state;
useEffect(() => {
actions.forEach(action => {
if (action.type === 'fetchUser') {
fetch(`/api/user/${action.id}`)
.then(res => res.json())
.then(data => {
dispatch({ type: 'RESOLVE', user: data });
})
}
// ... etc. for other action implementations
});
}, [actions]); |
@AlexeyDemedetskiy This is insufficient. There are use-cases when a side-effect needs to be executed without the state changing. |
@davidkpiano I never met these use-cases, so your experiences can be different. |
@davidkpiano I'm a bit tired so I've just skimmed this (will give it a proper reading tomorrow), but I'm already seeing some problems with the approach you are suggesting, maybe you can already correct me on them if I read something wrong? :)
I am sure that some of these things can be addressed by additional safeguards in an actual implementation, but that would get quite complicated and potentially very long. I think the idea of declarative side effects is quite appealing, but from the examples in the blog posts, I see more problems than solutions right now. Especially binding the execution of these async actions into component lifecycle seems a bit weird to me. And I wouldn't really know how we could teach this without completely confusing people. Could you provide one example implementation that does not have the problems I described so that we can see if this is feasible? |
I've spent a lot of time thinking about and writing code using concepts like side-effects as data, so I'm very excited to see If you want a battle-tested example of writing declarative side-effects in the redux ecosystem, @markerikson and I have already had plenty of discussions about redux-saga and why it's hard to nominate it for official support. Personally, generators are so incredibly useful for async flow control that it's worth the struggle to onboard developers to the concept. There are definitely downsides but I've built many large scale web applications using Having said that, the arguments against it are still valid and I have spent some time trying to figure out how to solve those shortcomings. Here are some libraries that I've built to try to make adoption easier. I lend these to you all as possible inspiration.
Please forgive me for citing my own projects, but I do feel like they are relevant and possible sources of inspiration to discuss. |
This unfortunately doesn't solve the main gripe I have with saga (which I - in theory - love). It's unfortunate, but as it currently stands, I think the best way to go is something promise/async-based. |
Totally agree, typescript is the worst part about using generators for async flow control, no argument there. I find the compromise acceptable, but others might not. |
@phryneas @neurosnap redux-loop provides declarative side effects (without generators) and we've been improving the type safety a lot lately. We have an upcoming major release that should fix the remaining issues with it. While I am a biased maintainer, I have used it for a large application for 4 years now and find it works very well. |
Maybe I can share a little bit a real-world example I am trying to tackle right now for some inspiration (you are all thinking this problem much better than me).
Any time one of this parameter changes, I need to re-fetch the discounts (from a specific API endpoint) Solution 1: call
|
@martpie I'd dispatch a thunk from a component - the thunk itself would decide if there is actually a need of refreshing some things, depending on when the last refresh was etc. (probably overridable from a thunk argument, so that lifecycle refresh could be handled differently from "the user pressed a refresh button" refresh) I don't really see why your "aggregation of data" there would be more complicated than in any other solution (given that thunks have access to the full state), but I believe that those values might actually be derived values - and those should never be persisted in the store, but rather be calculated in a (possibly memoized) selector. |
@martpie In case of redux-loop, you can make an action of REFRESH_DISCOUNT which only run API and doesn't change state case 'REFRESH_DISCOUNT':
return loop(state, Cmd.run(refreshDiscount, {
args: [getArgs(state, action)]
}); then whichever action that would trigger the refresh should call the action via case 'ADD_PRODUCT':
const newState = addProduct(state, action.product);
return loop(newState, Cmd.action({ type: 'REFRESH_DISCOUNT'; additionalParams: ... })) So you can refresh the discount from any slices. If you put the handler of REFRESH_DISCOUNT on reducer high enough so it has access to any necessary state, I think there is no problem with data aggregation |
You can also pass case 'REFRESH_DISCOUNT':
return loop(state, Cmd.run(refreshDiscount, {
args: [whatever, whatever2, Cmd.getState, Cmd.dispatch]
});
//refresh-discount.js
function refreshDiscount(whatever, whatever2, getState, dispatch){
const myPieceOfState = getState().my.piece.of.state;
dispatch({type: 'hello'});
} |
How sure are we that "declarative effects" are what we want? I thought it was, but the more I think about it, the more I realise I want first class support for data driven effects — React isn't just declarative, it's data driven — to update the view you update the data. I'm not sure that "effects are data" is the solution. In this case, it would still be possible to end up in an illegal state, where you've updated the state to a position that requires a side effect, but you don't also manually add the side effects the state. I think the quite widespread misuse of React's useEffect to create data driven side effects (i.e, dispatch this action when this state changes and this condition is true) shows that there is appetite for this. I would love to be able to set my state anywhere in my reducer, and the related side effects to just work. I like the idea that React UI is just another data driven side effect. However, React redux currently implements this via a store subscriber, which has a shortcoming, which is that it's very difficult to implement side effects that depend on order of execution. I.e, I need to perform a side effect before react redux renders with this new state, or I need to perform one after. Scheduling of side effects vs react redux definitely needs to be part of the discussion. Currently redux just runs all store subscribers in order of attachment to the store- I don't think this is sufficient. |
Just to add to my above comment, the reason why I consider the use of useEffect for one time side effects abuse is because it's very difficult with the api of useEffect to predict how frequently an effect will run, especially if you need to use multiple distinct pieces of data in your effect but then only want the effect to fire when some of those change, not all. Because of this it's generally best practice I believe to make your React effects resilient to overfiring- I consider it most useful for syncing data between React's declarative model and other imperative APIs you may need to use. I don't think it's actually a good api for firing one time data driven side effects, and I don't think the React team do either, |
I don't feel particularly qualified, but I'd like to drop my 2 cents. I don't think redux-loop fits well with the current status of RTK. Does returning a So what if we had an RTK native way to "queue effects" what might that took like Here's a half thought out idea: an effects object baked into createSlice. const slice = createSlice({
name: 'user',
initialState: {status: 'idle'},
reducers: {
gotUser(state, action) {
// ...
},
getUser(state, action) {
// ...
slice.effects.fetchUser()
},
getUserFromInput(state, action) {
// ...
slice.effects.handleInput(action.payload)
}
},
effects: {
async fetchUser() {
const user = await fetchUser()
return slice.actions.gotUser(user)
},
async handleInput(input, canceled) {
await wait(300)
if (canceled()) return
const user = await fetchUser(input)
return slice.actions.gotUser(user)
}
}
}) The calling If the same effect function is called while still being processed it's automatically canceled. I think this works only because it's baked into createSlice, it wouldn't work as a traditional middleware. I think the benefits of this approach are:
And to be clear I'm not suggesting this as an API. I'm just trying to broaden the discussion beyond "What middleware is best," because with toolkit there's an opportunity to use |
By using callbacks(hell) and promise-chains the javascript runtime is now comparable to windows 3.x that did cooperative multitasking (the program is giving back control to the os after a chunk of work like the promise function would give back control to the js runtime). At this operating system level it's absolutely necessary to handle parallelism and concurrency. Even single-threaded, it's just easier because your functions are atomic operations. By using redux thunk you are basically just sending a prayer to heaven: "don't let any race condition happen!". Let me explain it by a very very common requirement, that should be ridicolously simple: I'm pretty sure that it's very very hard to write a correct login/logout flow without race conditions with thunks. E.g. consider login attempts that are failing after the user has logged out because the sendLogout-call is hanging until network timeout for 30 or 60 seconds. With Thunks alone to code a somehow working login/logout flow you have to ignore race conditions. You have to ignore that the mobile network could be very slow. You have to ignore that results of login/logout/retry login because of lagging networks could come back in different order. If you do not ignore these things, the example gets very complicated because of the necessary thunks micromanagement. So to sum it up: redux-thunk is hardly solving any real-world problem. It's just for hobby projects or people who don't recognize their programming errors. Redux-saga is solving a big pain point (handling parallelism and concurrency without getting insane), that has always been a pain in computer science. Therefore it should be in IMHO. |
@RobIsHere FWIW, I agree that thunks have limits in how they can interact with data flow. However, I do not intend to make sagas a default in RTK for all the reasons listed here: |
How about augmenting a thunk with some basic cancellation similar to how |
I think you should look at createAsyncThunk. I believe that has cancellation
…On Fri, Nov 6, 2020 at 5:37 PM allforabit ***@***.***> wrote:
How about augmenting a thunk with some basic cancellation similar to how
useEffect does it? Maybe dispatch could return a cancellation fn and if
it's called it won't fire the thunk callback. I'm not sure how feasible
returning a fn from dispatch would be though!
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#349 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA4MSMPJBHQCHPEHR4V4ANDSOR3C7ANCNFSM4KSQCWIA>
.
|
Yep, function MyComponent(props: { userId: string }) {
const dispatch = useAppDispatch()
React.useEffect(() => {
// Dispatching the thunk returns a promise
const promise = dispatch(fetchUserById(props.userId))
return () => {
// `createAsyncThunk` attaches an `abort()` method to the promise
promise.abort()
}
}, [props.userId])
} That's certainly not as flexible as sagas or observables, and I'm not at all arguing it's a "best" or "final" approach, but it is more than you would typically get with a hand-written thunk. |
based on your answer, can I propose some help from my side? I'm ready do this by myself, because that's the only one thing that I miss in RTK right now.
@markerikson can you please let me know you think about this approach? |
@megagon That doesn't seem like something that would go inside of the thunks themselves, but rather in a middleware of some kind. I'd suggest playing around with writing a custom middleware first that does this sort of thing and see where that goes. |
Do you suggest just to write custom middleware internally for us? Or do you mean to make a middleware that will be delivered with all others redux's thunks? |
I'm saying that A) I think the logic of "tracking what promises exist" seems more suited for a custom middleware, and B) I'd like to see a working example in action to motivate further discussion. |
@RobIsHere I'm thinking about the list of requirements for redux-observable, and I'd say the list is somewhat like this:
@megagon |
I think a solution would be to hide saga behind some Interface. Then make ready-made building blocks like LEGO. There could be a
All composable and backed by sagas that execute unknown to the beginner-user. A pro-user could reuse the building blocks in his sagas. A beginner-user just composes some function calls and runs them and waits for a promise resolution/cancellation while he is totally unaware of the saga(s) working in background. That is my way of doing it. But I don't need this baked into redux-toolkit. Although I'm astonished a little bit because writing a new middleware is also like writing a saga but a lot harder. So where should this lead if everyone writes a new middleware for every problem he has? (reactive things like rxjs is something I do not use because of testability, code quality and maintainability) |
So at this point it feels like the "declarative data fetching" thing is going to be covered by RTK Query: https://rtk-query-docs.netlify.app Obviously when it does come out no one will have adopted it yet, and like any other new API, it will take a very long time to get picked up. But, in a sense we could say that it does "solve" the data fetching use case, and that's certainly the majority of the reason for doing async work with Redux. I'm still open to further discussions on this topic, but at that point it feels like there's less need for us to build in anything effects-wise. My gut says Maybe I'm being too dismissive of the options, but there's also a lot of inertia in the ecosystem at this point and I have to take that into account. |
I will throw in some thoughts, but warn me if this is not the correct place. I am a pretty noob Redux user. This discussion has gone mostly about the reducer side of things but I find the "consumer" side to be a harder concept. For the reducer side, I had written something very much like RTK Query and it is working for me so far. But there are some cases which it does not handle well. Suppose the following use case: // section of Login page component
const { data, error, isLoading } = useSelectLoginResult();
useEffect(() => {
if(error) showSnackbarForFiveSeconds(error);
}, [error]);
const onFormSubmit = () => dispatch(doLoginRequest(username, password)); This will briefly show a snackbar when there is an error in the login request. (I abstracted The first problem with this is, when this component is unloaded and loaded again, the snackbar will show again. Because The second problem is, when login request is done again, and the error is the same, the snackbar will not appear. Because Maybe because I am coming from an OOP and event-driven background but an event feels more right in this particular case. Something like this should be easy to write: // Reducer side
const loginRequestThunk = () => async (dispatch, getState, dispatchEffect) => {
try {
await fetch(/* ... */);
// dispatch etc.
} catch(error) {
dispatchEffect('loginError', error);
}
}
// Component side
useDispatchedEffect('loginError', (error) => showSnackbarForFiveSeconds(error)); But do you think would go well with the principles of Redux? |
Yeah, that sort of "one-off trigger" behavior has always been tricky with React and Redux in general, specifically because it doesn't entirely mesh well with the notion of "UI = f(state)". Hypothetically, one option would be to not actually trigger the toast itself based on the existence of I'm not saying this is the best way to handle this or the only way to handle this - it's just the first thing that pops into my head reading your comment. I'm actually kind of curious how XState handles this sort of scenario. Don't want to totally derail the thread, but if @davidkpiano would like to drop a comment describing that approach I'd be interested. |
This is one of the biggest pitfalls of effectful logic in React in general - it's based on what changed, instead of based on events and declarative effects. State machines handle when effects happen explicitly, and they always happen due to an event (on transitions). To make organization easier, statecharts define the 3 areas where effects can be defined:
Considering the above problem, we can quickly sketch a (partial) state machine that looks like this: XState takes the state-machine approach of So, the resulting event-state pairs might look like this:
So, because To emulate this, you need to somehow represent this "transition", so the // pseudocode!!
useEffect(() => {
if(error && previousStateWas(loading)) {
showSnackbarForFiveSeconds(error);
}
}, [error]); In Redux (with reducers), you can represent these declarative effects with a tuple: // Notice how effects from previous state are discarded each time
const reducer = ([ justTheState, ____ ], action) => {
// ...
return [nextState, effects];
} In XState, the above pattern just has a bit of structure around it: // ...
on: {
FAILURE: {
target: 'error',
actions: (_, event) => showSnackbarForFiveSeconds(event.error)
}
} |
Yeah, I thought it was likely something along those lines. Thanks for the clarification! |
Why not repurpose this thread to "finite declarative transitions and effect in redux"? No pun here, and no competition with #xstate or the amazing work @davidkpiano is doing with it, but there's a big potential here. Given the popularity of redux (especially in enterprise and large teams) and how projects based on it can benefit from refactoring to rtk-toolkit, it wouldn't be a far fetch to envision an API to write finite-state-machines and declarative effects with it. What is currently missing and what are the limitations? |
A user did open up #1065 a few days ago to discuss specifically adding some kind of state-machine-related util to our API, so there's overlap with this discussion. |
Awesome!will move to that ticket then, thanks @markerikson! |
Please make this stuff a peer-project that can be installed only if desired. So devs doing things like these in saga can continue to use redux toolkit without forking only to get the superflous stuff out. Things like #1065 are a piece of cake in saga, less than an hour of work, and everyone using saga already has an implementation like this. |
@RobIsHere Can you please show a code example (gist, etc.) of such an implementation? |
@RobIsHere something like that would probably be tree-shakeable or if it became too big (which sounds unlikely), maybe it's own entry point. But if we decided that we add it to RTK, it would not be it's own package. I assume that before people start forking things wildly to shave off that last kilobyte, they probably just turn on tree shaking in their bundler, with much higher gains over all their dependencies. |
I love using sagas. I also love redux-toolkit. So I've created a package that glues these 2 awesome things together: saga-toolkit. |
Reviving this thread a bit: I've been working on fleshing out a new "action listener middleware" that would allow running callbacks in response to specific actions (or potentially state changes). At the moment, I think we can actually replicate a decent subset of what you can do with sagas, like I just did some brainstorming on the difficulties of cancelation and some potentially interesting looking bits to investigate at #1648 (comment) . I'm very curious how much overlap there is between the current concept for that listener middleware, and this discussion - particularly things like longer-running workflows, error handling, and cancelation. On the one hand, we don't actually want to turn this into a full-blown attempt to rebuild sagas/observables with a homegrown equivalent. On the other hand, if we solve some of the same use cases by adding a few specific primitives and getting some decent bang for the buck in terms of API complexity and bundle size, that could be useful. Anyone in this thread have thoughts on potentially useful capabilities, overlap with things like state machines, desired side effects approaches, etc? Like, if someone were to pop up and say "here's how to do exactly the same things this prototype middleware does with XState or a similar FSM lib plugged into Redux", that would be a valuable piece of information ( paging @davidkpiano ), as would "this middleware is going in the wrong direction and here's a better alternative approach". |
This thread has been sitting around for a while. Lemme recap where things stand today:
|
@markerikson So the |
@zoubingwu those two apis complement each other, they don't replace each other. They have completely different mindsets and make sense in different situations for different use cases. None of them will be deprecated for the other. |
@phryneas OK, I just played with |
@zoubingwu A thunk is much more "direct and compact" - you dispatch the order to do something, and actions will be dispatched as a side effect (at least if using The listener middleware is "hey, if this (related or completely unrelated) things happens, also do this. |
I don't think we've got a lot more to discuss here, so I'm going to go ahead and close this issue. Happy to discuss more if anyone does have further ideas for improvements! |
Currently, Redux Toolkit adds thunks as the default supported approach for any kind of async / side effect behavior, largely because it's the simplest possible approach that works, and because it's also the most widely used Redux async middleware.
That's a large part of why I'm considering adding some kind of a
createAsyncThunk
API to abstract common data fetching and action dispatching logic. It matches what we already have built in to RTK, and what many Redux users are already doing.@davidkpiano has strongly argued that side effects should be declarative, instead, possibly along the lines of the
redux-loop
store enhancer.I'm open to discussing possible APIs and approaches here. My initial point of concern is that this is a very different kind of API than most Redux users are used to, and it would add an extra level of teaching and documentation to explain.
Discuss!
The text was updated successfully, but these errors were encountered: