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

Commit and Rollback should be optional #27

Closed
Bhullnatik opened this issue Sep 16, 2017 · 5 comments
Closed

Commit and Rollback should be optional #27

Bhullnatik opened this issue Sep 16, 2017 · 5 comments

Comments

@Bhullnatik
Copy link

Pretty much following up on this issue redux-offline#72 in this repo since it is actively maintained.

What do you think? That seeems like a reasonable idea, as you don't necessarily want to dispatch a commit/rollback action for every offline action, for example in the case of an optimistic update (as shown in examples in the README).

@sebasgarcep
Copy link

I personally agree with this. The change is simple and it will not break the library. @sorodrigo @wacii Any thoughts?

@wacii
Copy link
Collaborator

wacii commented Sep 19, 2017

Yeah, I'm fine with this. Not sure that I can think of a situation where I wouldn't want to be notified of a failure, but there is no reason for the library to impose that.

@sorodrigo
Copy link
Owner

I'm not sure about the use case. If you don't want to be notified about a failure, why not use an effect outside of redux-offline? I get that you might want to take advantage of the retry strategy, but then again why not add a rollback action in case it doesn't succeed? However this isn't a breaking change so not sure I disagree either

@sebasgarcep
Copy link

I have an use case right now where an action updates the local database and then syncs with the backend. These actions will be retried until they succeed and no conflict resolution has to be done on them, so they eventually will. Doing these actions outside of redux-offline means I have to reinvent the wheel to have a retry strategy in place, which doesn't make any practical sense. Unless I'm doing something wrong, I think this might be a valid use case.

@sebasgarcep
Copy link

#30 Solves this.

@wacii wacii closed this as completed Sep 27, 2017
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