add commitUpdate method, deprecate update #607
Conversation
| @@ -161,6 +162,10 @@ var RelayStore = { | |||
| return new RelayQueryResultObservable(storeData, fragmentPointer); | |||
| }, | |||
|
|
|||
| /** | |||
| * Adds an update to the store without committing it. The returned | |||
| * RelayMutationTransaction can be commited or rollbacked at a later time. | |||
There was a problem hiding this comment.
yay docblocks! maybe "committed or rolled back"
There was a problem hiding this comment.
Yep makes more sense 👯
xuorig
force-pushed
the
commit-update
branch
from
e09134b
to
c6d6c47
Compare
|
Addressed the comments ☕ |
xuorig
force-pushed
the
commit-update
branch
from
c6d6c47
to
3d9063e
Compare
| @@ -15,7 +15,7 @@ The Relay `Store` provides an API for dispatching mutations to the server. | |||
|
|
|||
| <ul class="apiIndex"> | |||
| <li> | |||
| <a href="#update-static-method"> | |||
| <a href="#commitupdate-static-method"> | |||
| <pre>static update(mutation, callbacks)</pre> | |||
There was a problem hiding this comment.
Also the link text: update -> commitUpdate
xuorig
force-pushed
the
commit-update
branch
from
3d9063e
to
47503fa
Compare
|
@plievone done |
|
Perhaps also the few mentions in examples and docs could be commitUpdated, as they are in the same repo and easy to grep? |
|
Good catch, I've updated docs and examples |
|
@facebook-github-bot import |
|
Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/1679180675656670/int_phab to review. |
|
@xuorig we'll land this as time permits - shipping it requires a codemod bc we don't want warnings everywhere we call |
|
👍 |
|
It seems to me that this implementation does not cover use case, when it just wants to avoid stacking many mutations in collision queue. Which is the use case for updating text input or anything where user can change values quickly and it wants to remove previous transaction if its just sitting on the queue with newer one. As far as I can see - to put mutation on the collision queue I have to commit it, but I can't rollback if it just sits on the queue and has not been sent, right? Maybe if rollback would be also possible when transaction is in |
|
@josephsavona @xuorig What do you think? Should I provide another PR that adjust |
@jardakotesovec That would be great - all that should be required is updating the status check in |
|
Does this cover the use case where I want to commit data to my local cache in different places (don't care about the transactions returned) but then just want to update everything that is stored in the cache, but now out of sync with the backend? |
|
@Globegitter It does not.. as decided in #550, control of transaction handling is given to the programmer. So with this PR, you have to keep track of the transactions on your own. But api could be extended for example to either provide function that commit all uncommited transactions as you suggested, or just provide you list of uncommited transactions so you can commit them. |
|
@jardakotesovec So right now we would have to manually keep track of each transaction that is being returned by But yeah for our use-case it would be useful to have an API to commit all uncommitted transactions (in one request). |
|
@Globegitter I think you should open new issue/PR to keep track of this use case and discuss potential improvements. Relay currently sends one http request per mutation. What you have described is correct (keeping list of transaction and committing all of them individually) |
ghost
closed this
in
Summary:This is follow up to #607 that adds posibility to rollback transaction with status `COMMIT_QUEUED`. There is one more missing piece for this use case that I was not sure how to approach. To replace transaction that is queued I need to first check if its status is still `COMMIT_QUEUED`. Issue is that transaction is removed from pendingTransactionMap once its successfully committed. So I can easily end up with exception from `getStatus()`, which is bit overreaction :-). So maybe return some status like `FINISHED` instead of throwing if its not in pending queue? I am of course very opened about naming :-). Closes #646 Reviewed By: josephsavona Differential Revision: D3021199 Pulled By: yungsters fb-gh-sync-id: 2ee1d2849d8bd5081ef57a42c136d644b61e95bd shipit-source-id: 2ee1d2849d8bd5081ef57a42c136d644b61e95bd
Follow up to #599, #550
Deprecate
RelayStore.update, rename toRelayStore.commitUpdatecommitUpdatenow returns theRelayMutationTransactionjust likeapplyUpdate.Changed to
commitUpdatein the docs.I used
warningto deprecate and also call the correct method, let me know if this is the way we should deprecate the function.