Instead of changing the API #484, let's approach and try to come up with a solution for the boilerplate problem within the current architecture.
// ABC.js
import { B } from "./B"
import { C } from "./C"
export const state = {
value: 0,
B: B.state,
C: C.state
}
export const actions = {
add: value => state => ({ value: state.value + value }),
B: B.actions,
C: C.actions
}
import { A } from "./A"
import { B } from "./B"
import { C } from "./C"
app({
state: {
A: A.state,
B: B.state,
C: C.state
},
actions: {
A: A.actions,
B: B.actions,
C: C.actions
}
})
const view = state => actions => (
<App>
<Feed state={state.feed} actions={actions.feed} />
<Trend state={state.trend} actions={actions.trend} />
</App>
)
// OR
const view = state => actions => (
<App>
<Feed
model={{
...state.feed,
...actions.feed
}}
/>
<Trend
model={{
...state.trend,
...actions.trend
}}
/>
</App>
)
@lukejacksonn @SkaterDad @okwolf @zaceno @Alber70g
A possible way to reduce boilerplate when composing modules/models, while still keeping state and action separate, could be to introduce an action that returns the initial state:
// ABC.js
import { B } from "./B"
import { C } from "./C"
export const actions = {
getInitialState: () => ({ value: 0 })
add: value => state => ({ value: state.value + value }),
B: B.actions,
C: C.actions
}
import { A } from "./A"
import { B } from "./B"
import { C } from "./C"
app({
actions: {
A: A.actions,
B: B.actions,
C: C.actions
}
})
The getInitialState() action would be executed automatically by app(), it could also be renamed getState() init() or simply state().
This action could also be used to subscribe to outside events (like init()/subscribe() was meant to).
Not sure this is a good idea, let's see what you guys think.
@JorgeBucaran would it be possible to provide actions with the state (model-actions) and the view with the model (state+actions)?
@Alber70g the key question here is how strongly do we feel about keeping the API consistent and simple by minimizing the number of concepts new users need to learn.
@whaaaley and I came up with an improvement to the current API without needing to merge state & actions, but making it easier to reduce boilerplate.
It consists of extracting the view from the props argument to app.
app(props, view, container)
Props is the state and actions, same API as 0.16.2. While we are at it, I find it more logical to call it model, instead of props, so I'll do just that.
const model = {
state: {
count: 0
},
actions: {
down: () => state => ({ count: state.count - 1 }),
up: () => state => ({ count: state.count + 1 })
}
}
const view = state => actions => (
<main>
<h1>{state.count}</h1>
<button onclick={actions.down}>-</button>
<button onclick={actions.up}>+</button>
</main>
)
app(model, view)
So almost nothing changes, but the advantage of this approach is that it makes it now easier to merge multiple models.
app(
combine({ A, B, C, D }),
view,
document.getElementById("root")
)
or
app(
{
...model,
...combine({ A, B, C, D })
},
view,
document.getElementById("root")
)
Of course you need to write that custom combine function, but surely this can be pushed to userland.
combine(models)Creates a single model from multiple models. A model is an object of {state,actions} pairs. This function creates a single model from multiple models organized by their keys.
const A = {
state: { ... },
actions: { ... }
}
const B = {
state: { ... },
actions: { ... }
}
const C = {
state: { ... },
actions: { ... }
}
combine({ A, B, C }) /*
{
state: {
A: A.state,
B: B.state,
B: B.state,
},
actions: {
A: A.actions,
B: B.actions,
B: B.actions,
},
}
*/
const combine = models => {
var out = {state:{},actions:{}}
for (var key in models) {
out.state[key] = models[key].state
out.actions[key] = models[key].actions
}
return out
}
const combine = models =>
Object.keys(models).reduce(
(obj, key) => ({
...obj,
...{
state: {
...obj.state,
...{ [key]: models[key].state }
},
actions: {
...obj.actions,
...{ [key]: models[key].actions }
}
}
}),
{}
)
Looks good but it would also make sense to change the view-signature while were at it:
app({state, actions}, ({state, actions}) => h(....))
Symmetrical to the conventional signature of components, and IMO nicer and less confusing to newcomers than state => actions => h(...)
@zaceno I am okay with either one (state, actions) or ({ state, actions }).
(state,actions) means less destructuring though, so better not to create another pain point!
@Alber70g @lukejacksonn @okwolf @whaaaley What do you think? :)
Symmetrical to the conventional signature of components...
Good point though! 馃馃槅
This is definitely a nice incremental improvement!
I'm voting for changing the view signature also. Dont really care which of the 2 proposed solutions, they're both nicer.
fwiw, I been talking to @whaaaley and I like this approach. For the view signature I would prefer (state, actions) but dont care too much.
I like this proposal even more (as opposed to the previous) because it keeps the state and actions separated while at the same time it allows actions and state on slices to be more explicit entangled.
It also allows to have actions still return _only_ state instead of state and actions.
The only thing I feel is kind of obsolete is the _initial state_. Right now we export the state as a part of the model that can be combined by the _combine_-function.
This doesn't have to be this way if we, like Redux does, call all _init_- actions first. The init action can return it's default state. This also removes a part of the cognitive load of thinking about initial-state, actions and the current state provided to actions, as a saparate thing.
My proposal would be to drop the state drop the initial _app_-call as a whole and provide a lifecycle hook which calls each function named _init()_ in the actions object. Starting from root and traversing trough the slices/props.
@Alber70g I don't follow, what's obsolete?
My proposal would be to drop the state drop the initial app-call as a whole and provide a lifecycle hook which calls each function named init() in the actions object. Starting from root and traversing trough the slices/props.
What would be the benefit of adding something like this potentially adding a lot more bytes to hyperapp?
I'm not really a fan of that idea. Being able to define the initial state before Hyperapp starts is really nice, in my opinion, and doesn't introduce special framework specific actions.
I think this is going to be Hyperapp's final API. Just a feeling! 馃槃
const model = {
state: {
count: 0
},
actions: {
down: () => state => ({ count: state.count - 1 }),
up: () => state => ({ count: state.count + 1 }),
/* actionName: data => (state, actions) => State */
}
}
const view = ({ state, actions }) => (
<main>
<h1>{state.count}</h1>
<button onclick={actions.down}>-</button>
<button onclick={actions.up}>+</button>
</main>
)
export const App = app(model, view, document.body) // => { state, actions }
import { app } from "hyperapp"
import { main, h1, button } from "@hyperapp/html"
const model = {
state: {
count: 0
},
actions: {
down: () => state => ({ count: state.count - 1 }),
up: () => state => ({ count: state.count + 1 })
}
}
const view = ({ state, actions }) =>
main([
h1(state.count),
button("-", {
onclick: actions.down
}),
button("+", {
onclick: actions.up
})
])
export const App = app(model, view, document.body)
Looks good. Is the root element a 3rd parameter missing from the example?
@SkaterDad Done.
Hmm, should we make it non optional? 馃馃く
I think it's nice that its optional currently, but I'm not that opinionated on it.
@SkaterDad I feel the same way, but maybe this can somehow be connected with #491.
@JorgeBucaran well, now that you have a feeling we should just ship 1.0 immediately 馃槀
The optionated root element make some troubles in some case.
This is not a secret now, I use Hyperapp to build web component, using the custom
element compliance <hyperapp-custom-element/>.
Using the new API, it will be something like :
document.querySelectorAll('hyperapp-custom-element).map(element => {
app(model, view, element)
})
In the case of there is no <hyperapp-custom-element/>, nothing will occur (the array is empty, no map iteration is called).
But if I am only doing the following :
app(model, view, document.querySelector('#app'))
And no #app exist in the dom, the whole body will be replaced by the app.
I use a custom root element, because the app is located somewhere in a landing page, if I remove the element from the html body, I want the app do not load not to replace the whole body.
I prefer the proposed view signature of ({state, actions}). The current (0.16.2) approach of double fat arrows state => actions => h() to me implies/suggests that there is an opportunity for currying, but that is not really the case.
@Alber70g ~I like your way of thinking! We're on the same page as far as "only actions should make state". The caveat here is that you need a way to load in old state. Having a state prop in app()
lets us load in initial state from local storage or some other place.~
Just kidding. Loading in previous state from <some external place> could also be done with an action. So I'm with @Alber70g on the idea that actions should init state.
As for ({ state, actions })... I'd prefer (state, actions) because less destructuring.
Thank you all for your feedback! 0.17.1 is out. 馃帀
I've updated the README (and other documentation) to reflect the changes, but I may have missed something, if you see something off, let me know.
I am very happy with the result (the new API changes) and feel this can be closed.
@lukejacksonn @Alber70g @vdsabev I haven't had time to write a pretty "pure function", but I got the following working without much code.
// Look! This is a model like in Albert's original proposal! 馃帀
const counter = {
count: 0,
down: () => state => ({ count: state.count - 1 }),
up: () => state => ({ count: state.count + 1 })
}
// Now we need to turn that into the model shape Hyperapp wants!
// We'll need a `combine` or `mapAlbertModelsToHyperModels` function.
const model = combine({ counter })
/* OR
import initialModel from "./model"
const model = {
...initialModel,
...combine({ counter })
}
*/
const view = ({ state, actions }) => (
<main>
<h1>{state.counter.count}</h1>
<button onclick={actions.counter.down}>-</button>
<button onclick={actions.counter.up}>+</button>
</main>
)
app(model, view, document.body)
The current API is not quite "boilerplate-free" out of the box, but it has the flexibility to allow you to write your models in a variety of styles (SEE HERE AND HERE).
The original proposal was partially convenient and I liked it, but it has issues like forcing you to hack or change how you write your code in order to read/save from/to local storage, a database, remote endpoint and make writing tests or HOAs _slightly_ more inconvenient.
I think the current API is a decent compromise and the way forward. 馃憢
@whaaaley you can still do:
const appActions = app(state, { init: payload => ({ ...payload }) }, view);
appActions.init(localStorage.getItem('myState'));
If I'm not mistaken and the render function is ran in a setTimeout()/is put on the callback queue, this wont even pose a render performance hit. Therefore you don't need to set the initial state as property of app.
@JorgeBucaran
It might be good to add a code example where different slices are exported as models and combined by the combine function?
My favourite bit about the models proposal was the way it made passing stuff down the view much easier.. and that when you called stuff it was by namespace.value not (state|actions).namespace.value.
But yes.. you could write a custom combine function that turns almost any data structure into what hyperapp accepts.
@lukejacksonn My favourite bit about the models proposal was the way it made passing stuff down the view much easier.. and that when you called stuff it was by namespace.value not (state|actions).namespace.value.
My favorite bit as well. Almost like a class.
Most helpful comment
I prefer the proposed view signature of
({state, actions}). The current (0.16.2) approach of double fat arrowsstate => actions => h()to me implies/suggests that there is an opportunity for currying, but that is not really the case.