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

callback functions for steps #31

Open
ElChache opened this issue Mar 2, 2019 · 18 comments
Open

callback functions for steps #31

ElChache opened this issue Mar 2, 2019 · 18 comments
Labels
enhancement New feature or request

Comments

@ElChache
Copy link
Collaborator

ElChache commented Mar 2, 2019

I would like (can't wait to!) use Fonda on the frontend with re-frame. I'd like to define a sequence of steps running asynchronous functions that trigger events as they fail/succed.

I propose to add the following options to each step:

  • :on-start Called before the processor is executed
  • :on-success Called after the processor executed successfully
  • :on-error Called after the processor failed (by anomalies or exceptions)
  • :on-complete Called after the processor was executed, either failing or succeeding.

The value for each of the options would be a function that gets the result returned by the processor function.

@ElChache
Copy link
Collaborator Author

ElChache commented Mar 6, 2019

This is an example of a way I'd like to use fonda on the frontend

(defn init-fonda
  [{:keys [on-fetch
           set-application-data
           init-widgets]}]
  {:steps
   [{:name        :fetch-application-data!
     :path        [:user-data]
     :on-complete on-fetch}

    {:path        [:user-data-indexed]
     :fn          #(impl/index-user-data-by-id (:user-data %))
     :on-complete set-application-data}

    {:path       [:active-widgets]
     :fn         #(get-in % [:user-data-indexed :dashboard-widgets])
     :on-success init-widgets}]

   :catalog
   {:fetch-application-data! fx/fetch-application-data!}

   :ctx
   {}})

@arichiardi
Copy link
Owner

You can probably use tap there at the end of a step in order to achieve the :on-complete.

I wonder if you have enough info in the tap or the error callback for you needs though

@ElChache
Copy link
Collaborator Author

ElChache commented Apr 5, 2019

After implementing the injector step I think there is no way to implement this feature with taps. Or at least I can't think on any way.

To implement it with taps, a function should be implemented that traverses the steps and adds taps after each step that provides a :on-* callback.

But since the steps in a injector step are dynamically injected by fonda, I can't think on a way to also traverse those steps and add the taps for them, without modifying fonda itself.

Adding the :on-* natively wouldn't add much complication to the library and can be implemented with few lines of code. And of course they would be optional.

I propose to implement them natively so fonda could be used on the frontend with libraries like re-frame

@ElChache ElChache added the enhancement New feature or request label Apr 5, 2019
@arichiardi
Copy link
Owner

I am not clear or why you'd like to inject the callback though ..are these steps dynamic?

In your above example the function :on-* are static and that is why I was thinking of tap.

@arichiardi
Copy link
Owner

I agree the change is not that big, I just wonder if we can use tap which is already there and had that use case in mind

@ElChache
Copy link
Collaborator Author

ElChache commented Apr 5, 2019

On the following example:

[{:path      [:steps]
  :processor (fn [_] ...)
  :on-success (fn [] ...)}
 {:inject (fn [_]
            {:path      [:steps]
             :processor (fn [_] ...)
             :on-success (fn [] ...)})
  :name   "injector1"}
 {:path      [:steps]
  :name      "processor2"
  :processor (fn [_] ...)
  :on-success (fn [] ...)}]

where the injected step wants to use a callback function as well... I wouldn't know how to implement that with taps

@arichiardi
Copy link
Owner

arichiardi commented Apr 5, 2019

So in one of the our functions I have to do the same and what I do is to inject a [processor, tap]

@ElChache
Copy link
Collaborator Author

ElChache commented Apr 5, 2019

I think the problem is that there is no possibility to add a tap for errors... all the errors logic would be in a error-handler

@arichiardi
Copy link
Owner

Yep but I would open a different issue for that one, the tap solution is only for successful completion

@arichiardi
Copy link
Owner

Or we can keep talking here but to me these two are separate use cases

@ElChache
Copy link
Collaborator Author

ElChache commented Apr 5, 2019

implementing the callbacks on the steps allows me to do :on-complete callbacks like this

{:path        [:user-data]
  :name        :fetch-application-data!
  :on-complete on-fetch}

where on-fetch is a function that dispatches ::on-fetch, which removes/adds the error on db (if fetch application-data returns anomaly, sets error, otherwise, removes)

but maybe what I am trying to do is different enough that deserves it's own "fonda for frontend" library

@ElChache
Copy link
Collaborator Author

ElChache commented Apr 5, 2019

at the end, it's a very specific case for frontend management of errors. never mind if you think it doesn't make sense on the actual fonda

@arichiardi
Copy link
Owner

As you know I am trying to keep the number of callbacks to a minimum.

In particular semantically a tap after a successful step is a callback of sorts so I guess we are covered already there.

It seems you have both success and error logic in one function though...maybe...can you show me the implementation of that callback?

@arichiardi
Copy link
Owner

And thanks for writing a tangible use case down, it really is helpful!

@ElChache
Copy link
Collaborator Author

ElChache commented Apr 5, 2019

no prob! I have a generic function that sets/removes errors:

(defn update-error
  ([error-key]
   (fn [db [res]]
     (if (a/anomaly? res)
       (assoc db error-key res)
       (dissoc db error-key)))))

Then I use it like this:

(re/reg-event-db
  ::on-fetch-application-data mw
  (re-helpers/update-error
    :fetch-application-data-error))

@ElChache
Copy link
Collaborator Author

ElChache commented Apr 5, 2019

but you know, I might be over-engineering things lol

@arichiardi
Copy link
Owner

arichiardi commented Apr 5, 2019

Ah ah, ok that is what I was missing, soooo...given that in fp land we don't like ifs AND we are basically doing that if in Fonda to understand if to error out, my humble opinion is that it should not be duplicated in your library and we should provide a better API for your use case.

Tap is for success, and maybe we should have an error step for anomalies/errors.

All the ifs should be baked in fonda

@ElChache
Copy link
Collaborator Author

ElChache commented Apr 5, 2019

yes yes agree that would be pretty, to handle all in fonda... I'll do some brainstorming to what the api for error taps could be...I'm out of ideas right now because we don't want to break the principle that errors short-circuit the steps flow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants