Skip to content
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

Doesn't seem to be calling connect after actions are batched? #1

Closed
colbycheeze opened this issue Jan 31, 2017 · 11 comments
Closed

Doesn't seem to be calling connect after actions are batched? #1

colbycheeze opened this issue Jan 31, 2017 · 11 comments

Comments

@colbycheeze
Copy link

colbycheeze commented Jan 31, 2017

Not sure exactly what is happening, but none of the components update their props after a set of batched actions are dispatched. The store has all of the updated data, but not the components.

This happens with arrays passed OR even normal dispatches after hooking up the reduxBatch middleware via

  const store = createStore(rootReducer, composeEnhancers(
    reduxBatch, applyMiddleware(...middleware), reduxBatch
  ), initialState)
@arcanis
Copy link
Contributor

arcanis commented Jan 31, 2017

Hm that's weird, I just checked and listeners seem to be correctly called, @connect should have no issue being notified of store updates. Can you try using the 0.0.2 version I just pushed? It should fix a small issue where too many notifications were sent, maybe it will also solve your problem.

@Noitidart
Copy link

Excuse the off topic @colbycheeze -
Thanks @arcanis for your fast-awesome maintenance/support - I have been thinking super hard for like 2 months now to pick up a batching lib but saw so many issues. After seeing your support and reading the whole topic you based this on - reduxjs/redux#1813 (comment) - I am going to give this a go :D <3 Thanks for it!

@arcanis
Copy link
Contributor

arcanis commented Jan 31, 2017

Thanks, let me know if it works for you :) I've been using it on a project of mine with success (cf https://github.com/manaflair/todo-demo for a small demo project that shows how to use it), but sometimes subtle differences in environments (or dependencies updates) can break things. I hope it will be fine!

@jimbol
Copy link

jimbol commented Mar 28, 2017

Has this issue been resolved? I think I am still seeing it.

Edit: To clarify, notifyListeners is getting called but subscribers aren't actually notified. Effectively, these actions don't get called until a not-batched action updates subscribers.

@arcanis
Copy link
Contributor

arcanis commented Mar 28, 2017

Would you happen to have a minimal testcase I could use to reproduce the issue? I haven't yet experienced this bug myself, so it would definitely help. If not, I'll try to make one on my side later this week.

@arcanis
Copy link
Contributor

arcanis commented Mar 28, 2017

I've tried the following code, and it seemed to work fine, can you check on your end?

let redux = require('redux');
let reduxBatch = require('@manaflair/redux-batch').reduxBatch;

let store = redux.createStore((state = {}, action) => {
    console.log(action);
    return state;
}, reduxBatch);

store.subscribe(state => {
    console.log('been notified!');
});

store.dispatch({ type: 1 });
store.dispatch([ { type: 1 }, { type: 2 } ]);
{ type: '@@redux/INIT' }
{ type: 1 }
been notified!
{ type: 1 }
{ type: 2 }
been notified!

If it does, then I'll need some guidance to reproduce the exact issue. If it doesn't, please tell me which version of redux and redux-batch you're currently using (yarn list / npm list should give you this information).

@jimbol
Copy link

jimbol commented Mar 29, 2017

Yea by the looks of it, it should work. Perhaps it's having issues in combination with redux-saga. I'll try to find some time to investigate. We switched to redux-batched-action for now.

@arcanis
Copy link
Contributor

arcanis commented Mar 31, 2017

I have fixed an edge-case issue that could be the cause of the behaviour you've experienced, but without test case it's hard for me to be sure (cf 8c17160). If you happen to try again, let me know if it works better with the release 0.0.3 :)

I'll close this issue for the time being, but feel free to reopen it if it's not resolved.

@arcanis arcanis closed this as completed Mar 31, 2017
@jimbol
Copy link

jimbol commented Mar 31, 2017

Thanks for looking into it!

@arcanis
Copy link
Contributor

arcanis commented Apr 17, 2017

Bad news: I believe the bug's still there. Good news: I have a testcase \o/ cf #4

@arcanis
Copy link
Contributor

arcanis commented Apr 17, 2017

I just released a v0.1.0 that should fix this bug.

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

No branches or pull requests

4 participants