Hyperapp: Combine the concept of state and actions into models 🤯

[jorgebucaran]

EDIT: @Alber70g suggested an even more extreme simplification:

Instead of app({ model, view }, root) → app(model, view, root), but we can discuss that on another issue.

We did it once and here we go again. Like everything, there are trade-offs involved, not to mention the pain that comes with the churn. There's also "done is better than perfect", but Hyperapp is not done yet, and we still have some time before 1.0. I need your help to evaluate if this idea holds or not, and identify possible drawbacks or disadvantages.

The proposal (via @Alber70g) is to combine the concepts of state and actions into a new abstraction: _models_. A model is a regular object that holds your state props and actions.

The typical counter would look like this:

app({
  model: {
    count: 0,
    down: () => model => ({ count: model.count - 1 }),
    up: () => model => ({ count: model.count + 1 })
  },
  view: model => (
    <main>
      <h1>{model.count}</h1>
      <button onclick={model.down}>–</button>
      <button onclick={model.up}>+</button>
    </main>
  )
})

An application that is wired to state and actions from multiple sources, could look like this:

app({
  model: {
    A,
    B,
    C,
    D
  }
})

Instead of the current:

app({
  state: {
    A: state.A,
    B: state.B,
    C: state.C,
    D: state.D
  },
  actions: {
    A: actions.A,
    B: actions.B,
    C: actions.C,
    D: actions.D
  }
})

And an app that subscribes to external events.

const { tick } = app({
  model: {
    time: Date.now(),
    tick: () => ({ time: Date.now() })
  },
  view: model => <Clock time={model.time} />
})

setInterval(tick, 1000)

You can see that the app() now returns the wired model, so you can access not only actions, but also state props.

Now, you can think of models like a "module" that contains your state and actions under the same namespace.

// A counter model
const counter = {
  count: 0,
  add: n => model => ({ count: model.count + n })
}

app({
  model: { counter },
  view: ({ counter }) => (
    <main>
      <h1>{counter.count}</h1>
      <button onclick={e => counter.add(1)}>Add</button>
    </main>
  )
})

Notice how usage of the model inside the view function mirrors how you define the model above.

Pros

• Simplify your code (save bytes!)
• Simplify core (save bytes!)

Cons

• Breaking change
• Complicate saving the state to local storage, as you will have to exclude actions from the model before you save:

saveToStorage(withoutActions(model))

Notes

• No impact on how slices work.

[vdsabev]

EDIT: Like @okwolf said, this will not actually work because of state slices 😞
And you can still have your state / actions if you really want to:

app({
  model: {
    state: {
      count: 0
    },
    actions: {
      add: (n) => ({ state }) => ({ count: state.count + n })
    }
  },
  view: ({ state, actions }) => (
    <main>
      <h1>{state.count}</h1>
      <button onclick={() => actions.add(1)}>Add</button>
    </main>
  )
});

[lukejacksonn]

I really like the idea of this and see it making _module_ code much more portable; encouraging sharing and reuse. Can't see many downsides, it feels like one of those, how did we not see this before moments 🤔

Would what @vdsabev suggests actually serve as a migration strategy (reducing that churn feeling) meaning that although it is a breaking change.. you wouldn't have to rewrite old apps.. just wrap your existing state/actions in model and destructure them in the view function?

[okwolf]

@vdsabev And you can still have your state / actions if you really want to

Wait - if we still have slices then wouldn't actions be its own slice and actions.* not have access to its aunt state slice? 🤔

[jorgebucaran]

@okwolf Good point, I totally missed it. What @vdsabev suggested is actually not correct. Not a problem for me though.

[SkaterDad]

Hmm.... the application code size wins are certainly appealing. Also a fan of getting rid of view: (state) => (actions) => h(.....) in favor of a simpler (model) => h(....)

It's a little messier this way, mixing state and actions, but with editors like VSCode doing their magic Intellisense, it's unlikely people will get too confused. This is the biggest sticking point for me, losing that clear separation.

You mention in the other issue that this enables dynamically creating actions? Will hyperapp's core have to inspect all return values for functions and "wire" them up?

[jasoncslaughter]

Does the model keep any sort of name spacing?

For example, with the following model:

model: {
     videos: {
          exampleAction: exampleActionCode,
          exampleState: exampleStateObject    
     },
     controls: {
          exampleAction2: exampleAction2Code,
          exampleState2: exampleState2Object
     }
}

Is the model passed to exampleAction going to be videos, or is it going to be model?

[Alantir]

@JorgeBucaran about serialization:

localStorage.model = JSON.stringify(model) will omit the functions (actions) by default.

Object.assign(model, JSON.parse(localStorage.model)) Will override the initial state values in model if they are saved, and leave everything else untouched. Works with deep nested objects.

[Swizz]

As I use storage a lot, the con, is really breaking for me. But helpers can be done in userland. So.. :thinking:

