search

day8 / re-frame-http-fx

A re-frame "effects handler" for performing Ajax tasks (via cljs-ajax)
MIT License
259 stars 27 forks source link

New :on-request handler #18

Closed kennethkalmer closed 4 years ago

kennethkalmer commented 6 years ago

Allow dispatchers to keep track of raw xhrio objects, as per #3

kennethkalmer commented 6 years ago

I just took a stab at this as mental exercise. I'm busy wiring this up in a real life app to see how it performs in both cases mentioned in #3 (debouncing duplicate requests and cancelling long/slow requests)

kennethkalmer commented 6 years ago

As with all things non-deterministic, this is tricky. That said, my first refactorings for both cases held up quite nicely. Although it is more verbose than it was, it is very clear and has the same shape as all the other handlers in the app. Being able to place the requests in the app-db and later recall it to cancel is quite neat.

Wondering if @deepxg & @flexsurfer have any thoughts on if this implementation could solve their needs?

kennethkalmer commented 6 years ago

👋

Any feedback on this PR?

Frozenlock commented 6 years ago

As noted in cljs-ajax changelogs:

It is intended that the default implementation for JavaScript uses XmlHttpRequest directly and XhrIO is deprecated in 0.8. Be warned.

kennethkalmer commented 4 years ago

Any chance of getting this merged, or is there possibly an alternative strategy that we could use to stop relying on a fork of a great project?

I should have done a better job earlier to address @Frozenlock's concerns around cljs-ajax moving backends too. This PR only captures the return value of the original function used to perform the ajax request, and then dispatches that value to the handler. I might have missed something in the initial objection that could have been obvious to someone with deeper knowledge of the differences between Closure and XmlHttpRequest, but for this level of abstraction I don't a problem.

superstructor commented 4 years ago

Thanks for the feature @kennethkalmer 👍

http-fx is probably going to be superseded at some point in the not too distant future. Exploration of that has already been underway since last year in http-fx-alpha and re-frame-fetch-fx.

But in the meantime I don't see any disadvantage to merging this improvement and its got a reasonable use case of being able to cancel long running requests.