Skip to content

Idea: Batching actions #542

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

Closed
danielkcz opened this issue Aug 16, 2015 · 21 comments
Closed

Idea: Batching actions #542

danielkcz opened this issue Aug 16, 2015 · 21 comments

Comments

@danielkcz
Copy link

What about being able to dispatch more actions at once? Currently the dispatch accepts single object as a result from action creator. Could it perhaps accept an array? It would then process all the actions sequentially before invoking change.

I've come to the situation in my editor app, that on press of a key I have to split the text (updating text of a current line), create a new line (with remaining text) and set focus on that new line. As of now I have this logic within single reducer, but that makes reducers rather complex and not really reusable. To keep a logic out off reducers, it means I would have to dispatch three consecutive actions. That means the re-render after each one. Perhaps I am worried too much and it might be just fine like that?

I guess that another option could be having a single action that would be basically processed by three different reducers, but I would have to basically call some external functions from reducer to provide necessary changes. Some other time I just want to update text of line or just create a new line. That would mean a different set of actions.

@ForbesLindesay
Copy link
Contributor

That last option sounds best, it's generally best to fire actions that describe what happened (e.g. User typed character) rather than what mutations you want (e.g. Update text of line)

@quirinpa
Copy link

I'm using this middleware i made:

export default function nextMiddleware({ dispatch }) {
    return (next) => (action) => {

        let ret = next(action);
        const { then } = action;

        if (then) {
            let nextActions = then;
            if (!Array.isArray(nextActions))
                nextActions = [nextActions];

            nextActions.map(item => dispatch((typeof item === 'function') ? item(dispatch) : item));
        }
        return ret;
    };
}

it's available on my profile as a repo idk if it's updated though.

I use it to dispatch notifications when the user tries to login or register

@danielkcz
Copy link
Author

@ForbesLindesay I guess you are right, but it means I would need merge result from all interested action creators. The ogic about "what is happening" have to be combined at the time of dispatch and then split again in reducers. It's kinda tied together and it would be also harder to examine such dispatch seeing data from all action creators there.

@quirinpa I haven't though about middleware, that might be good way to go. Thanks !

@taylorhakes
Copy link
Contributor

@quirinpa You can't do it with just middleware. Each call to dispatch triggers all the subscribers.

You need a higher order store that wraps the dispatcher and only calls the subscribers once.

@quirinpa
Copy link

@FredyC No problem man :)
@taylorhakes It is working for me. My subscribers are getting fired fine (i think). I'm actually not using this currently but i use something similar:

const SUCCESS = 'SUCCESS';
const FAILURE = 'FAILURE';

function bindPayloadOrNext(type, status, then, dispatch) {
    return payload => {
        if (then) dispatch({ type, status, payload, then: then(payload) });
        else dispatch({type, status, payload});
    };
}

export default function clientMiddleware(client) {
    return ({ dispatch/*, getState */}) => {
        return (next) => (action) => {

            const { type, promise, onSuccess, onFailure, ...rest } = action;
            if (!promise) {
                return next(action);
            }

            // console.log('MIDDLEWARE CLIENT ', action.type);
            // console.log(action);

            next({type, ...rest});
            return promise(client).then(
                bindPayloadOrNext(type, SUCCESS, onSuccess, dispatch),
                bindPayloadOrNext(type, FAILURE, onFailure, dispatch)
            );
        };
    };
}

which is an adaptation of the ClientMiddlewareCreator from erikras' react-redux-universal-hot-example.

I use it in my login action:

import { LOGIN, REGISTER } from './';
import { notify } from './notes'; // this is my other action

export function login(username, password) {
    return {
        type: LOGIN,
        promise: (client) => client.post('/login', {
            data: {
                username: username,
                password: password
            }
        }),
        onSuccess: () => notify('Login successful!', 'ok'),
        onFailure: notify
    };
}

it seems to be working properly. What am i missing? cheers.

@taylorhakes
Copy link
Contributor

