RFC: Action/Thunks listeners

[ctrlplusb]

Hey all;

The previous RFC process I opened for the computed property was extremely useful - I received a lot of quality feedback which helped guide me to the best API. Therefore I'd like to use the same process for an adjusted action/thunk listener API.

The current action/thunk listeners API (configured via the listenTo config) has a few shortcomings, namely:

• Error payloads are not exposed when listening to failed thunk events
• If you are listening to multiple actions there is no way to distinguish which action is being responded to.
• The mechanism by which we listen to specific stages of a thunk does not feel as fluid as the rest of the API
• Can't listen to a "finally" stage of thunks (i.e. failed or successful)

A lot of these concerns have been raised very helpfully by @allpro - which is highly appreciated.

Prior to us going to v3 (with an opportunity to introduce some small breaking changes), I definitely would like us to address these shortcomings. Either enhancing the existing API, or providing an alternative richer one.

Related #233 #244 #237

I am going to be laying out a few proposals based on some experiments I have made, taking into account the above related issues.

[ctrlplusb]

@allpro @jamaljsr

[jamaljsr]

Hey @ctrlplusb,

I really like this proposal. I agree with the direction your going in and also think this would be easier to grok than the listenTo config option. I have just a couple pieces of feedback to contribute.

1. Nitpick: why listenerAction vs actionListener?
   • I feel like listenerAction reads a bit awkward in a sentence. Ex: In response to a bug report: "To solve your problem, I suggest you add a listenerAction to your model which adds a message to your log array". I think it would read better as actionListener and thunkListener.
   • In addition to this, I feel like we (easy-peasy consumers) can have a better mental model of the action/thunk/listener functions and their callback signatures. It's easier to remember that all actions have the (state, payload) => {...} signature, while thunks have (state, actions) => {...} and listeners have (state, target) => {...}. As a newcomer to the library, I often mixed these signatures up and had to refer back to the docs.
2. I believe a common use case for thunks, which I immediately came across, is to display a loader in the UI. Therefore, I need to listen to all of the thunk stage types. Would the below code be the way to accomplish this using the new API, now that we know which stage triggered the listener via target.type?

   const storeModel = {
     todos: {
       items: [],
       status: {
         loading: false,
         error: null
       },
       addTodo: action(() => ...),
       saveTodo: thunk(() => ...),
       saveListener: listenerAction(
         (state, target) => {
           const { startedType, succeededType, failedType } = storeModel.saveTodo;
           switch (target.type) {
             case startedType:
               state.status.loading = true;
               break;
             case succeededType:
               state.status.error = target.error;
               // intentional "break;" missing to continue below
             case failedType:
               state.status.loading = false;
               break;
           }
         }, 
         actions => [
           actions.saveTodo.startedType,
           actions.saveTodo.succeededType,
           actions.saveTodo.failedType,
         ]
       )
     }
   };
   I honestly feel like automatically storing some thunk stage metadata in state would be nice to include in the library somehow. I created a model helper function to add these listeners and metadata to my models and plan to create an issue with the suggestion and some code later, but that's going a bit off topic from this RFC, so I digress.

Overall, I think your suggested changes are an improvement to the library. Thanks for asking for our feedback.

[allpro]

RE naming: I think actionListener implies "listener OF an action", which is inaccurate. It's really "listener that IS an action". So while it sounds better, I think it's misleading.

However changing "listener" to "listenTo" makes it clearer:

• actionListenTo
• thunkListenTo

OR lose the word 'listen' in favor of something more natural-language, like:

• actionOn( action, on-event )
• thunkOn( thunk, on-event )
• actionWhen( action, when-event )
• thunkWhen( thunk, when-event )
• actionAfter( action, after-event )
• thunkAfter( thunk, after-event )

I like 'actionOn' / 'thunkOn'. It's concise and clear because "on" is a universal action-prefix - onChange, onClick, onEvent, etc.

If that isn't clear enough then a more verbose option is:

• actionOnEvent( action, on-event )
• thunkOnEvent( thunk, on-event )

[allpro]

There is still a gap in the listener params. There are actually 2 "payloads" it needs...

The first is the "action payload" - the input to original action. This is the only payload for an "action listener", and may contain important identifiers/metadata for a "thunk listener" as well.

