redux-action-buffer icon indicating copy to clipboard operation
redux-action-buffer copied to clipboard

Order of actions dispatched within promises in v1.1.0

Open benjaminmock opened this issue 7 years ago • 5 comments

I discovered some strange behaviour with the 1.1.0 release. When dispatching actions within promises the resulting order is no longer correct (or actions are missing). The problem seems to be caused by the use of setImmediate/setTimeout.

I tried to replicate this behaviour in the following test (works with 1.0.1 fails for 1.1.0):

import test from 'ava'
import { createStore, applyMiddleware } from 'redux'
import actionBuffer from '../index'

test('buffers async actions in correct order', t => {
    const actionHistory = []

    const store = createStore(
        (state, action) => {
            if (action.type.indexOf('@@') !== 0) actionHistory.push(action)
            return {}
        },
        null,
        applyMiddleware(actionBuffer('BREAKER'))
    )

    const action1 = {type: 'ACTION1'}
    const action2 = {type: 'ACTION2'}
    const breaker = {type: 'BREAKER'}

    store.dispatch(action1)

    return new Promise(resolve => {
        store.dispatch(breaker)
        store.dispatch(action2)
        resolve()
    }).then(() => {
        t.is(actionHistory[0], breaker)
        t.is(actionHistory[1], action1)
        t.is(actionHistory[2], action2)
    })
})

benjaminmock avatar Jun 06 '17 11:06 benjaminmock

I think this commit broke it: https://github.com/rt2zz/redux-action-buffer/commit/d5e7222c64a00ba177bdaf8ac1dc179244d366e8

~~likely due to the setImmediate usage. I will revert.~~

rt2zz avatar Jun 13 '17 17:06 rt2zz

actually @benjaminmock I am not sure what the best resolution here is. That PR solved an issue with redux-persist. Is it incorrect if the queue drains on setImmediate? I am uncertain but open to input. Your test should be fixed with

    return new Promise(resolve => {
        store.dispatch(breaker)
        store.dispatch(action2)
        setTimeout(resolve, 1)
    }).then(() => {
        t.is(actionHistory[0], breaker)
        t.is(actionHistory[1], action1)
        t.is(actionHistory[2], action2)
    })

rt2zz avatar Jun 13 '17 17:06 rt2zz

note: I agree this is messy, just not sure what best resolution is. We can probably fix the original issue in redux-persist but that will take more time and care.

rt2zz avatar Jun 13 '17 17:06 rt2zz

@rt2zz Thank you for your input. So far I could not come up with a clean solution either.

The test covers a perfectly valid use case. action1 triggers a loading indicator, action2 hides it in a promise, that is returned by the http client. With the current release the loading indicator is shown after the request returned successfully. Fixing this with another setTimeout feels quite wrong (and may even be not possible in every case).

For now my fix is to just use your 1.0.1 release. But I'll have a look at the code again.

benjaminmock avatar Jun 19 '17 08:06 benjaminmock

Any news on this?

Also experiencing this issue, 1.0.1 does seem to fix it for me as well. Unfortunately, I do use redux-persist, so 1.1.0 would be a betterf fit in my situation.

rafatwork avatar Mar 16 '18 14:03 rafatwork