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

[Feature request]: Aborting queries #50

Open
nenadalm opened this issue Oct 2, 2019 · 11 comments
Open

[Feature request]: Aborting queries #50

nenadalm opened this issue Oct 2, 2019 · 11 comments

Comments

@nenadalm
Copy link

nenadalm commented Oct 2, 2019

Hi. It would be nice if I was able to abort query, e.g. if user changes page before previous one was fully loaded.

I found that the http library seems to support that, but there is no access to the channels from re-graph: https://github.com/r0man/cljs-http/blob/235d45d889f93d3ea79fba772d243eaa122806cc/src/cljs_http/core.cljs#L11

Maybe If I could somehow tag the query and then was able to cancel all queries tagged with the tag?

;; loading dashboard data with 2 queries to not block in case one query will take significantly longer than other
(re-frame/dispatch [::re-graph/query
                    "{ things { id } }"
                    {:some "variable"}
                    [::on-thing]
                    {:tag :dashboard}])
(re-frame/dispatch [::re-graph/query
                    "{ things2 { id } }"
                    {:some "variable"}
                    [::on-thing]
                    {:tag :dashboard}])

;;;
;;; Go to different page
;;;

;; cancel any in-progress requests from previous page
(re-frame/dispatch [::re-graph/cancel-query {:tag #{:dashboard}}])

;; load current page data
(re-frame/dispatch [::re-graph/query
                    "{ user_things { id } }"
                    {:some "variable"}
                    [::on-thing]
                    {:tag :user-list}])
@oliyh
Copy link
Owner

oliyh commented Oct 5, 2019

Hi,

There exists the concept of cancelling subscriptions. Subscriptions are given an id (for the purposes of cancellation) so I think a similar mechanism could work for queries.

I think it would be up to your application to keep track of what query ids belong to which tag, as you've described it above - I don't think that belongs in re-graph. Would also have to make sure it would work for the clj implementation as well.

@oliyh
Copy link
Owner

oliyh commented Oct 5, 2019

Note that the query will almost certainly have been received on the server so the effect would really be to ignore the response which could be achieved without actually cancelling the HTTP request.

@nenadalm
Copy link
Author

nenadalm commented Oct 7, 2019

Cancelling http request would also have advantage of not receiving data from the server (which might be nice in case of many requests and slow connection). But ok I guess this can be closed then?

@oliyh
Copy link
Owner

oliyh commented Oct 7, 2019

Hi,

The 'cancel-query' could be implemented so I'm happy to keep this open for that, if it would be any use?

@nenadalm
Copy link
Author

nenadalm commented Oct 7, 2019

yes it would. thanks.

oliyh added a commit that referenced this issue Nov 1, 2019
and deduplicating inflight queries/mutations with the same id
ids are randomly generated if missing

Paves the way for #50
oliyh added a commit that referenced this issue Nov 9, 2019
@oliyh
Copy link
Owner

oliyh commented Jan 26, 2020

Hi,

I implemented this a while ago but haven't gotten around to writing the docs yet, however it is quite straightforward, see 81e877f - you need to give your queries ids:

(re-frame/dispatch [::re-graph/query
                    12345
                    "{ things { id } }"
                    {:some "variable"}
                    [::on-thing]])

(re-frame/dispatch [::re-graph/abort 12345])

Could you try it in 0.1.12-SNAPSHOT and let me know if it works as required?

Many thanks

@nenadalm
Copy link
Author

nenadalm commented Feb 9, 2020

Hi. Thanks. I've tried with websockets turned off.

First I fixed compilation (nenadalm@c79d0a8 - cljs-http doesn't have http.client/abort fn), then I tried to make request and abort it, but it wasn't aborted.

@oliyh
Copy link
Owner

oliyh commented Feb 9, 2020

Hi,

Thanks for the compilation fix, I have integrated that into master now. I think we may need a test harness with either a real GraphQL server or by seeing if we can stub out cljs-http sufficiently to test this - I think I've implemented it properly unless you can see anything glaringly obvious, otherwise I'm not sure how else to proceed.

@oliyh
Copy link
Owner

oliyh commented Feb 9, 2020

And also on clojars 0.1.12-SNAPSHOT

@nenadalm
Copy link
Author

nenadalm commented Feb 9, 2020

Thanks. I intend to take another look at this but it might take a while - should be this month.

@nenadalm
Copy link
Author

It seems that r0man/cljs-http#46 is why I can't abort requests.

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