The second is the "response payload" - the data returned to a thunk, like an API response.

Since an error IS a "response", I'd follow the FSA concept of error-as-payload, with a boolean error flag. This aligns target with the FSA action syntax everyone already knows.

One way to provide both payloads is to add a "response" key for the response-payload. The "payload" key is the original payload. So both action and thunk listeners have target.payload, but only thunk listeners have target.response.

Putting this all together...

// Action payload
target = {
    name: "...",
    type: "...",
    payload: [ original ]
}

// Successful thunk response
target = {
    name: "...",
    type: "...",
    payload: [ original ],
    response: { data-object }
}

// Failed thunk response
target = {
    name: "...",
    type: "...",
    payload: [ original ],
    response: { error-object },
    error: true
}

[allpro]

The "succeededType" name bugs me. ;) It's hard to read and spell! Normally I refer to "success" and "fail" handlers; never a "succeeded handler". So how about these more succinct names...

• startType
• successType
• failType

If you want to keep them matching the string versions, then just change the "success" type, including the new string: @thunk.doSomething(success), so...

• startedType
• successType
• failedType

[allpro]

It would be convenient to also have a "thunk.settledType" for brevity, but it's not important. Could add it at any time.

[ctrlplusb]

Amazing feedback both - really appreciate you taking the time to do so.

---

Naming

I agree with @allpro in that actionListener and thunkListener could be misleading, but I completely appreciate where @jamaljsr is coming from - listenerAction doesn't exactly roll off the tongue.

I like some of your suggestions @allpro, my favourites being:

• actionOn(handler, targets)
• thunkOn(handler, targets)

What do you think @jamaljsr ?

---

UI Loading states

I appreciate you raising this as a common concern @jamaljsr - whilst it may be useful to expose some additional meta data I would prefer we leave this level of a problem in user land for now. Like you state, a helper could fairly easily be written to encapsulate this behaviour.

One issue I can see though - the thunk/action type properties are on available on the action/thunk creators (i.e. the functions used to dispatch), not on the model definitions.