Will slices still being supported ?

[alber70g]

Thank you for putting it up here @JorgeBucaran. Thanks all for looking into this proposal.

@Swizz It's inspired because of the slicing. Each function in that specific sub-state/path will be provided with it's own slice of the state.

Another use case that's been simplified is "generic" components having their own state. Let's say you'd have a custom input component like this, and you want the underline to switch color based on the focus.

export const InputView = (props) => <div>
  <label>{props.label}
    <input type="text" 
      onfocus={props.focus} 
      onblur={props.blur} 
      onchange={e => props.onchange(e.target.value)} />
  </label>
  {props.focussed 
    ? <div class="underline-focus" />
    : <div class="underline" />}
</div>;

export const InputModel = {
  focussed: false,
  value: '',
  focus: _ => model => ({ focussed: true }),
  blur: _ => model => ({ focussed: false }),
  onchange: value => model => ({ value }),
}

Now you can use this in multiple places:

import { InputView, InputModel } from './input';
app(
  {
    model: { username: InputModel, password: InputModel },
    view: model => <div>
      <InputView label="Username" {...model.username} />
      <InputView label="password" {...model.password} />
    </div>
  },
  root
);

[selfup]

Hmm seems most would still be doing::

const model = {
  ...state,
  ...actions,
};

const {
  whatDoWeDoHere,
} = app(
  model,
  view,
  root,
);

console.log(whatDoWeDoHere); // all of the state and actions?

// or will we have to filter all functions out to then be sure we are only returning actions?
// this can add a good chunk of bytes but if people prefer the API it could be worth it

🤔

I dunno the current API provides some clear separation of concerns

We should still stick to some form of organic API

Otherwise we will need to start providing mapStateToModel and mapActionsToModel helpers

---

However from a point of being able to slice this way as @Alber70g explained:

export const InputModel = {
  focussed: false,
  value: '',
  focus: _ => model => ({ focussed: true }),
  blur: _ => model => ({ focussed: false }),
  onchange: value => model => ({ value }),
};

I can see how this _might confuse some_ thinking about stateful components even though it just binds to the global store.

But in terms of development/code review this is quite nice 👍

[jorgebucaran]

The problem I find with this approach is reading from and saving to local storage. Saving involves traversing the model tree and plucking all the functions, but reading is much more complicated and I don't have a simple solution.

An alternative before we call this off, without the problems of saving/loading to and from storage, but not as nice as the original proposal is to keep the same API when assembling your app:

app({
  state: {
    A: state.A,
    B: state.B,
    C: state.C,
    D: state.D
  },
  actions: {
    A: actions.A,
    B: actions.B,
    C: actions.C,
    D: actions.D
  }
})

But merge state and actions for you when we give it to you inside the view/actions. Allowing the following:

```js
view: model =>

Hello

...instead of the more verbose:

```js
view: state => actions =>
  <main>
    <h1>Hello</h1>
    <AComponent A={{ ...state.A, ...actions.A}} />
    <ZComponent A={{ ...state.A, ...actions.A}} B={{ ...state.B, ...actions.B}} />
  </main>

[vdsabev]

For a possible solution to restore state from localStorage or wherever, try the following:

1. Go to https://lodash.com/docs/4.17.4
2. Open the console
3. Enter:

_.merge(
  {
    A: {
      a: 1,
      b() {},
      C: {
        d: 2,
        e() {},
        F: {
          g: 3,
          h() {}
        }
      }
    }
  },
  {
    A: {
      a: 4,
      C: {
        d: 5,
        F: {
          g: 6
        }
      }
    }
  }
)

I think you'll find it merges the model just fine.

[jorgebucaran]

Thanks @vdsabev for the idea. 👏

The following example walks you through creating some models and wiring it all up inside index.js. The example also shows you how to read from and save to localStorage. The only part relevant to "salvaging" this proposal is how we use a deep merge strategy to combine the persisted state and the model.

This shows that it's possible to get away with models, but that doesn't mean we're going forward with the idea. From your feedback on this issue and slack, it seems some of you definitely like the idea, but a lot of people also appear to be indifferent, so maybe it's not as big as a deal as we thought! 🤔😄

// Let's start with our sample models...

//Z.js
export const Z = {
  z: 4,
  zf() {}
}

//B.js
import { Z } from "./Z"
export const B = {
  b: 0,
  bf() {},
  Z
}

//A.js
import { B } from "./B"
export const A = {
  a: 1,
  af() {},
  B
}

//X.js
export const X = {
  x: 5,
  xf() {}
}

// Okay, we've reached index.js, where loading/saving and wiring will happen.

//index.js
import { A } from "./A" 
import { X } from "./X" 

// A and X are models, they have all your stuff state/actions.

import { merge } "./merge"

// We need a deep merge(obj, src) function, like a recursive 
// Object.assign that deeply merges src into obj and returns the result.

