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

Allow persistence and filtering #14

Closed
wants to merge 3 commits into from

Conversation

wtrocki
Copy link
Contributor

@wtrocki wtrocki commented Oct 19, 2018

Motivation

Allow queue persistence and filtering.
Support use cases when:

  • Queue should only process mutations
  • Page was refreshed and queued elements will be removed

Implementation

Using interface that can match local storage or any other platform storage
No extra packages
No change in current behavior (additional object passed to add extra storage and filter )

TODO

More unit tests

@codecov-io
Copy link

codecov-io commented Oct 19, 2018

Codecov Report

Merging #14 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #14   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines          25     55   +30     
  Branches        3     12    +9     
=====================================
+ Hits           25     55   +30
Impacted Files Coverage Δ
src/QueueLink.ts 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f0d77e4...da060c3. Read the comment docs.

@troyberg
Copy link

Thanks for taking the time to try and improve this package, @wtrocki! These 2 features are the final missing pieces for a React Native app I am currently working on. However, when I try testing your branch, my app build begins failing with no error output, which is making it difficult to locate where the conflict is. Just wondering if you might have any clues that would help determine why this is happening. I'd love to test these features out.

@wtrocki
Copy link
Contributor Author

wtrocki commented Nov 18, 2018

Thanks for heads up. I haven't checked React Native use case yet. I think I need to improve this PR by:

  • Adding debug logging
  • Adding apollo client to reschedule operations (rebuild observer chain after restart)
  • Test both React Native and Browser use cases

@wtrocki
Copy link
Contributor Author

wtrocki commented Nov 20, 2018

I have done some experiments and I think that this PR will change behavior of the package to such extend that it will need to be renamed to not cause issues with current implementations

If you looking for already functional example of the queue with offline support see https://github.com/aerogear/offline-conflict-strategies/tree/master/web/src/sdk/mutations

Just copy this two classes into your app
I'm going to port that to separate apollo-link-offline-queue

@helfer
Copy link
Owner

helfer commented Dec 7, 2018

Hi @wtrocki, first off, thanks a lot for the PR and my apologies for not responding sooner.

I agree that this is a fairly significant change that might be better off in a separate package.

Alternatively to this implementation for persisting the queue, how about adding public functions that allow getting and setting the content of the queue? That way, any necessary persistence/hydration for orderly shutdowns can be done by an external function.

If the goal is to provide persistence + replay even in the case of unplanned shutdowns, then I think that concern would be best solved in a separate link which persists operations while they're pending, and replays all operations on startup. However, like I commented in another thread, this will not give you the kind of offline support you may expect when using it with Apollo Client, because it's not enough for the link to replay the requests, the store also needs to know about pending operations if their results should be optimistically showed to the user.

@wtrocki
Copy link
Contributor Author

wtrocki commented Dec 7, 2018

Fully agree. Closing PR.

@wtrocki wtrocki closed this Dec 7, 2018
@wtrocki wtrocki deleted the persitence-filtering branch December 7, 2018 10:18
ikkibower added a commit to ikkibower/apollo-link-queue that referenced this pull request Dec 3, 2019
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

Successfully merging this pull request may close these issues.

None yet

4 participants