Therefore your following code would not work:

    saveListener: listenerAction(
      (state, target) => {
        // you are referencing the model definition, not the resolved action creator
        //                                                               👇
        const { startedType, succeededType, failedType } = storeModel.saveTodo;

There is an issue here for us. I can't predefine these types on an action/thunk definition as I won't know it's fully qualified path until it is composed into a full store model and then passed into createStore.

I have been trying to think of a mechanism around this that isn't overly complex. One option I could think of is returning the resolved target types as a 3rd parameter to actions/thunks.

    saveListener: listenerAction(
      // Resolved target types as an array with matching array index
      //                  👇
      (state, target, { targets }) => {
        const [startedType, succeededType, failedType] = targets;
        switch (target.type) {
          case startedType:
            state.status.loading = true;
            break;
          case succeededType:
            state.status.error = target.error;
            // intentional "break;" missing to continue below
          case failedType:
            state.status.loading = false;
            break;
        }
      }, 
      // 👇 the resolved types of targets will be provided to the handler
      actions => [
        actions.saveTodo.startedType,
        actions.saveTodo.succeededType,
        actions.saveTodo.failedType,
      ]
    )

The same could apply to a listenerThunk.

    saveListener: listenerThunk(
      // Resolved target types as an array with matching array index
      //                  👇
      (actions, target, { targets }) => {
        const [startedType, succeededType, failedType] = targets;
		// ...
      }, 
      // 👇 the resolved types of targets will be provided to the handler
      actions => [
        actions.saveTodo.startedType,
        actions.saveTodo.succeededType,
        actions.saveTodo.failedType,
      ]
    )

---

Thunk Stage Type Names

@allpro I like your concise version. I have mistyped succeeded many times already. Happy to go with the following then.

• startType
• successType
• failType

---

Thunk Results "Payload"

@allpro another great point. You are on fire. Yep, I can understand how this might be useful. I am happy to consider extending the target argument to now contain the following:

const {
  name, // Short type name
  type, // Fully qualified type
  payload, // Actual payload provided to target
  result, // ONLY used when listening to thunks. Will contain anything returned by a thunk.
  error, // ONLY used when listening to thunks. Will contain the error if they failed.
} = target;

I don't really see the need to wrap the payload in an array, or equally to wrap the response. I personally prefer the more explicit result/error properties rather than having mixed responsibilities for a single property.

[ludwigbacklund]

Great stuff! I just have a quick question that might have been discussed before. What is the reasoning behind the order of the arguments for actionOn and thunkOn? Can listeners exist without target resolvers (i.e only the first argument is passed)? Otherwise it sounds more natural, in my head, having the target resolver come first and the handler second. (Also you are still using listenerAction at the bottom of your revised proposal 🙂 )

[ctrlplusb]

Thanks @ludwigbacklund - I really like the suggestion of putting the target resolvers first in the arguments. That reads much nicer. 👍👍

Yeah they can't exist without a resolver so this makes great sense.

[allpro]

const {
  name, // Short type name
  type, // Fully qualified type
  payload, // Actual payload provided to target
  result, // ONLY used when listening to thunks. Will contain anything returned by a thunk.
  error, // ONLY used when listening to thunks. Will contain the error if they failed.
} = target;

I don't really see the need to wrap the payload in an array, or equally to wrap the response. I personally prefer the more explicit result/error properties rather than having mixed responsibilities for a single property.

For clarity, I was not suggesting to "wrapping the payload in an array". I used square bracket notation to mock the payload value because it can be anything, like a simple ID. I didn't want to imply it had to be an 'object'.

If you are going to put errors in an error key, it would be good to document that a 'target' can have ONLY a "response" OR an "error" key - never both. This is because the error is the response! This may help clarify things since this differs from regular action syntax.

To clarify the either/or nature, here's a common async pattern.

function fetchSomething(payload) {
    return api.getSomething(payload);
}

thunk(async getSomething() {
    let resp = null
    try {
        resp = await fetchSomething({ id: 1234 })
    }
    catch(error) {
        resp = error
    }
})

Note that fetchSomething() can only return one 'response': either a data-response from a successful request or an error-response like a 503 (server-generated) or a timeout (client-generated).

[allpro]

I updated feature-request #241 because it would impact the listener target object by adding another key, so I wanted to plant the idea now while this is still in flux. For reference, below are the param/keys changes such a request would entail. See the comment in #241 for why I'm requesting it.

// Action params
action(state, payload, meta)
thunk(actions, payload, meta)

// Dispatch action - FSA format
dispatch({
    type: '...',       // Required
    payload: { ... },  // If exists
    meta: { ... }      // If exists
})

// Sample thunk listener, with all possible params
actionListenTo(state, target - {
    type: '...',
    name: '...',
    payload: { ... },
    meta: { ... },
    response: { ... },
    error: ?
})

[jamaljsr]

Naming
I agree that the actionOn and thunkOn names are better and also to swap the arguments as @ludwigbacklund suggested. It reads a lot better with these changes.

UI Loading states
I like the approach you outlined above. This would definitely solve my problem. It does feel a tad bit out of place having the targets get passed in as the third parameter. What do you think about putting this array on the target parameter alongside the type key which will always be one of the items in this array? You could name the key resolvers since it will always map to the targetResolvers array?

const auditModel = {
  logs: [],
  onCriticalAction: actionOn(
    // targetTypes contained within 3rd argument (to thunk or actions listeners)
    //                    👇 
	(state, target) => {
      // Our targetTypes matches the array index of our targetResolver function
      //      👇 0       👇 1        👇 2         
      const [loggedIn, loggedOut, addedTodo] = target.resolvers;
      switch (target.type) {
        case loggedIn: \\...
        case loggedOut: \\...
        case addedTodo: \\...
      }
	},
    (actions, storeActions) => [
      storeActions.session.loggedIn,  // 👈 0
      storeActions.session.loggedOut, // 👈 1
      storeActions.todos.addedTodo,   // 👈 2
    ]
  )
}

Everything else looks great! Thanks for taking our input 👍

[allpro]

It does feel a tad bit out of place having the targets get passed in as the third parameter. What do you think about putting this array on the target parameter alongside the type key which will always be one of the items in this array? You could name the key resolvers since it will always map to the targetResolvers array?

I agree with @jamaljsr that a third param seems unnecessary since you already have a catch-all target object.

The key "resolvers" seems logical, but here are some other ideas for variety:

• events
• listeners
• triggers

[ctrlplusb]

Wow, this has been so great. I am truly grateful for all this super valuable feedback and suggestions everyone. It feels good to be making these decisions more community driven. ❤️

Here is the updated proposal based on all your feedback.

Proposal v3

This proposal introduces two new listener APIs, namely; actionOn and thunkOn. This allows us to have APIs with specific behaviour and typing information related to listeners.

In addition to these new APIs the semantics of the actions that fire to represent each stage of a thunk have changed. These changes are aimed at satisfying the common use-case, whilst making them more convenient.

This proposal deprecates the listenTo configuration value that is currently available on action and thunk, however, it provides all the same capabilities as the deprecated APIs. Therefore migration should not be painful.

We will briefly introduce the two new listener APIs, with an example of each. Thereafter we will review the concepts and configuration that are shared between them.

---

APIs

The actionOn and thunkOn APIs have similarities to the action and thunk APIs respectively, however, there are some important distinctions;

1. You firstly need to define a targetResolver function as the first argument to your listener definitions, which will receive the store actions and is responsible for resolving the target(s) to listen to.
2. The handler for the action/thunk listener will receive a target argument instead of a payload argument. This target argument is an object containing a lot of useful information about the target being handled, including the payload.

actionOn

A listener action is responsible for performing updates to state in response to configured targets being executed.

onAddTodo: actionOn(
  // targetResolver:
  actions => actions.addTodo,
  // handler:
  (state, target) => {
    state.auditLog.push(`Added a todo: ${target.payload}`);
  }
)

thunkOn

A listener thunk is responsible for performing side effects (e.g. a call to an HTTP endpoint) in response to configured targets being executed.

onAddTodo: thunkOn(
  // targetResolver:
  actions => actions.addTodo,
  // handler:
  async (actions, target) => {
    await auditService.add(`Added a todo: ${target.payload}`);
  }
)

targetResolver

The first argument you provide to a listener definition is the targetResolver function. This function will receive the following arguments:

• actions (Object)

  The local actions relating to where your listener is bound on your model.

• storeActions (Object)

  All of the actions for the entire store.

The function should then resolve one of the following:

• An action

  actions => actions.addTodo

• A thunk

  actions => actions.saveTodo

• The explicit string type of the action to listen to

  actions => 'ROUTE_CHANGED'

• An array of actions/thunks/strings

  actions => [
    actions.saveTodo,
    'ROUTE_CHANGED'
  ]

target Argument

The target argument that a listener handler receives contains the following properties:

• type (string)

  The type of the target action being responded to. e.g. "@actions.todos.addTodo"

• payload (any)

  This will contain the same payload of the target action being responded to.

• result (any | null)

  When listening to a thunk, if the thunk succeeded and returned a result, the result will be contained within this property.

• error (Error | null)

  When listening to a thunk, if the thunk failed, this property will contain the Error.

• resolvedTargets (Array<string>)

  An array containing a list of the resolved targets, resolved by the targetResolver function. This aids in performing target based logic within a listener handler.

---

Listening to thunks

In the examples above the listeners were configured to listen to an action.

You can just as easily target a thunk.

onAddTodo: actionOn(
  actions => actions.saveTodo, // 👈 targeting a thunk
  (state, target) => {
    if (target.error) {
      state.auditLog.push(`Failed to save a todo: ${target.error.message}`); 
    } else {
      state.auditLog.push(`Saved a todo: ${target.payload}`);
    }
  }
)

When you listen to a thunk your listener will be fired any time the target thunk succeeds or fails.

If the thunk failed the error property of the target argument will be populated.

If the thunk succeeded and returns a value, then the result property of the target argument will be populated with the returned value.

---

Listening to multiple actions

A listener can have multiple targets. To aid having logic specific to each target type, both the actionOn and thunkOn handlers will receive a resolvedTargets array contained within the target object.

The resolvedTargets array contains the list of the action types resolved by the targetResolver function you provided to the listener. This array will match the index order of the types resolved by the targetResolver.

const auditModel = {
  logs: [],
  onCriticalAction: actionOn( // Resolved targets, by array index
    (actions, storeActions) => [ //         👇
      storeActions.session.loggedIn,  // 👈 0 
      storeActions.session.loggedOut, // 👈 1
      storeActions.todos.addedTodo,   // 👈 2
    ],
    (state, target) => {
      // The target argument will additionally contain a "resolvedTargets" 
      // property, being an array containing all the types of the resolved 
      // targets. This allows you to easily pull out these type references and
      // then perform target based logic against them.
      // Note how the array index of each type matches the array index as 
      // defined in the targetResolver function above.
      //       👇 0      👇 1       👇 2
      const [loggedIn, loggedOut, addedTodo] = target.resolvedTargets;

      // The target current being handled
      //              👇
      switch (target.type) {
        case loggedIn: \\...
        case loggedOut: \\...
        case addedTodo: \\...
      }
    }
  )
}

---

Action Type / Name Breaking Changes

Currently we export the following helpers allowing you to get the action types:

• actionName
• thunkStartName
• thunkCompleteName
• thunkFailName

Instead of having these helpers we will extend the actions/thunks to contain action type identifiers directly on them.

For example, given the following store model;

const storeModel = {
  todos: {
    items: [],
    addTodo: action(() => ...),
    saveTodo: thunk(() => ...)
  }
};

const store = createStore(storeModel);

We can access the type of each action/thunk like so;

const actions = store.getActions();

// actions:
console.log(actions.todos.addTodo.type); // @action.todos.addTodo

// thunks:
console.log(actions.todos.saveTodo.type); // @thunk.todos.saveTodo

// You can also access the type information for specific stages of a thunk:
console.log(actions.todos.saveTodo.startType); // @thunk.todos.saveTodo(start)
console.log(actions.todos.saveTodo.successType); // @thunk.todos.saveTodo(success)
console.log(actions.todos.saveTodo.failType); // @thunk.todos.saveTodo(fail)

A note one the different types attached to thunks:

• thunk.startType

  Represents an action that is fired when the thunk has started

• thunk.successType

  Represents an action that is fired when the thunk has succeeded (i.e. no errors)

• thunk.failType

  Represents an action that is fired when the thunk has failed

• thunk.type

  Represents an action that is fired when the thunk has completed (failed or succeeded)

Using this refactoring you can configure your listeners to target a specific action stage of a thunk, for example;

onAddTodo: actionOn(
  actions => actions.saveTodo.successType, // 👈 only targetting thunks that succeed
  (state, target) => {
    state.auditLog.push(`Successfully saved a todo: ${target.payload}`);
  }
)

---

@allpro - don't worry, I am not overlooking your concerns/requests about meta data. I am pondering this separately and will put forward some proposals for this. 👍

[ctrlplusb]

@allpro @jamaljsr @ludwigbacklund

[jamaljsr]

Looks solid to me! 👍

A couple typos I see:

The second first argument you provide to a listener definition is the targetResolver function. This function will receive the following arguments:

The targetTypes resolvedTargets array contains the list of the action types resolved by the targetResolver function you provided to the listener. This array will match the index order of the types resolved by the targetResolver.

[ctrlplusb]

Awesome, thanks @jamaljsr - I've fixed those typos too. 👍

[ctrlplusb]

Ok, I've built out this RFC, complete with tests and Typescript definition updates.

I have yet to update the website documentation, but will do so.

I am going to get this merged into the v3 branch to open up opportunities for early adopters to battle test it.

We'll leave v3 in beta for a little while until the dust settles on all the changes. I feel like we are getting really close to a robust API though.

[ctrlplusb]

Once again thank you all. And please feel free to continue to criticise v3. I'd like us to get to a truly happy place with the API and then lock it down for long term support, avoiding breaking changes at all costs.

[allpro]

🎉 Great work Sean. It's damn hard to fit so many features into a simple, intuitive API.

I hope to soon start training 25 developers, in 3 countries, to use Easy-Peasy, and phase-out Saga. So it's great news that the API is stabilizing.

[ctrlplusb]

Awesome, thats great to hear @allpro.

I am really appreciative of your constructive criticisms and recommendations that allowed us to evolve and simply the API even further. 👍

The website docs, including the tutorials are busy getting massive overhauls on them. They should be a lot more useful as a tool in teaching Easy Peasy as they will follow a structure where you are encouraged to refactor an existing application as you learn each of the APIs. I'm hoping they may form a platform with which to base a video tutorial series on too. 🤞

[ctrlplusb]

Fully documented within v3 website now