manaflair / redux-batch

Enhance your Redux store to support batched actions
171 stars 12 forks source link

This project is not obsolete #21

Closed ArcanisCz closed 4 years ago

ArcanisCz commented 5 years ago

I would like to remove this Note from documentation:

Note: Since this package got written, redux-saga improved and using all([ put(...), put(...) ]) seems to properly batch the two actions into a single subscription event, making redux-batch redundant in this scenario.

since my experiment with latest stable (to this date) react, react-redux, redux, redux-saga libraries shows, you cannot achieve batched dispatch by native means. You can see it in one of my very simple test projects https://github.com/ArcanisCz/xianxia-game/tree/all-vs-put particularly in file https://github.com/ArcanisCz/xianxia-game/blob/all-vs-put/src/game/saga.js (there is a console log in render method of simple component connected to RESOURCE_QI)

/** Two renders */
// yield put(addResource(RESOURCE_QI, 1));
// yield put(addResource(RESOURCE_QI, 1));

/** Two renders */
// yield all([
//     put(addResource(RESOURCE_QI, 1)),
//     put(addResource(RESOURCE_QI, 1)),
// ]);

/** One render */
yield put([addResource(RESOURCE_QI, 1), addResource(RESOURCE_QI, 1)]);

Should i try to recreate the simpliest case possible instead of this project?

I didnt dig deep enough to know what is going on. But i remember like year or two years ago, that react would do only one render even without all(), when there were no async call between two yield puts.

Noitidart commented 5 years ago

I think this is now handled at the react level with unstable_batchedUpdates see here - https://github.com/reduxjs/react-redux/releases/tag/v7.0.1

Batched Updates

React has an unstable_batchedUpdates API that it uses to group together multiple updates from the same event loop tick. The React team encouraged us to use this, and we've updated our internal Redux subscription handling to leverage this API. This should also help improve performance, by cutting down on the number of distinct renders caused by a Redux store update.

ArcanisCz commented 5 years ago

Oh, thanks for the info, didnt remember this one! Although i suspected, it has to do something with changes in react-redux and fibers.

There is not much info about unstable_batchedUpdates to be honest, aside from few tweets from Dan.

Should we just wait til React 17 where batched updates will be hopefully integrated natively, or change redux-batch (or create other middleware) to use react-redux feature (from that documentation you linked) for puts in all effect?

import { batch } from "react-redux";

function myThunk() {
    return (dispatch, getState) => {
        // should only result in one combined re-render, not two
        batch(() => {
            dispatch(increment());
            dispatch(increment());
        })
    }
}

EDIT: i have finally found some example of usage of unstable_batchedUpdates https://stackoverflow.com/a/52025835 , it seems it needs to be called explicitly. Also, automatic batching might not be even in React 17 https://github.com/facebook/react/pull/5880#issuecomment-416031247 .

Noitidart commented 5 years ago

Awesome research thanks for sharing!

I would look for more examples of how to use batch from react-redux for redux-saga and redux-observable and share in the readme here maybe. So we don't obselete this without telling people what we moved to.

ArcanisCz commented 5 years ago

I put together some dirty experiment, and it seems (from quick testing) this simple middleware can replace redux-batch (for my use case). But it maintains the API of redux-batch - you have to add an array to put() effect (or in dispatch()). It would be way more interesting to detect put effects wrapped in all or detect all dispatchs in one event loop to wrap them into batch() (and i dont have any ideas now about how to do it).

import {batch} from 'react-redux';

const newReduxBatchMiddleware = store => next => action => {
    if (Array.isArray(action)) {
        batch(() => action.forEach(next));
    } else {
        next(action);
    }
};

const middleware = compose(
    applyMiddleware(sagaMiddleware, newReduxBatchMiddleware),
    window.__REDUX_DEVTOOLS_EXTENSION__ ? window.__REDUX_DEVTOOLS_EXTENSION__() : (x) => x,
);

EDIT: found solution without changing API!

const newReduxBatchMiddleware = store => next => {
    const queue = [];
    let immediateId = null;

    return (action) => {
        queue.push(action);

        clearImmediate(immediateId);

        immediateId = setImmediate(() => {
            const toBatch = queue.splice(0, queue.length);
            batch(() => toBatch.forEach(next));
        });
    };
};

not sure about implications of setImmediate, but it works as expected

/** One render */
yield put(addResource(RESOURCE_QI, 1));
yield put(addResource(RESOURCE_QI, 1));

/** One renders */
yield all([
    put(addResource(RESOURCE_QI, 1)),
    put(addResource(RESOURCE_QI, 1)),
]);

/** Two renders */
yield put(addResource(RESOURCE_QI, 1));
yield delay(0);
yield put(addResource(RESOURCE_QI, 1));
markerikson commented 4 years ago

The unstable_batchedUpdates() usage within React-Redux itself only attempts to minimize the nested re-renders from within a single dispatch. That does not overlap with the goal of this project, which attempts to allow dispatching multiple actions at once while only notifying store subscribers a single time.

Absvep commented 4 years ago

@markerikson Thanks, please see my question at https://stackoverflow.com/questions/59849868/batching-actions-in-redux-saga if possible. Maybe more people will find an answer to proper way to batch redux-saga actions at SO. I myself still wonder how to do it... Best Regards

markerikson commented 4 years ago

Yep, was in the process of writing an answer there when I wrote the above comment.

Absvep commented 4 years ago

@markerikson Ah thanks a lot!

arcanis commented 4 years ago

If @markerikson says it's not obsolete, I think I can close this issue then 😄

markerikson commented 4 years ago

Yeah. For reference, I recently wrote a post on A Comparison of Redux Batching Techniques.

Absvep commented 4 years ago

One more thing, should one use batch from react-redux in React event handlers to attempt to minimize the nested re-renders from within a single dispatch when dispatching multiple actions there? Or does unstable_batchedUpdates() already handles it so dispatch is unneccesary in React event handlers? best regards

markerikson commented 4 years ago

React-Redux has to cascade component updates through the tree. React-Redux v7 uses batch() internally to help combine that work into fewer re-renders.

React already will batch renders caused by multiple queued updates within a single React event handler.

As the API doc then describes, you yourself can use batch() to also batch renders caused by multiple store dispatches somewhere outside React event handlers, like async thunks.

Absvep commented 4 years ago

Thanks a lot Mark!

sheryarshirazi commented 1 year ago

Is it still be valid usage case to use this package with recent react v18,