// We could create a component that saves the model to localStorage.
const AutoSave = ({ model }) => localStorage.setItem("model", JSON.stringify(model))

// Here we read from localStorage and save the previously persisted state.
const persistedA = JSON.parse(localStorage.getItem("state"))
/* e.g.
{
  A: {
    a: 999,
    B: {
      b: 999,
      Z: {
        z: 999
      }
    }
  }
}
*/

const model = { A: merge(A, persistedA), X }
/*
{
  A: {
    a: 999,
    af(){},
    B: {
      b: 999,
      bf(){},
      Z: {
        z: 999,
        zf(){}
      }
    }
  }
}
*/

app({
  model,
  view: model =>
    <Autosave model={model}>...</Autosave>
})

[jorgebucaran]

This proposal is great; it solves two annoyances with current hyperapp (1) wiring and (2) passing modules as props down to components, but it also introduces a problem that requires a hack to circumvent (de-serialization) as well as other minor issues like @selfup mentioned here.

Someone also mentioned to me they also like the explicitness of separating state and actions during the wiring, and if that's too annoying, you can always create a wrap() or modules() function to support app().

I am interested in eliminating pain points, but not by introducing new problems.

[jorgebucaran]

@rexrhino Is the model passed to exampleAction going to be videos, or is it going to be model?

I missed your question. The answer is videos, not the global model, as you would expect from current hyperapp slice arch.

[jorgebucaran]

@selfup Hmm seems most would still be doing...

I missed this first too, but actually we wouldn't do that anymore. Instead we would export models.

export const boombox = {
  on: true,
  volume: 43,
  speakers: 2,
  switch: () => model => ({ on: !model.on }),
}

And about what app() returns, it would return the model with the wrapped actions and the initial state, which is what you would expect now that there are no state or actions.

this can add a good chunk of bytes but if people prefer the API it could be worth it

I thought we would save bytes, but probably not, I need to finish the implementation. I think not much more bytes if at all.

I dunno the current API provides some clear separation of concerns

State and actions are the same concern, I don't think we violate that principle here. Someone could even argue that we are misunderstanding the principle currently. 🤯

Otherwise we will need to start providing mapStateToModel and mapActionsToModel helpers

Hopefully not, probably not, most likely not if my understanding of the proposal is correct.

---

Having said all that, I am still not comfortable with how this creates problems when wiring the model after loading persisted state from local storage or another source.

[lukejacksonn]

I am still not comfortable with how this creates problems when wiring the model after loading persisted state from local storage or another source.

Has anyone else got a problem with this?

[jorgebucaran]

@lukejacksonn How would you solve the problem?

[lukejacksonn]

Let us just confirm what the problem is:

Merging the model passed to app (which includes mixed state/actions) with a model coming from localStorage that just contains state.

Is this correct?

[lukejacksonn]

It seems like both @vdsabev and yourself have thought this one through and found valid solutions using a deepMerge implementation. I would tend to do the same.

[whaaaley]

I'm for merging state and actions but I'm against them sharing a namespace. I would solve this by ditching the concept of initial state completely and opt for actions that init state instead.

No state, just actions.

{
  actions: {
    Foobar: {
      howdy: sir => ({ sir })
    }
  }
}

Result after actions.Foobar.howdy('Mr. Cowboy') is called.

{
  state: {
    Foobar: {
      sir: 'Mr. Cowboy'
    }
  },
  actions: {
    Foobar: {
      howdy: sir => ({ sir })
    }
  }
}

But what about inital state?

const actions = app({
  actions: {
    Foobar: {
      howdy: sir => ({ sir })
    }
  }
})

// Init state
actions.Foobar.howdy('Mr. Cowboy')

[vdsabev]

After thinking more about model today (and rewriting a small app with that API), I've come to the conclusion that using model would remove a lot of mental load for me. Specifically:

1. Writing the model as a single encapsulated unit (and slice) containing both data and actions
2. Using model.someValue and model.someAction instead of state.someValue and actions.someAction
3. Passing the model downstream to child components instead of both state and actions

State without actions is just a static object, unaware of the concept of time. Actions without state are just math, unaware of the concept of space. But together, they form an application.

We can use that application from a console, a browser, or a server. We can store the state in memory, files, a remote database, or local storage. It doesn't really matter all that much, because it's a side effect of your model. Not everyone needs it, and if they do - it's a single function call away.

It might be up to preference, but to me merging state and actions into a single model feels completely natural.

[selfup]

How would this change the size of hyperapp? deepMerge I would assume takes up quite some space?

[jorgebucaran]

@selfup Hyperapp would not give you this deepMerge function, you'd have to roll your own when you are reading from local storage and want to create the initial model.

import { view } from "./mainView"
import { someModel } from "./someModel"
import { deepMerge } from "./utils"

