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

[Enhancement]: Add a default Effect Handler For Promises #808

Open
mike-thompson-day8 opened this issue Nov 3, 2024 · 7 comments
Open
Assignees

Comments

@mike-thompson-day8
Copy link
Contributor

mike-thompson-day8 commented Nov 3, 2024

Something basic like:

(rf/reg-fx
  :promise
  (fn [{:keys [promise on-success on-failure]}]
    (-> promise
        (.then #(rf/dispatch (conj on-success %)))
        (.catch #(rf/dispatch (conj on-failure %))))))

Example Usage:

(rf/reg-event-fx
  ::fetch-data
  (fn [_ [_ params]]
    {:promise {:promise (js/fetch "https://api.example.com/data")
              :on-success [::fetch-success]
              :on-failure [::fetch-error]}}))
              
;; Success handler
(rf/reg-event-db
  ::fetch-success
  (fn [db [_ result]]
    (assoc db :data result)))

;; Error handler  
(rf/reg-event-db
  ::fetch-error
  (fn [db [_ error]]
    (assoc db :error error)))
@souenzzo
Copy link

souenzzo commented Nov 4, 2024

Maybe also add a :on-finally or :always handler?

@jurjanpaul
Copy link

First impression: to me this looks a bit awkward compared to other re-frame syntax (overloaded :promise keyword - for which I don't have a better suggestion either - and why not keep closer to the underlying mechanism by having :then and :catch instead of :on-success and :on-failure?)

And isn't a call to js/fetch a side effect already?

Could you point to a discussion of the problem that this solves?

@jurjanpaul
Copy link

I wonder if passing a function that returns a JS promise would mitigate things:

(rf/reg-event-fx
  ::fetch-data
  (fn [_ [_ params]]
    {:promise {:fn #(js/fetch "https://api.example.com/data")
               :then [::fetch-success]
               :catch [::fetch-error]}}))

@stevebuik
Copy link

Seems like nice sugar.

We do a lot of re-frame testing using jvm i.e. cljc

I presume this would not have any adverse affects on the portability?

@green-coder
Copy link
Contributor

green-coder commented Nov 6, 2024

Something slightly different I would love Re-frame to support, is promises in the effects and waiting for them in this way:

(rf/reg-fx :fetch
  (fn [{:keys [url]}]
    (js/fetch url)))

(rf/reg-event-fx ::fetch-data
  (fn [_ [_ params]]
    {:fx [[:promise {:fx [:fetch {:url "https://api.example.com/data"}]
                     :then ,,,
                     :catch ,,,}]]}))

@migalmoreno
Copy link
Contributor

I've been using https://github.com/smogg/re-promise as a promises effect handler and it works just fine

@souenzzo
Copy link

souenzzo commented Dec 3, 2024

I'd add two more features:

  • Support to delay values (will only start the fetch inside the reg-fx call, as re-promise do.
  • Support literal values
(rf/reg-fx
  :promise
  (fn [{:keys [promise on-success on-failure]}]
    (-> promise
      force
      (js/Promise.resolve)
      (.then ...)
      ...)))

;; all supported:
:promise (js/fetch "https://api.example.com/data")
:promise (delay (js/fetch "https://api.example.com/data"))
:promise {:literal :value}
:promise (delay {:literal :value})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants