jorgebucaran / hyperapp

1kB-ish JavaScript framework for building hypertext applications
MIT License
19.07k stars 779 forks source link

Combine the concept of state and actions into models 🀯 #477

Closed jorgebucaran closed 6 years ago

jorgebucaran commented 6 years ago

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

Cons

Notes

vdsabev commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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

SkaterDad commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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:

view: model => 
  <main>
    <h1>Hello</h1>
    <AComponent A={model.A} />
    <ZComponent A={model.A} B={model.B}/>
  </main>

...instead of the more verbose:

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

@lukejacksonn How would you solve the problem?

lukejacksonn commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

jorgebucaran commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

Thanks for the response! πŸ˜„

vdsabev commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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

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

Instead of:

  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 commented 6 years ago

@cjh9 This is a valid point as well! Thanks for the input πŸ™

w-mcilhagga commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

@w-mcilhagga Your .set works too.

jorgebucaran commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

@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!