const persistedModel = deepMerge(
  someModel,
  JSON.parse(localStorage.getItem("model") // persisted model
)

const model = {
  ...persistedModel,
  other: 1,
  stuff: true
}

app({ view, model })

[selfup]

So we would have to keep this as a 3rd party lib right? Otherwise we are starting to introduce dependencies to get hyperapp to work as needed no?

This would increase the number of bytes to get Hyperapp to run 100%...is my only concern 🤔

Just thinking out loud here! 🚀

---

Your example looks sane btw

[jorgebucaran]

@selfup Not even that. It would be completely left to the user to write their own deepMerge, use something like lodash _.merge or similar, etc.

So far the issue only appears to be an issue when you want to generate a model from a persisted model (since it was persisted, the functions a.k.a actions are gone, so we need to deep merge it with the source model that has them).

I was discussing just this on slack the other day with @lukejacksonn and @Alber70g. I love this proposal, but feel unsure because we complicate de-serialization. They claim that complicating de-serialization is a small price to pay considering how this resolves two of hyperapp most annoying collateral effects of the architecture: (1) initial wiring setup and (2) passing slice state and actions separately down to components.

[selfup]

Thanks for the response! 😄

[vdsabev]

I think the complexity of the deep merge is being vastly overestimated.
Here's something I whipped up in 15 minutes: https://codepen.io/vdsabev/pen/XzvMqM
Let me know if I missed a use case 😄

EDIT: example usage:

import { h, app } from 'hyperapp';
import merge from '@hyperapp/merge';

import { A } from './A';
import { X } from './X';

const LocalStorageStore = (key) => ({
  save: ({ model }) => localStorage.setItem(key, JSON.stringify(model)),
  load: () => JSON.parse(localStorage.getItem(key))
});

const LocalStorageStoreA = LocalStorageStore('model.A');

app({
  model: {
    A: merge(A, LocalStorageStoreA.load()),
    X
  },
  view: (model) =>
    <div>
      <LocalStorageStoreA.save model={model.A} />
      A: {model.A}
    </div>
});

We could even put the merge inside the store and get a completely opaque store, like this:

import merge from '@hyperapp/merge';

const LocalStorageStore = (key) => ({
  save: ({ model }) => localStorage.setItem(key, JSON.stringify(model)),
  load: ({ model }) => merge(model, JSON.parse(localStorage.getItem(key)))
});

app({
  model: {
    A: LocalStorageStoreA.load({ model: A }),
    ...
});

[jorgebucaran]

@vdsabev If we need to provide a custom merge function then I know for a fact this proposal will not move forward. We must find a different solution, or come to terms that there will be no custom merge function and another solution needs to be suggested. For example, building it into the local storage read/load pair of functions.

[jorgebucaran]

Another thing that bugs is that with this there would be only two named props in the object we pass to app(), model and view.

app({model,view}, container)

...so we might as well simplify that to:

app(model,view,container)

[vdsabev]

If we need to provide a custom merge function then I know for a fact this proposal will not move forward.

Would you say the same for the router?

We don't need to provide a custom merge function, it's a convenience to have an official one that is well tested and maintained. De/serialization is an advanced use case that not everyone uses - I haven't in the past 7 years, although I now know I should have - I've come to appreciate how extremely useful it can be.

You get to decide if model is the direction you want to take Hyperapp in or not. This project has already contributed to the happiness of thousands of developers, and we're grateful to have it 🙏

I happen to think it would be better with model instead of state and actions.

[jorgebucaran]

@vdsabev I agree that state+actions=model feels more natural, but I don't think it's natural to have @hyperapp/merge.

So, we need to find a few different ways to solve the de-serialization issue, document them and convey the message that this is just normal business. If we need to go stray out of our way to explain ourselves, then that's a hint we are not on the right path.

[cjh9]

So one thing that will be good if we keep state and actions together is that we avoid boilerplate code:

```app({
model: {
my:{
fancy:{
counter:{
count: 0,
down: () => model => ({ count: model.count - 1 }),
up: () => model => ({ count: model.count + 1 })
}
}
}
},
view: model => (

{model.my.fancy.counter.count}

)
})

Instead of:

```app({
  state: {
    my:{
      fancy:{
        counter:{    
          count: 0
        }
      }
    }
  },
  actions: {
    my:{
      fancy:{
        counter:{    
          down: () => model => ({ count: model.count - 1 }),
          up: () => model => ({ count: model.count + 1 })
        }
      }
    }
  },
  view: state => actions => (
    <main>
      <h1>{state.my.fancy.counter.count}</h1>
      <button onclick={actions.my.fancy.counter.down}>–</button>
      <button onclick={actions.my.fancy.counter.up}>+</button>
    </main>
  )
})

I feel that the argument to keep state and actions apart are similar to the argument "keep css and html apart because separation of concerns", "don't use inline onclick event handlers, you shoud register events in javascript with .addEventListener instead" or "don't use inline styles".

Actually you want to be able to create small reusable components with css, html and js that solve a particular task in the system and does it well. The state can still be global, that makes data routing easier. Take the best from oo and functional programming and combine them, the golden middle way :)

Look how taboo it was when react introduced inline styles https://vimeo.com/116209150

[selfup]

@cjh9 This is a valid point as well! Thanks for the input 🙏

[w-mcilhagga]

We don't need a custom merge function if hyperapp splits the model into state & actions when it is initialized, and just returns the actions part, with all the actions wrapped up as before. Something like this: (Changes marked as comments)

  function init(source, newActions/*initially {}*/, path) { // CHANGE: new parameter
    for (var key in source) {
      typeof source[key] === "function"
        ? (function(key, action) {
            newActions[key] = function(data) { // CHANGE: put the action in newActions object
              source = get(path, model)

              if (typeof (data = action(data)) === "function") {
                data = data(source)
              }

              if (data && data !== source && !data.then) {
                repaint((model = setDeep(path, merge(source, data), model)))
              }

              return data
            }
       delete source[key]    // CHANGE: remove the action from the state
          })(key, source[key])
        : typeof source[key] === "object" &&
          !Array.isArray(source[key]) &&
          init(source[key], newActions[key]={}, path.concat(key))  // CHANGE: extend the newActions object
    }
  }

[I think I mixed up source & model a bit in the suggested changes, but you get the idea]
Obvs. you return newActions, not the model

If this is done, specifying a model is simply a convenience to the end user, with all the pros that have been listed. Your state would be a POJO and could be (de)serialized easily

[w-mcilhagga]

Although if you were going to have hyperapp pull the model part, it might be better to explicitly mark the actions e.g.

  model: {
    count: 0,
    actions: {
        down: () => state=> ({ count: state.count - 1 }),
        up: () => state=> ({ count: state.count + 1 })
    }
  }

This would make it clear that the actions are different from the state, With this convention, init would simply look for actions in each part of the model and pull them out.

[jorgebucaran]

@w-mcilhagga The problem is not serializing the model, that's easy because JSON.stringify removes the functions for you. The issue is de-serializing the model will give it to you without the actions, so you need to deep merge it with your model source before passing it to the app() call.

[w-mcilhagga]

I think this would be a problem in just one case: where you load a state (from localstore, server, wherever) & have to merge it with actions before passing to app().

The solution would be to pass just the actions to app & then (somehow) load the state later - which obvs. has to be the same shape as the actions. This solution also works for cases where you might load/save multiple states in a single session (e.g. an SVG drawing app). This wouldn't be the responsibility of hyperapp, but the actions themselves. For example, you'd have to write your model as

  model: {
    count: 0,
    actions: {
        down: () => state=> ({ count: state.count - 1 }),
        up: () => state=> ({ count: state.count + 1 }),
        set: () => newstate => newstate
    }
  }

The set action will then trigger a merge of the newstate with the existing state, presumably overwriting each property. The shape has to stay the same, but that's standard. Deserialization would then be as simple as

var actions = app(model_with_just_actions_defined)
actions.set(state_without_actions_from_another_place)

Of course, it would be easy to build set/get into hyperapp itself.

[w-mcilhagga]

So I can see maybe a few cases to consider:

1) You can define your state at the same time as the actions, in a .js file. In this case app(model) is nicer than app(state,actions)

2) You can load all of your state from some storage before you define the app. In this case app(state,actions) is better than var a = app(model); a.set(state)

3) You can load some of your state before you define the app (e.g. menus, other gui mixins), but the rest has to be loaded after. In this case I think var a = app(model); a.set(state) works better, because the state you can preload is written as a model. This is the SVG drawing app case.

The question is which case is most frequent. If 2, the model approach isn't worth it. If 3, then the model is better (Case 1 is not that common, I'd venture)

[jorgebucaran]

@w-mcilhagga I think this would be a problem in just one case: where you load a state (from localstore, server, wherever) & have to merge it with actions before passing to app().

Yep! That's it. The thing is, you always have to do this when loading the ~state~ model from somewhere, because under this proposal there is no more state or actions, just model.

[jorgebucaran]

@w-mcilhagga Then the only advantage of the app(model) approach is when you can define your state (maybe a default state) at the time you call app.

Apart from the other dozen of advantages, I can't imagine any situation when you don't know at least the initial state schema.

[jorgebucaran]

@w-mcilhagga var a = app(model); a.set(state)

If this is a userland thing you came up with me, looks good to me. As long as it doesn't need help from core. 💥💯👍

EDIT: I get it now. Haha very clever! ❤️

[jorgebucaran]

@w-mcilhagga The whle question is which case is most frequent. If 2, the model approach isn't worth it. If 3, then the model is better (Case 1 is not that common, I'd venture)

I think case 1 is more common, but every case is important. The reason this proposal is worth the trouble is because of the other goodies it brings to the table, better described here. 😄

[w-mcilhagga]

Any time the state needs to persist between sessions (e.g. on a server) I think you're in cases 2 or 3. Case 1 only applies to state being written in a .js file.

And you're right, there are other goodies in this proposal

[jorgebucaran]

@w-mcilhagga Your .set works too.

[jorgebucaran]

@w-mcilhagga I am speculating. I really couldn't possibly know which case is more common. But if it's 1 or 3 then this checks out, and if it's 2, then your model.set or model.loadFromStorage could be a decent work around. Then there's also deepMerge(persistedModel, initialModel).

EDIT: Actually, no, model.set|loadFromStorage doesn't work because our merge is shallow, not deep. You need to deepmerge yourself, no other way around.

@lukejacksonn @SkaterDad Thoughts?

[alber70g]

If we'd change the actions to return only a state(slice), instead of a full model (state + actions), we would be able to load a deserialized state this way.
This would require a change in Hyperapps internals. Internally the state and actions would be separate, but when implementing actions we'd have to merge state+actions to models to provide actions as payload => model => state.

[alber70g]

@whaaaley (https://github.com/hyperapp/hyperapp/issues/477#issuecomment-350339604)

I'm for merging state and actions but I'm against them sharing a namespace. I would solve this by ditching the concept of initial state completely and opt for actions that init state instead.

One way of implementing this would be to call an init-action that returns the initial state of each slice. But that also requires that init-actions have to be called in order to prevent 'closer-to-the-root' slices to overwrite 'deeper'.

const model = {
  counter: {
    increment: payload => model => ({ count: model.count+1}), // this can't be the initial state
    init: () => ({ count: 0 })
}

[w-mcilhagga]

@JorgeBucaran you said:

EDIT: Actually, no, model.set|loadFromStorage doesn't work because our merge is shallow, not deep. You > need to deepmerge yourself, no other way around.

Not a problem (I think). When I suggested this, I wrongly assumed the merge was deep (and thought - bit of a performance hit but whatev). Shallow merge works fine, when you're setting the state from the top of the object, because you're basically trying to obliterate everything with the new state anyway. If the settable part of the state isn't at the top of your model, put the set at the top of what is meant to be obliterated. Like this

model: {
    static: {...},
    variable: { 
       count: 0,
       actions: {
           up:  () => state=> ({ count: state.count + 1 }),
           set: () => newstate => newstate
       }
   }
}

Then you do

var a = app(model)
a.variable.set(stuff_that_is_pojo)

My thoughts are that once you have a mix of static and variable state (which I'm guessing is a lot of the time), hyperapp should leave it to the user.

This is starting to get a bit speculative, but I think it would work. But we are relying on establishing a programming idiom, and the issue is whether it seems natural to people or just a bit weird.

[lukejacksonn]

@whaaaley @Alber70g I came across a similar realisation when playing with counters.. in this scenario both the reducers and the view depend on a count value from the model, but one doesn't exist until it is the first update of that value. They both have default values for count.

const model = {
  set: (count=0) => ({ count }),
  sum: (x=0) => ({ count=0 }) => ({ count: count + x })
}

const view = ({ count=0, sum, set }) =>
 main([
    h1(count),
    button({ onclick: e => sum(-1) }, 'dec'),
    button({ onclick: e => sum(1) }, 'inc'),
    button({ onclick: e => set(10) }, 'set'),
  ])

app(model, view) 

Perhaps abusing destructuring with defaults a little but I thought it interesting still. You could also use the set reducer like an init function. I played around with an action that you can only call once. Something like:

init: data => model => model.setup ? model : ({ setup: true, ...data })

There are lots more interesting patterns to find I'm sure!

[alber70g]

@lukejacksonn This works, but destructuring everywhere with default state is quite a terrible thing, since when you have more state in that slice than just one variable, possibly even nested states, it's getting pretty messed up. Even more with a multitude of actions. They all have to do that default value and destructuring to provide the default values for nested props.

In Redux this is solved by doing switch-case over the action-name, where the initial run through all actions is an action-name called 'init' where, in each reducer, the default state is returned.

Using an init action that only gets called once would be much cleaner. This init could potentially provide the place to put deserialization stuff.

[jorgebucaran]

@w-mcilhagga Shallow merge works fine, when you're setting the state from the top of the object, because you're basically trying to obliterate everything with the new state anyway. If the settable part of the state isn't at the top of your model, put the set at the top of what is meant to be obliterated.

Exactly, so it will obliterate child props that have actions, you'll end up with a model that has only the state, not the actions. By the way, I am talking about a model.loadFromStorage, which is the only use case I am worried about.

[w-mcilhagga]

@JorgeBucaran either I'm missing your point or you're missing mine. So I'll give a single summary of what I am thinking.

a) A model = state+actions, and is a good way of specifying the app since it reduces repetitious boilerplate & makes composition of models easier.

b) The problem is deserializing the model because the actions get lost during serialization leaving only state.

c) My suggestion is that the init in hyperapp should separate the model into an actions object and a state object internally. The state object has had all the actions stripped out. Example code for this kind of init is in a previous comment.

d) Only the state object gets serialized, and only it gets deserialized.

e) If you need to deserialize a state from localstorage, you just copy it over the existing state using set, which is user-defined. Shallow copy is fine. This doesn't touch the actions, because they have already been separated out by init. If you need to deserialize just a piece of the state, your model has to define a set action in the appropriate model location.

As far as I can tell, this solves the deserialization problem, by pushing it into userland where a trivial line of code (set) does the job.

[jorgebucaran]

@w-mcilhagga We are going to push the deserialization problem to userland anyway.

To summarize my stand on this: I think that the serialization/deserialization issue is not as big as I thought it was and that in a real app you probably won't need to save the entire model, but only parts with the relevant data.

From a slack convo (you should join us if you can use slack :P):

In a real app, you won't be saving the entire model to local storage, or at least not a model with sub models or actions in it. You probably shouldn't and you likely don't need to (you can avoid it if you structure your app in a way to avoid it so)

So, you don't need to merge anything. Take the todo app for example, you want to save the todo items to local storage. This is probably an array with a bunch of todos like:

[ 
  { id: 1, item: "do homework", done : false },
  { id: 2, item: "release 1.0", done : false },
  ...
]

If your use case does involve persisting a model that has actions in it, then a regular

Object.assign(persistedModel, initialModel)

...works for you if your model does not contain any sub models. So, if you can't avoid saving a model with actions to local storage / db, then you need a shallow merge as it probably doesn't have any sub models in it.

Worst case scenario: you need a deep merge (non-trivial line of code, but not over the top anyway) if the model you want to save contains sub models, but I think this should be avoided. And as such, we need to inform users this is a bad practice anyway.

Also, think about this: what if you are using models from a 3rd party library and such library changes its model schema/props after a version change? You'd end up with a corrupt model even if your data is good.

[jorgebucaran]

For the record, here is a minimal impl. of the kind of deep merge we could use:

const revive = (a, b) =>
  Object.keys(b).reduce(
    (obj, key) =>
      Object.assign({}, obj, {
        [key]:
          a[key] === undefined
            ? b[key] // revive original function here!
            : !Array.isArray(a[key]) && typeof a[key] === "object"
              ? revive(a[key], b[key]) // recurse into plain objects
              : a[key] // just copy primitives and arrays!
      }),
    {}
  )

const model = revive(modelFromLocalStorage, originalSourceModel)

[alber70g]

I'm glad my proposal found it's way. I'm also happy that we can move forward. I for one would be unhappy if I'd had to fork this project for this functionality.

Let's not forget to thank @JorgeBucaran for creating and pulling this project.
He's making sure this project doesn't go haywire by making it a real challenge to persuade and convince and bring the right arguments to good ideas.
Because of him and through this way we will be able to create something truly beautiful!

[jorgebucaran]

@Alber70g Thanks! Not so fast though, I am still working on an actual proposal that is similar to what we've discussed here, but without the "problems". As it turns out, there are other issues with the single model approach, namely testing, HOAs (see logger here), naming, etc.

@okwolf When you are online, could you please explain your issue with the proposal from our convo yesterday? 🙏

[alber70g]

No problem. Yesterday I had quite a thorough discussion on slack which let
me think about initial state in a different way. A more 'Redux' way, so you
will. Have a look at this thread for more in depth arguments:
https://hyperapp.slack.com/archives/C41ECC0V6/p1513033334000236?thread_ts=1513030959.000138&channel=C41ECC0V6

On Tue, 12 Dec 2017 at 10:33, Jorge Bucaran [e-mail]
wrote:

@Alber70g https://github.com/alber70g Thanks! Not so fast though, I am
still working on an actual proposal that is similar to what we've discussed
here, but without the problems. As it turns out, there are other issues
with the single model approach, namely HOAs (see logger here
https://github.com/hyperapp/logger/pull/12/files#diff-1fdf421c05c1140f6d71444ea2b27638R7),
naming, etc.

@okwolf https://github.com/okwolf When you are online, could you please
explain your issue with the proposal from our convo yesterday? 🙏

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/hyperapp/hyperapp/issues/477#issuecomment-350995924,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAfjbJMakNWt_aT6KH5KrNo32xyQkyiUks5s_kh0gaJpZM4Q5vWf
.

[SkaterDad]

I ran into another slight annoyance with this new API.

Let's say you're modeling a form. With the existing API, you can pretty cleanly update your form state and ship it off directly to an endpoint. With the new API, you will need to strip out the functions first, or store your form data in a nested object, defeating the beauty of the new API.

model: {
   field1: '',
   field2: '',
   updateField: ({field, value}) => ({ [field]: value }) //not important...
   submit: () => (model) => {
      const stateOnly = removeTheFuncs(model)
      postToMyEndpoint(stateOnly ).then(...)
   }
}

// OR....

model: {
  formData: {
    field1: '',
    field2: '',
  }
  updateField: ({field, value}) => (model) => ({
   formData: {
     ...model.formData,  //
     [field]: value
    })
  },
  submit: () => (model) => {
     postToMyEndpoint(model.formData).then(...)
  }
}

// current api
state: {
// ... other stuff
  form {
    field1: '',
    field2: '',
  }
},
actions: {
//  ... other stuff
  form {
    updateField: ({field, value}) => ({ [field]: value }) 
    submit: () => (state) => {
       postToMyEndpoint(state).then(...)
    }
  }
}

Situations like this come up pretty often for me, so I'm not entirely sure how I feel about this API change anymore. There is definitely a big win to keeping state and actions separated, with boilerplate being the only drawback (which is still less in HA than in many other frameworks).

[jorgebucaran]

@SkaterDad Yes, same here. Also see this https://github.com/hyperapp/logger/pull/12/files

[jorgebucaran]

@SkaterDad Removing the functions can also be done more succintly like JSON.parse(JSON.stringify(model)) if you don't mind the perf hit, but I do mind.

[SkaterDad]

@JorgeBucaran @SkaterDad Removing the functions can also be done more succintly like JSON.parse(JSON.stringify(model)) if you don't mind the perf hit, but I do mind.

True. It does seem wasteful, though.

Hopefully some of the ideas of this API can be salvaged if it doesn't go through. This very likely could reduce our application bundle sizes, but the tradeoffs are becoming apparent. Keeping state & actions separated seems to be a net win.

The old modules approach was pretty handy for reducing the initial application config boilerplate. I never minded threading individual bits of state & actions to the View functions that needed them. Explicit usually wins for me.

[jorgebucaran]

Thanks for your valuable feedback @SkaterDad! I opened #489 to discuss ideas. 🙏

[lukejacksonn]

I would not be satisfied with JSON.parse(JSON.stringify(model)) as a solution either. But neither of your alternative solutions seem bad to me.. I would probably go for some slight adaption:

model: {
   field1: '',
   field2: '',
   updateField: ({ field, value }) => ({ [field]: value })
   submit: () => ({ field1, field2 }) => {
      postToMyEndpoint({ field1, field2  }).then(...)
   }
}

or

model: {
  formData: {}
  updateField: ({field, value}) => ({ formData }) => ({
    formData: { ...formData, [field]: value }
  },
  submit: () => ({ formData }) => {
     postToMyEndpoint(formData).then(...)
  }
}

The boilerplate is not the only downfall of the current arch.. the cognitive load is real too. Maintaining two trees in parallel is not so easy when apps get large. It is _very_ easy to make a mistake and with the amount of time you have to write out namespace names makes it much harder to refactor at scale, so does throwing around arbitrary state objects (I have said it before, but I think of this similar to the difference between export default and export const x some interesting opinions on this here) instead of being explicit about the bits you need.

If we look at this example postToMyEndpoint(state) it is the _easiest_ but how are you (or any other developer looking at your code, perhaps a backend guy) know what state is expected to be at this point in time. One alternative postToMyEndpoint(formData) is a little better, because at least you know the name (or type) of data coming in. Finally postToMyEndpoint({ field1, field2 }) which is the most verbose.. but also the most explicit.

Disclaimer: I don't know which of any of these solutions is the _right_ answer.

[SkaterDad]

@lukejacksonn It is very easy to make a mistake and with the amount of time you have to write out namespace names makes it much harder to refactor at scale, so does throwing around arbitrary state objects...
...
Finally postToMyEndpoint({ field1, field2 }) which is the most verbose.. but also the most explicit.

I agree. Being more explicit about what you're sending to an endpoint (or any actions for that matter) is probably for the best. It would definitely help prevent over-sending data to the server, where it may get rejected for having extra keys. I pass state & actions to my view components piece by piece as a way to enforce an interface (and allow the view component to be more reusable). I would be 100% fine with refactoring my code to that style in order to get the model API, and it may be less buggy in the long run because of it.

You make a compelling argument about the effort it takes to maintain the 2 parallel state/actions trees. I've been working on a pretty large app in hyperapp for months now, and at times it does feel confusing. I've generally not had big issues with refactoring, but my development speed is probably still compromised a bit.

I really do find the new models API appealing. :confused:

EDIT: To clarify, I mostly want an API change to make it easier to "nest" state/action pairs, much like the modules let you do in past versions. It seems to make composing functionality simpler (not related to view layer concerns).

[jorgebucaran]

Closing in favor of #489.