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

dispatch can only be invoked once in each logic? #18

Closed
tylerlong opened this issue Dec 25, 2016 · 7 comments
Closed

dispatch can only be invoked once in each logic? #18

tylerlong opened this issue Dec 25, 2016 · 7 comments

Comments

@tylerlong
Copy link

tylerlong commented Dec 25, 2016

Check the code here: https://github.com/tylerlong/hello-async/blob/1d6d7159a4528b1af7a560ad9c8232b7e6c9493e/redux-logic/src/logics.js

I can show the notification, but cannot hide it.

I checked the examples in your repo and I find that dispatch is only invoked once in each logic. And I guess that's the limitation of this middleware.

@tylerlong
Copy link
Author

tylerlong commented Dec 25, 2016

If I create an extra logic and move the second dispatch there, it works like a charm.

https://github.com/tylerlong/hello-async/blob/2f037cd96da276a446822cfc1530113af1cb070d/redux-logic/src/logics.js

So I guess I am right in concluding that you can only invoke dispatch once in each logic.

Do you think this limitation is necessary? Do you think it's a big disadvantage for this library?

@tylerlong
Copy link
Author

I tried done, it doesn't work:

process ({ getState, action }, dispatch, done) {
    const id = nextNotificationId++
    dispatch(showNotification(id, action.text))
    setTimeout(() => {
      dispatch(hideNotification(action.id))
      done()
    }, 5000)
  }

@tylerlong
Copy link
Author

I also tried

processOptions: {
    dispatchMultiple: true // turn on multiple dispatch mode
  }

It doesn't work either.

@tylerlong
Copy link
Author

tylerlong commented Dec 25, 2016

It is trivial with RxJS:

image

But this project's goal is:

"I wrote the rxjs code so you won't have to."

I hope this issue could help you to find a balance between abstraction and flexibility.

@jeffbski
Copy link
Owner

@tylerlong Thanks for creating the issue.

I've put together a jsfiddle that shows how you might do this with redux-logic.

You would use the dispatch and done to perform multiple dispatch, otherwise it assumes a single dispatch. I'm not sure why it didn't work for you when you tried this, but you might compare your own code against the jsfiddle or the snippet below.

http://jsfiddle.net/ezp58328/

let nextNotificationId = 0
const showNotificationWithTimeoutLogic = createLogic({
  type: 'SHOW_NOTIFICATION_WITH_TIMEOUT',
  process ({ getState, action }, dispatch, done) { // include done for multidispatch
    const id = nextNotificationId++
    dispatch(showNotification(id, action.text))
    setTimeout(() => {
      dispatch(hideNotification(id))
      done();
    }, 5000)
  }
});

You could also return an observable if you would prefer to use them. I want people to be able to use whatever they are comfortable with. (callbacks, promises, observables, async/await).

let nextNotificationId = 0
const showNotificationWithTimeoutLogic = createLogic({
  type: 'SHOW_NOTIFICATION_WITH_TIMEOUT',
  process ({ getState, action }) { // can just return an observable too
    const id = nextNotificationId++
    return Observable.merge(
      Observable.of(showNotification(id, action.text)),
      Observable.of(hideNotification(id)).delay(5000)
    );
  }
});

It is a challenge to find the right abstractions for people. I'm always looking to improve them, so I do appreciate advice and suggestions.

@tylerlong
Copy link
Author

@jeffbski

I tried again and it did work! I remember that I tried everything possible. Maybe I made some mistake or I forgot to compile the code or clear the cache. I've no idea what was wrong but now it works like a charm!

Here is the sample project I created: https://github.com/tylerlong/hello-async/tree/master/redux-logic Feel free to send me PR if you want to change anything there.

Feel free to close this issue and thank you for the help!

@jeffbski
Copy link
Owner

Fantastic. I'm glad it worked this time.

I like your project hello-async. It is really good to be able to see different ways to accomplish things side by side.

I'm going to check it out more thoroughly when I have time, but it looks like a great idea.

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

2 participants