The original question was about batching actions. He wanted to call a bunch of actions and then trigger the subscribers only after all the actions. Your code takes an array of actions and calls the dispatch for each action. It doesn't batch the actions (at least the way he originally wanted).

If you take a look at the dispatch function https://github.com/rackt/redux/blob/master/src/createStore.js#L92, you will see that your example code calls the subscribers multiple times (once for each action in the array), not just once.

@tappleby
Copy link
Contributor

I wrote a higher order store store enhancer: redux-batched-subscribe which will allow you to debounce the calls to subscribe handlers so they only happen once for consecutive actions.

@quirinpa
Copy link

@taylorhakes You can use only one next action though, and if you set the "next" of that action, it basically "batches them" consecutively. Well, to each their own xD

@johanneslumpe
Copy link
Contributor

@quirinpa But it misses the point of the question ;)

@taylorhakes
Copy link
Contributor

@tappleby Very cool. That is the point I was trying to get across.

@quirinpa
Copy link

I honestly didn't even read the array part. TL;DR lol you redux people confuse me

@taylorhakes
Copy link
Contributor

@quirinpa I apologize if any of my responses sounded hostile. I want you to feel like a redux person! The more community involvement the better. I was only trying to bring up a point about how redux works.

Everyone is relatively new at using redux (it's only a few months old) and is trying to figure out the best ways to develop with it. Most times there is no definitive right or wrong answer.

@mindjuice
Copy link
Contributor

There is also redux-batched-updates from @acdlite.

@quirinpa
Copy link

@taylorhakes :) nah, not hostile. Ofc i appreciate your input most of the times i ask stuff in order to learn more, but i know how i can sound :P ur awesome and the community aswell :D, i still have a lot to learn but ofc i want to be a part of it. xD cheers! And thanks!

@danielkcz
Copy link
Author

Thank you guys. I guess that the approach of @tappleby sounds the most useful for my case. My reducers are synchronous, so I can easily use setImmediate and have it to invoke subscribers after all the actions are processed.

Using batchedUpdates sounds not that useful for this case or perhaps I am missing a point.

@danielkcz
Copy link
Author

Hmm, I've just realized that it's not solution ideal too, because it means that every dispatch would become asynchronous even for the cases where I don't need it (single action). Maybe that batchedUpdates might be more useful after all. That way the React would decide when to actually re-render and I can keep dispatching without worries.

@nickdima
Copy link

I'm using redux-combine-actions together with redux-batched-updates for combining/batching actions.

@danielkcz
Copy link
Author

@nickdima Thanks, that looks very interesting. I like the idea of having a combined action and then followed by separate actions.

@quirinpa
Copy link

i use these now ^^ :

export default function alsoMiddleware() {
    return (next) => (action) => {

        if (!action.also) return next(action);

        let ret = next(action);
        const { also } = action;

        let alsoArr = also;
        if (!Array.isArray(alsoArr))
            alsoArr = [alsoArr];

        alsoArr.forEach(item => next(item));

        return ret;
    };
}

import {defer} from 'lodash';

export default function deferMiddleware({ dispatch, getState }) {
    return next => action => {
        if (action.defer)
            defer(() =>
                dispatch(action.defer(getState())));
        next(action);
    };
}

export default function clientMiddlewareCreator(client) {
    return () => {
        return (next) => (action) => {
            if (!action.promise) return next(action);

            const { type, onSuccess, onFailure, ...rest } = action;

            next({type, ...rest});
            return action.promise(client).then(
                (payload) => next({
                    status: STATUS.success,
                    defer: onSuccess.bind(null, payload),
                    payload,
                    type
                }), (error) => next({
                    status: STATUS.failure,
                    defer: onFailure.bind(null, error),
                    error,
                    type
                }));
        };
    };
}

Still thinking middleware is awesome XD
.... didn't mean to occupy so much space lol sorry

@gaearon
Copy link
Contributor

gaearon commented Oct 27, 2015

The approach we suggest is this: #911 (comment)

@pbadenski
Copy link

Link is broken. Correct link: #911 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants