Hyperapp: Remove update & thunks.

Created on 25 Oct 2017  ยท  42Comments  ยท  Source: jorgebucaran/hyperapp

Obviously this issue needs more info, but the core idea is to remove the concept of update and thunks unless we can demonstrate that using only actions is not enough to do everything that can be accomplished with actions _and_ thunks.

I'll update this issue with a bit of history about why thunks were introduced and why I think they are dispensable now that events and mixins are not part of our architecture.

An counter-alternative to this proposal: https://github.com/hyperapp/hyperapp/issues/433

EDIT: This is just a proposal and it may not happen! A different proposal may come out of this or nothing at all. We are just brooding together! โ˜๏ธ๐Ÿค“โค๏ธ

Community Discussion

Most helpful comment

@JorgeBucaran I think the "computed property" getWorth() example without thunks is more logical. Using an action w/ thunk for that is definitely overkill, but creative.

The example where you have action(state, { update }) also makes good sense, but does require us to write our own update() actions at every level of the state/actions tree that we need them. Probably not a big deal, but it is more boilerplate. I currently only use the thunks for Promises after you removed the resolve hook.

I look forward to reading more about why this issue was raised.

All 42 comments

The two things I find thunks good for are:

A) Keeping the list of actions neatly limited to just actions that will be called from the UI (less need for "utility actions" that will only be called from other actions). However, I think making a generic update action in each module is a quite acceptable alternative.

B) Ability to get values computed from the current state/actions in the view. Like:

actions: {
  calcSum: state => _ => state.a + state.b
}

This is not a good enough reason to keep thunks, but I would really like to see some replacement-way of creating getters -- if not through thunks, then some other way -- as it can be very convenient at times (and is not "cheating" with the purity IMO)


Those are mostly aesthetics / convenience arguments for keeping thunks. So if there's a significant simplification to be won in hyperapp's source code in removing thunks, then I think they should go.

From the Slack.

With thunks

(state, actions) => update => {
  update({ fetching: true })
  fetch(...).then(data => update({
    data, fetching: false
  }))
}

Without thunks!

(state, { update }) => {
  update({ fetching: true })
  fetch(...).then(data => update({
    data, fetching: false
  }))
}

/cc @Swizz @lukejacksonn

Getters are another typical use case of thunks.

Say you are writing the next generation Warcraft-spinoff in Hyperapp, and your orcs module needs to compute some values based on your state slice.

With thunks as getters

// orcs.js

export default {
  state { ... },
  actions: {
    getWorth(state) {
      return () => state.gems * state.gold * state.oil
    },
    attackHumans(state, actions) {
      // ...
      const worth = actions.getWorth()
      // ...
    }
  }
}

Without thunks!

```js
// orcs.js

function getWorth({ gems, gold, oil }) {
return gems * gold * oil
}

export default {
state { ... },
actions: {
attackHumans(state, actions) {
// ...
const worth = getWorth(state)
// ...
}
}
}

Would it be an option to add getters to modules/apps?
This way a module can export it's getters along it's state/actions:

// orcs.js

export default {
  state { ... },
  getters: {
    getWorth(state, getters, data) {
      // data is unused in this example.
      return state.gems * state.gold * state.oil
    }
  },
  actions: {
    attackHumans(state, getters, actions) {
      // ...
      const worth = getters.getWorth()
      // ...
    }
  }
}

The drawback is that the API gets bigger for not much added value...

@Mytrill getters were discussed on the slack (with me championing them), but basically, like you said it's more complexity for not that much more convenience. So probably not happening.

Other question: What to return when we don't want to update the state? undefined?

@Mytrill Return nothing. Feature since day one. ๐Ÿ˜‰

Another possible place to put getters would be directly in the state:

const state = {
  todos: [],
  getTodos(state, { completed, toDo }) {
    return state.todos.filter(todo => (completed && todo.done) || (toDo &&!todo.done))
  }
}

The API would stay as simple (or almost...) but HA's code would be more complex.

PS: I am not trying to push for getters, just offering ideas.

@JorgeBucaran I think the "computed property" getWorth() example without thunks is more logical. Using an action w/ thunk for that is definitely overkill, but creative.

The example where you have action(state, { update }) also makes good sense, but does require us to write our own update() actions at every level of the state/actions tree that we need them. Probably not a big deal, but it is more boilerplate. I currently only use the thunks for Promises after you removed the resolve hook.

I look forward to reading more about why this issue was raised.

@SkaterDad Write once, use everywhere. The definition of getState and update/setState can be shared across modules, but you do have to put it there. In that case, we could populate your actions with both setState and getState out of the box.

@SkaterDad On the other hand, you may not always use them, since you can just call an action for that. There is really little point to a setState action when you can call an action with a descriptive & self-documenting name.

So, instead of setState({ fetching: true, spin: true }), why not just toggle().

Agreed that having a descriptive name for actions is nicer, and it makes it more obvious which "scoped" part of the state you're updating.

Without thunks!

(state, { update }) => {
  update({ fetching: true })
  fetch(...).then(data => update({
    data, fetching: false
  }))
}

In the above example, is update still a special Hyperapp's function or is it part of actions that we need to implement? It it's latter, can you show the custom implementation of update that the user needs to do?

@rajaraodv The latter. It can be as complicated as you want, but this will do it.

update: (state, actions, newState) => newState

That is, of course, if you must have an update or setState function at all. You could, instead have actions named exactly as the thing they do and call those instead.

So, instead of setState({ fetching: true, spin: true }) โ†’ toggle().

Just when you start thinking there might not be anymore breaking changes before 1.0 ๐Ÿ˜‚

Without thunks AFAIK there will be 3 built-in options left for async in core, none of which I find terribly exciting:

  1. Write twice as many actions as normal, one to trigger the async stuff and another to update the state with the result. Double the boilerplate, double the fun!
  2. Users will avoid the boilerplate by writing their own state setter action. This leads to a very imperative style of development.
  3. Users will revolt and move the async code into the event listeners of their view code, only calling actions for updating the state with the results.

Ultimately I think this is a move in the wrong direction since it would encourage not returning a value from actions and be a move toward imperative programming and away from our functional programming roots (by relying almost entirely on side effects). I would personally much rather see a move towards the "effects as data" mantra of Elm, and I'm planning on implementing library support for this at some point down the road. This would greatly expedite the priority of writing that HOA for me.

To follow @okwolf I said on slack something similar to 1.

I dont want to create an actions for just toggling a fetch property in the state. And actions are namespaced so if I want to toggle a fetch property in different place I will need a toggleFetch in all the namespaces

In other words, I dont want to make an async action that depend to a/some sync actions to update the state.

I know, the only one point I have about the thunk is the ease of use.

Ultimately I think this is a move in the wrong direction since it would encourage not returning a value from actions.

But you do like calling actions from other actions. What's the alternative? Pass update to actions (instead) of the actions object? I've played with that too, but the changes are more drastic. Can you handle the truth? ๐Ÿบ ๐Ÿ˜

@JorgeBucaran the alternative would be to return a value from actions. A Promise would work, although I'd prefer something more like a POJO. The important point is that it's just data. All of the magic would be moved outside of app code by performing the effect this data describes and then calling an action to update the state with the results after the effect. This could either be a custom action to call, or the effect as data could come with a description of which state to update with the results.

@okwolf To make actions really pure, we'd first need to not share the actions object with actions! ๐Ÿ˜

@JorgeBucaran To make actions really pure, we'd first need to not share the actions object with actions! ๐Ÿ˜

I'd be ๐Ÿ‘Œ with that, if there were a value actions could return describing an effect of calling other actions.

@okwolf Please write a pseudo code example. Code is worth more than a 1000 words.

I'm happy to see this discussed. The moment I read Thunks in the docs, I thought, why not promises? It doesn't remove thunks but makes it more promise friendly, also forces hyperapp to think about rejected promises.

With promises!

(state, { fetching }) => {
  fetching(true)
  return fetch(...)
    .then((data) => {
      fetching(false)
      return {data}
    })
}

With await!

async (state, { fetching }) => {
  fetching(true)
  const data = await fetch(...)
  fetching(false)
  return {data}
}

Also wish we had finally, https://github.com/tc39/proposal-promise-finally

I believe I have a use-case that is much harder (or rather, dirtier) to achieve if we remove the ability for an action to return a value without updating the state: some of my actions return other actions that are dynamically wired to a specific slice of the state.

Explanations: my state is a binary tree where each node is very complex (lots of different properties) and may be of different type (contains different properties), therefore different nodes may have different actions that I would like to apply on them.
So what I have done is: I decoupled the logic that creates/gets/deletes node and the logic that actually modify nodes, here is a simplified snippet to illustrate:

// these are the actions that are relevant for nodes of type 1
// here, state should be a single node
const node1Actions = {
  setProp1: (state, actions, prop1) => ({ prop1 }),
  setProp2: (state, actions, prop2) => ({ prop2 }),
}
// these are the actions that are relevant for nodes of type 2
// here, state should be a single node
const node2Actions = {
  setProp3: (state, actions, prop3) => ({ prop3 }),
  setProp4: (state, actions, prop4) => ({ prop4 }),
}

// these are the "tree" actions, that create/get/delete nodes and return pre-wired actions
const treeActions = {
  createNode(state, actions, { path, type }) {
    return update => {
      // path is a dot separated string, set(state, path, node) recursively goes through the tree, adds the node at the given path and return the modified state
      update(set(state, path, { type }))

      return actions.getActionsForNode(path)
    }
  },
  getActionsForNode(state, actions, path) {
    return update => {
      // get the node at the given path
      const node = get(state, path)
      if(node.type === "type1") {
        // this returns initialized actions for the node at the given path:  { setProp1(prop1) {...}, setProp2(prop2) {...} }
        return initializeActions(update, path, node1Actions)
      } else {
        // this returns initialized actions for the node at the given path:  { setProp3(prop3) {...}, setProp4(prop4) {...} }
        return initializeActions(update, path, node2Actions)
      }
    }
  }
}

// later on, in my view I call like this: 
function view(state, actions) {
  actions.createNode({ path: "left.right.left", type: "type2" }).setProp2("blah")
}

This would of course be doable by extracting the createNode() and getActionsForNode() outside of Hyperapp (like Jorge demonstrated with the orcs.js getWorth() function) but then almost all my update logic would live completely outside of Hyperapp.

PS: this use case is the reason for #431
PS2: If anyone has a better way of doing this, please let me know! I'd be happy to do something less complicated/hard to debug...

In the Slack, one of the arguments to remove the thunks is that they add complexity to Hyperapp's code.
After tinkering with the immutable state for a while, I found that thunk themselves don't add much complexity.

However, here are the 2 features that adds complexity IMHO:

  • being able to call update() with a function of the state, this adds complexity because it forces update() to have a way to get the current state slice
  • making the update() function return the current state slice, again, this forces update() to have a way to get the current state slice

IMHO these 2 features are unnecessary: in those case we can just call/return other actions. So maybe we could start by removing those? Or at least discuss them in a separate issue?

IMHO these 2 features are unnecessary: in those case we can just call/return other actions. So maybe we could start by removing those? Or at least discuss them in a separate issue?

Good point! ๐Ÿ‘ ๐Ÿค”

But we can't just keep half thunks. Either we remove them or not.

But we can't just keep half thunks. Either we remove them or not.

What do you mean by half thunk? Do you find these 2 features to be significant?

Yes. If update does not return a state slice then what would it return? I am okay with nothing.

But it must take a function that takes the state, because, you know async.

Yes. If update does not return a state slice then what would it return? I am okay with nothing.

I agree with this.

But it must take a function that takes the state, because, you know async.

I disagree with this: I would assume that a lot of async actions are about getting some data from the server, or updating it. When getting back a reply I would think it can be safely merged in the current state. When it cannot, then it make sense to extract this "merging" logic into a different action and call it.

Note: I haven't used Hyperapp enough to actually have encountered these issue, if you (or someone else, more experimented) has often seen these problems, then I withdraw my comment.

Maybe I'm not using the current thunks to their fullest potential, but I've never passed a function into the update call. I just toss a partial state object into it and move on.

actions: {
  fetchData: (state, actions) => (update) =>
    axios.get('/endpoint')
      .then(res => update({ data: res.data }))
      .catch(err =>update({ error: parseError(err) })
}

If we can keep that functionality and reduce hyperapp core's complexity, it would be very convenient. But as stated before, it's not hard to replace with a little more boilerplate.

@SkaterDad this is what that would look like without thunks in core:

actions: {
  update: (state, actions, data) => data,
  fetchData: (state, { update }) => {
    fetch('/endpoint').then(res => res.json())
      .then(data => update({ data }))
      .catch(err => update({ error: parseError(err) })
  }
}

And if you need a fresh state then (I think):

actions: {
  append: (state, actions, data) => ({ data: state.data.concat(data) }),
  fetchData: (state, { append }) => {
    fetch('/endpoint').then(res => res.json())
      .then(data => append(data))
      .catch(err => append(parseError(err))
  }
}

[EDIT] \axiosfetch\s

@JorgeBucaran Please write a pseudo code example. Code is worth more than a 1000 words.

Sure, I don't have a full POC yet, but I was imagining for my _effects as data_ (โ„ข๏ธ) library that I would continue to use objects for state updates and add arrays for effects. So perhaps if we wanted to capture a simple fetch use case:

actions: {
  showFetching: (state) => ({
    fetching: true
  }),
  fetchedData: (state, data) => ({
    data, fetching: false
  }),
  fetch: (state, data) => [
    [
      "action",
      "showFetching"
    ],
    [
      "http",
      { url: "...", action: "fetchedData" }
    ]
  ]
}

This maps pretty closely to how the same code would be written in Elm. We can cut down on the extra actions if we want to allow each of the items in the returned array to perform updates:

actions: {
  fetch: (state, data) => [
    [
      "update",
      { fetching: true }
    ],
    [
      "http",
      {
        url: "...",
        response: "data",
        update: { fetching: false }
      }
    ]
  ]
}

Helper functions would be provided to cut down on the boilerplate of creating these effects:

actions: {
  fetch: (state, data) => [
    effect.update({ fetching: true }),
    effect.http({
      url: "...",
      response: "data",
      update: { fetching: false }
    }),
    effect.action("profit", { amount: 9001 })
  ]
}

that seems way more complicated than my promise/await pseudo code...

@Pyrolistical the point is that when everything is data then testing and debugging what actions are doing is simple because it's loggable/comparable. This follows closer to our functional programming roots from Elm.

@okwolf how is my example here hard to test (not a sarcy comment, a genuine question, I am no testing expert)?

And @Pyrolistical you can write an HOA to make hyperapp handle promises returned from actions. It has also been supported by core in the past through resolve. It has been proposed that we could Promise.resolve(action(state, data)).then(update) with the logic that primitive types would just pass through and promises only pass through once resolved; this was disregarded as introducing Promise into core was undesirable.

Promise is standard and thunks are not.

I am in favour in removing thunk and do plain actions, but I also hope we can agree the generic update action is an anti-pattern.

Promise is standard

Very close, yes http://caniuse.com/#feat=promises.

I am in favour in removing thunk and do plain actions

I think I am too now.

I also hope we can agree the generic update action is an anti-pattern

I am not sure I agree update: (state, actions, data) => data is a perfectly valid action.

update: (state, actions, data) => data is a valid action, but sorta by passes the value of hyperapp. ie. why even have actions when you can just rewrite all actions to the generic update one? having more domain specific action names would help readability of the code

@lukejacksonn you would need to mock/fake/stub/spy the axios API to test that your logic chained to its Promise is correct. You would need to do the same thing for the update action.

Using _effects as data_, you test by simply calling a pure action function with the test state object and an object of data and then assert that the correct effects are returned. This function is pure because it will always return the same value for the same input, guaranteed. ๐Ÿ‘

At this point actions are basically reducers from Redux, except that they can return new state or _effects as data_. I'm a fan of pushing the imperative parts of the code out into libraries and writing the app code as functional as possible.

@Pyrolistical I think that choice is up to the developer to make, you can brand it an anti-pattern but there is nothing technically wrong with it (especially not now state slices exist). For instance a mouse module might look like this..

export default {
  state: {},
  actions: {
    update: (_, __, data) => data
  },
  init: (_, { update }) => addEventListener('mousemove', update)
}

@okwolf oh sorry I didn't mean to have copy an axios demo.. I updated it to fetch but I guess you would have to stub that right? How does your effect.http look/work and how do you test it?

@lukejacksonn How does your effect.http look/work and how do you test it?

In my dummy example effect.http is just a helper function for creating an array representing an HTTP effect. It doesn't immediately send any network requests but instead describes what request _should_ be sent later, and what to do with the result. It might look something like:

effect.http = options => [
  "http",
  options
]

If you had a sample action that used it:

actions: {
  selectOrc: (state, orcId) => [
    effect.http({
      url: `https://api.backend.com/games/${state.gameId}/orcs/${orcId}`
      response: "currentOrc"
    })
  ]
}

You could test it thusly:

test("selecting an orc requests the data from the api", () => {
  const gameState = { gameId: 555 }
  expect(actions.selectOrc(gameState, 666)).toEqual([
    effect.http({
      url: `https://api.backend.com/games/555/orcs/666`
      response: "currentOrc"
    })
  ])
})

@okwolf What is http? How does hyperapp know what to do with "http"? And where does effect come from?

In this code:

actions: {
  fetch: (state, data) => [
    [
      "update",
      { fetching: true }
    ],
    [
      "http",
      {
        url: "...",
        response: "data",
        update: { fetching: false }
      }
    ]
  ]
}

Is http an action defined elsewhere that Hyperapp calls like so:

http({ url, response, update })

In that case, what is update?

@JorgeBucaran "http" would be a string that the HOA I haven't written yet understands as an effect when used in the return value from an action. It would do the dirty work of actually making a network request and then what the effect data describes with the response. effect.http would be a provided function as part of the imaginary library I haven't written a line of read code for yet. update is a string/object key I came up with on the spot as a shorthand for specifying which state to update within an effect if you have a one-off and don't want to write another action to call from the effect instead.

I'm not proposing adding anything related to http to core, but it might be cool to already have support for _actions as effects as data_ return the actions to fire instead of the actions parameter. This would however sacrifice some nifty static typed features like autocomplete on available actions, so I can understand not wanting to make the change.

I was thinking about that, while playing with few implementations. As separate export is good one too (like in freactal - back in the days there was hardUpdate and softUpdate, currently it seems like just update.

So it would look like

import { h, app, update } from 'hyperapp'

app({
  actions: {
    foo: (state, actions, { left }) => update({ left: left + 1 })
  }
})

But in any way, i'm seeing thunks as best way ever. Not worth removing them. Won't simplify code and they are not so hard "to learn" - hence, just return a function if you want to update the state, what's so hard?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ghost picture ghost  ยท  3Comments

dmitrykurmanov picture dmitrykurmanov  ยท  4Comments

jbrodriguez picture jbrodriguez  ยท  4Comments

dmitrykurmanov picture dmitrykurmanov  ยท  3Comments

zhaotoday picture zhaotoday  ยท  3Comments