anthonyshort / deku

Render interfaces using pure functions and virtual DOM
https://github.com/anthonyshort/deku/tree/master/docs
3.41k stars 130 forks source link

Abstraction of things that call setState #218

Closed ashaffer closed 8 years ago

ashaffer commented 8 years ago

Is there an accepted pattern for doing this? My motivating example is the form below:

function render(component, setState) {
  const {state} = component
  const {dirty, user, submitted} = state

  const {valid, errors} = validateUser(user)
  const errMsgs = userMessages(errors)

  return (
    <form>
      <SignupInput placeholder='FULL NAME' onChange={setField('name')} error={getError('name')} />
      <SignupInput placeholder='EMAIL' onChange={setField('email')} error={getError('email')} />
      <SignupInput type='password' placeholder='PASSWORD' onChange={setField('password')} error={getError('password')} />
    </form>
  )

  function setField (field) {
    return function(element) {
      const value = element.delegateTarget.value
      setState({
        user: assign({}, user, {[field]: value}),
        dirty: assign({}, dirty, {[field]: true})
      })
    }
  }

  function getError (field) {
    return (submitted || dirty[field]) && errMsgs[field]
  }

It is a simple signup form with validation and a model, and i'd like to be able to abstract out setField, getError, and the little bit of validation setup in render, but setField's dependence on the setState function is making things complicated.

The solutions I can think of are:

The problem with all of them to varying degrees is that they make it less clear where state is being manipulated (which I suppose may just be an inherent tradeoff of abstraction that can't be eliminated).

anthonyshort commented 8 years ago

We had that problem with our forms and tried a few different ways of managing the state. Which were basically the three you mentioned there. The way we do it is by storing all the state on the Form component that holds all of the fields and set it like you've done there. It's not ideal but it works for now.

Ideally state would be kept out of the component and we wouldn't need setState but expecting new people to figure that out would be pretty crazy. I'd want to get rid of it completely, but it's hard when simple cases like this would require so much more setup.

If you come up with something simple we should put it into a guide in the docs.

ashaffer commented 8 years ago

@anthonyshort I think the storing of all the state on the form is right. In general I think concentrating state as high up in the tree as possible is the way to go. I thought of two other possible solutions to this problem that seem pretty good to me:

Solution 1: A 2nd order Form component, like so:

Form.js

export default function (validate, messages, Contents) {
  return {
    render() {
      // ... setup validation

      return (<form><Contents setField={setField} getError={getError} /></form>)

      function setField() {
        // ...
      }
      function getError() {
        // ...
      }
  }
}

SignupForm.js

function render(component) {
  const {props, state} = component
  const {setField, getError} = props

  return (
    <Block>
      <SignupInput placeholder='FULL NAME' onChange={setField('name')} error={getError('name')} />
      <SignupInput placeholder='EMAIL' onChange={setField('email')} error={getError('email')} />
      <SignupInput type='password' placeholder='PASSWORD' onChange={setField('password')} error={getError('password')} />
    </Block>
  )
}

export default Form(validateUser, userMessages, {render})

This has the benefit of working with deku as-is and keeping the app-level component stateless.

Solution 2: An actions property on component with methods that are curried by deku to be passed setState.

E.g.

SignupForm.js

import {setField, getError} from 'form'

function render(component) {
  const {props, state, actions} = component
  const {setField, getError, createUser} = actions

  return (
    <form onSubmit={onSubmit}>
      <SignupInput placeholder='FULL NAME' onChange={setField('name')} error={getError('name')} />
      <SignupInput placeholder='EMAIL' onChange={setField('email')} error={getError('email')} />
      <SignupInput type='password' placeholder='PASSWORD' onChange={setField('password')} error={getError('password')} />
      <button type='submit'>Sign Up</button>
    </form>
  )

  function onSubmit() {
    createUser(user)
  }
}

const actions = {
  setField, 
  getError,
  createUser(setState, user) {
    api.createUser(user).then(() => setState({created: true}))
  }    
}

export default {render, actions}

This method is even more explicit about what is happening than the higher-order component, and allows these helper methods to really just be helper methods rather than shoehorning them into the component model, but obviously requires some modifications to deku's core or possibly a custom virtual-element. This method also has the performance benefit of not storing these functions in render's closure, which should be helpful for GC pressure, though i'm not sure exactly how much of a problem that really is.

EDIT: Fixed example 1.

voronianski commented 8 years ago

@ashaffer I really like actions concept :+1: it will be huge! /cc @anthonyshort

anthonyshort commented 8 years ago

I like where this is going! Something like this could help remove the need for weird flux architectures and remove the complexity around setState. It would be good to think about all the different use cases and see if this solves many of those problems.

ashaffer commented 8 years ago

Following some sort of 'conservation of state setting locations' principle, you could also then remove setState from render, which would let render really just be in charge of rendering.

A slight issue with this approach is it somewhat restricts the locations where state may be set. In my form currently my state object looks roughly like:

{
  user: {name, email, password},
  dirty: {name, email, password}
}

However, if setField were abstract, probably user would have to be changed to something generic like model and the 'dirtiness' state of each field would have to be packaged into the property, like this:

{
  model: {
    name: {value, dirty},
    email: {value, dirty},
    password: {value, dirty}
  }
}

This also raises the possibility of naming conflicts and other nasty stuff like that. Although you could perhaps mitigate that by doing something like:

const actions = {setField('model')}

Which would also let me go back to using the slightly nicer user property. Also, presumably these actions would need at least setState and state in order to function properly, but what about props?

The props issue could perhaps be addressed by something like an initialActions function?

function initialActions(props, setState) {
  return {
    setField: setField('user', setState),
    onSubmit: function(state) {
      if(state.valid && props.onSubmit)
        props.onSubmit()
    }
  }
}

Which really is just the same thing as initialState except that setState is passed to it. You could even just address this minimally by passing setState to initialState and telling people to put these functions on state itself which would keep deku as clean and minimal as possible. I kind of dislike the idea of diluting the meaning of the term 'state' in this way though, so i'm more inclined towards putting these in an actions object.

anthonyshort commented 8 years ago

I'll probably need to sit down and really think about how this will work. But for the dirty flag, we don't really need it for each property. We can just set the instance to dirty an re-render the entire thing. That's what we do at the moment when state changes.

I've been trying to find a way to get setState out of render and the other hooks. It makes things dirty. This setup is reminding me of this blog post about the way Snabbdom does it: https://medium.com/@yelouafi/react-less-virtual-dom-with-snabbdom-functions-everywhere-53b672cb2fe3 We're ending up with a single model and a handler. The only thing I'm not sure about is how this setup would work in a real app with real state.

We'll probably look through our app (which is pretty complex) and see how something like this would make our work easier.

ashaffer commented 8 years ago

@anthonyshort In my case at least, I do actually need an individualized dirty flag for each field. For instance, I don't want to render an 'invalid email address' message if the user hasn't yet entered anything (or tried to submit the form), and I wouldn't want to render it just because they started entering stuff in one of the other fields.

anthonyshort commented 8 years ago

Ah, I see, you mean 'dirty' in terms of the form fields. I thought you meant that the state/model had changed :D

ashaffer commented 8 years ago

I really like the idea of having an explicit mapping of behavior to intent, as done by snabbdom in that article, cycle.js and also Raynos/mercury (which is probably the most similar to this actions concept). I also really like the idea of just an update function that accepts an intent and its parameters, possibly with the actions = {...} syntax being a shorthand for building a function with a switch statement?

What's cool about update being a single function in addition to its syntactic cleanliness and implementation simplicity for deku is that you can compose around it. For instance:

function update(setState, intent, ...params) {
  switch(intent) {
    case 'set_value':
      const [field, value] = params
      setState({[field]: value})
    break
    // ...
  }
}

could then be composed around by say, an undo stack that records all of the intents and the new states they produce and allows you to wind forwards or backwards in time easily. Or something that abstractly blocks actions until a loading state resolves, or all kinds of crazy things. You could even eliminate the setState function and just allow update to return a new state (or promise, perhaps) - that would be really functional. Then there would be no such thing as setting state, and the only way to manipulate state would be to send things to your update function. This would also make it easier to abstract things like my setField function, as you wouldn't need to pass around this setState token everywhere. E.g.

function update(intent, ...params) {
  switch(intent) {
    case 'set_value':
      const [field, value] = params
      return setField(field, value)
    break
    // ...
  }
}

function setField(field, value) {
  return {[field]: {dirty: true, value: value}}
}

EDIT: And maybe a render for that component would look something like this?

function render(component, send) {
  const set = setValue(send)

  return (
    <TextInput onChange={set('username')} />
  )
}

const setValue = autocurry(function(send, field, element) {
  const value = element.delegateTarget.value
  send('set_value', field, value)
})
anthonyshort commented 8 years ago

Yeah! This all sounds great. I've had thoughts about this sort of thing for a while but I haven't got my head around it completely yet. I think I'm thinking about too many use-cases. It reminds me of Cycle, but it will have a much friendlier API. It's also how Redux pretty much works.

function render ({props, state}, handler) {
  return (
    <TextInput onChange={handler('complete', props.todo)} />
  )
}

export let actions = {
  'complete': function (state, ...params) {

  }
}

But I don't think I fully realize how an app would be structured, with API calls, data being passed down etc. It looks like this architecture would favour having a single model, instead of props and state. You might have a better idea of how this could work than I do at the moment :)

You could even eliminate the setState function and just allow update to return a new state (or promise, perhaps) - that would be really functional.

This would be great. I'm not sure how native event handling fits in here, because it's a side-effect. I'm not sure how pure functional programming would go about handling that.

If you can spec out how this might look, we can get this in.

ashaffer commented 8 years ago

As far as deku is concerned, my proposal is actual incredibly simple. It's just this:

  function renderEntity (entity) {
    var component = entity.component
    var fn = typeof component === 'function' ? component : component.render
    if (!fn) throw new Error('Component needs a render function')
    var result = fn(entity.context, setState(entity))

    if (!result) throw new Error('Render function must return an element.')
    return result
  }

turns into, in the simplest possible implementation, this:

  function renderEntity (entity) {
    var component = entity.component
    var fn = typeof component === 'function' ? component : component.render
    if (!fn) throw new Error('Component needs a render function')
    var result = fn(entity.context, action)
    if (!result) throw new Error('Render function must return an element.')
    return result

    function action(...args) {
      setState(entity, update(context, ...args))
    }
  }

So the two primary functions of a component are now update and render, which are both truly pure functions, in the sense that they accept arguments and return a value, and have no other side-effects (except kind of sort of still the dom event handling, but you could still interpret that as being pure, provided the event handlers don't hold any state themselves).

This enables you to write components like this:

import {setField} from 'form'

function render(component, action) {
  return (<TextInput onChange={handleChange('username')} />)

  function handleChange(field) {
    return function(element) {
      action('set_field', element.delegateTarget.value)
    }
  }
}

function update(component, name, ...args) {
  const {props, state} = component

  switch(name) {
    case 'set_field':
      return {user: setField(state.user, ...args)}
  }
}

export default {update, render}

This allows you to abstract out things like my setField function as just pure functions that take parameters and return a value. It also doesn't force you to use the named action pattern, you can send things to your update function in whatever form you like. You could even create a little helper method to translate an actions object into an update function:

import {setField} from 'form'
import {updateActions} from 'deku-helpers'

const actions = {setField}
export default {render, update: updateActions(actions)}
function updateActions(actions) {
  return function(component, name, ...args) {
    return actions[name](component, ...args)
  }
}

But the important part is that it would leave all that up to the user and keep deku super simple.

As for the idea of merging props and state, from the perspective of the render function you definitely could do that if you made this change. My initial reaction though is that I think the difference between the two is super important to emphasize, because whether or not a component holds any state of its own seems like maybe the most important fact about it. Although, I suppose you could determine that now simply by the presence of the update function, which is even more obvious. There are two problems I see with it:

  1. Property name collisions, which is probably not a huge deal, the component should be aware of the name of its own props.
  2. Is update allowed to set the value of things passed in as props? IMO it shouldn't be, but it seems like a strange rule to enforce, given that they are on the same object. But maybe just making those values constant with respect to update will be fine and not confusing.

EDIT: I just realized that also, since update is just a function, you can decorate it with new actions or modify existing actions by composition as well, which seems like a neat/clean way of doing it. For instance, decorating your update with standard form-related actions could look like this:

export default {
  render
  update: compose(form.update, update)
}

EDIT2: I should add, at the moment, i'm really only thinking about my own use cases (i'm attempting a rewrite of a very large angular SPA with deku, so i'm sure i'll have more soon). Which, thus far in the deku app i'm building are just some buttons and some relatively simple forms. So if you have other use cases in mind that we could come up with solutions for that'd be great, the more use-cases the idea is tested on first the better.

anthonyshort commented 8 years ago

We're using deku for our entire app at Segment and the hardest thing we've found with the virtual approach is data flow. I'm wondering how this change could make large app state updates, like modifying a user account or a project, that also includes a request to the server. We have all sorts of complex things happening because we're using app.set.

One way to do this would be to bubble the actions to the top of the component tree and then fire them along the edge of the tree top-down, like DOM events. This means you could pass in something like a project into props (which we can freeze during development so it's actually immutable) and then call the action, and have it mutate the state of a much higher component.

// Some button deep down the chain that removes a project
function render({props, state}, handler) {
  return (
    <button onClick={e => handler('project.remove', props.project)}>Remove Project</button>
  )
}

export default {render}
// The top-level app
function render({props, state}, handler) {
  return <App projects={state.projects} />
}

function update({props, state}, name, ...args) {
  const [project, changes] = args // I'm only half sure you can actually do this :D

  switch (name) {
    case 'project.update':
      return { ...state, projects: api.projects.update(project, changes) }
    case 'project.remove':
      return { ...state, projects: api.projects.remove(project) }
  }
}

export default {update, render}

Then it's up to deku to move events around the tree of components. To make sure you know what actions are available, you could have an actions file that exports symbols or constants for the action names so that you don't get clashes but that would be up to the user.

The concerns you mentioned about merging state/props probably aren't a huge problem, but I do like having that distinction. Naming collisions wouldn't be too much of an issue, the views should have defined models and you can enforce it with the validate hook if you needed to.

With this whole approach we have pure state reducers, (state, actionType, actionData) => newstate, and those updates can be composed pretty nicely.

Couple of things I'm not sure about:

  1. Should actions be able to be stopped while bubbling/capturing? If so, how? Do events need to bubble up or should they just start from the top-down?
  2. Would this make forms less painful? @lancejpollard is working on improving a bunch of ours
  3. Do we still need the concept of context?
  4. Does it make actions too much of a black box? If you name something wrong you could get name clashes and who knows what could happen.
  5. Will this be cleaner than app.set?
  6. Where should API requests live? Can we make these less of a side-effect that can happen in the middle of anywhere?
joshrtay commented 8 years ago

I think bubbling stoppable events would remove the need for context. You could implement bubbling by saying that unhandled actions (update returns undefined) bubble.

ashaffer commented 8 years ago

I was not originally thinking these events would go anywhere but inside the component, but ya this could be a cool way of solving the data-flow problem.

Should actions be able to be stopped while bubbling/capturing? If so, how? Do events need to bubble up or should they just start from the top-down?

Hmm what are you thinking is the use-case for capturing? My reaction is that flowing up would really be the only useful direction for the event to go, and that it should definitely be stoppable, though I think it should be an explicit stop (e.g. e.stopPropagation() or a return false or something) rather than an implicit stop, because its very possible you'd want multiple components in the tree to handle an action. Although maybe you could make propagation opt-in by forcing them to return an empty object?

Would this make forms less painful? @lancejpollard is working on improving a bunch of ours

So, just thinking about my own form problem, I think this would allow you to just import a standard set of form actions into any form component. E.g. 'form.set', 'form.submit'. And your input's could just bubble up a 'form.set' with a field value based on their name. E.g. <input name={name} onChange={handler('form.set', props.name)} />

Do we still need the concept of context?

I think that depends on which part of context you think is important. In your example of the project remove button deep down in the tree, if you wanted to say, render the project's name, e.g. <Button>Remove {props.project.name]</Button>, you'd still need to manually propagate the project down the entire tree, which could be cumbersome. Context would allow you to avoid that problem still. However, i'm not sure that tradeoff is worth it anymore. It seems right to me to pass stuff all the way down, even if it's cumbersome, but that could just be because I haven't built a full-sized app yet :).

Does it make actions too much of a black box? If you name something wrong you could get name clashes and who knows what could happen.

That is definitely the biggest problem with this idea. I'm not sure how to resolve it, or whether we could really resolve it a priori rather than just trying it and seeing how it turns out.

As for the other two questions, I don't have much of an opinion yet. I haven't really worked with app.set or api requests very much yet with deku.

ashaffer commented 8 years ago

In thinking about this some more...i'm thinking that you currently implement your project removal thing something like this?

RemoveButton:

const propTypes = {
  project: {
    source: 'currentProject'
  }
}

function render({props, state}, setState) {
  return (
    <div>
      <button disabled={state.loading} onClick={handleClick}>Remove project</button>
      <div class={state.error ? 'show' : 'hide'}>
        <div class='error'>{state.error}</div>
      </div>
    </div>
  )

  function handleClick() {
    setState({loading: true})
    api.project
      .remove(props.project)
      .then(function() {
        setState({error: null, loading: false})
      }, function (error) {
        setState({error: error, loading: false})
      })
  }
}

api:

export default {
  project: {
    remove (project) {
      http.delete('/project/' + project.id).then(function() {
        app.set('currentProject', null)
    })
  }
}

In the new event-based system, I see how you would handle the deletion of the project. But what about things like a loading-state for buttons or displaying an error if an asynchronous operation fails? How would the button get informed about that?

ashaffer commented 8 years ago

Another thing that is almost certainly necessary is that the update method be able to emit new events up the tree as well, so that update methods can aggregate, combine or otherwise translate lower-level events for the consumption of higher-level components.

Also, crazy idea: Replace all dom event handling with this strategy, so there is only a single unified event abstraction. E.g.

TextInput:

function render () {
  // Trying to assign localFn here would throw an error or something, so that
  // we can ensure events are handled solely by handler()
  return (<input name={props.name} onChange={handler('onChange')} onInput={localFn} />)

  function localFn() { }
}

function update ({props, state}, emit, name, ...args) {
  switch(name) {
    case 'onChange':
      const value = element.delegateTarget.value
      emit('set', props.name, value)
      break
  }
}

And the way higher-level components would listen for bubbling is this:

function render() {
  return (<TextInput name='username' onSet={handler('set')} />)
}

function update({props, state}, emit, name, ...args) {
  switch(name) {
    case 'set':
      // do things
      emit('change')
  }
}

This would solve two problems simultaneously. It solves the naming conflict problem, because each component could have local aliases via the handler method if it wanted to <Form onChange={handler('set_field')} />, and it also makes the actions substantially less magical. Your update does not receive an action unless it's setup with handler on one of the elements in your render.

To maintain the sort of fiction that these are actually pure, these emit/handler functions should be asynchronous, and considered just syntactic sugar for this:

function update(emit) {
  const msgs = []

  // emit is syntactic sugar for this
  msgs.push(['set', field, value])

  return {
    msgs: msgs,
    state: state
  }
}

EDIT: Actually, upon thinking about it further, I think @joshrtay's solution for bubbling is the right one. An event bubbles until it is handled, as determined by causing an update to state. In other words, an event bubbles until it becomes part of someone's state.

This also means you can write components that have no update function themselves, but still use handler() as a way of propagating/translating events upwards. And you can also eliminate that emit function I added above, which was awful.

Deku's entire dataflow model could now be succinctly expressed as 'props flow down, events flow up'.

ashaffer commented 8 years ago

Ok, last repeated post, I swear. But I think i've got it now. If you ignore the thing I said earlier about forcing all event handlers to be handler(), you can do api requests in a neat way:

function render({props, state}, handler) {
  return <button onClick={handleClick}>Remove project</button>

  function handleClick() {
    handler('remove_project_start')
    api.project
      .remove(props.project.id)
      .then(handler('remove_project_success'), handler('remove_project_fail'))
  }
}

And I think the biggest instance of multiple components being interested in a single event will likely be where the local component wants to update its state (in this case, say, a loading bool), and it still wants to notify some unspecified parent. For that, you can just emit two events. A local one and a global one.

So, as I see it the complete concept is:

Thoughts?

EDIT: Also, how about changing handler to trigger?

anthonyshort commented 8 years ago

These are all great ideas. The main thing I'm worried about with the bubbling is that we'd fall into the trap of needing to trace events around to see where state was mutated. I'm going to read more into the Elm architecture to see how they handle it. It makes total sense for local state, and maybe app state can be done elsewhere, maybe even outside of deku.

ashaffer commented 8 years ago

Ya, i'm less certain about the event bubbling. I created an implementation of the whole thing though if you or anyone else wants to play around with it:

https://github.com/ashaffer/deku/tree/update-state

disclaimer: I've only written tests for it, not actually used it yet. So it may be that it is broken in various major ways.

EDIT: I'd also add an argument that i'm only about 70% convinced by: this event bubbling strategy is not actually adding a new abstraction to deku, it's just extending the existing DOM abstraction which the programmer has to contend with anyway. I suppose this is really an argument for saying whichever solution is chosen, it should also probably subsume normal DOM event handling as well.

joshrtay commented 8 years ago

if you have an implicit initialize event, you could get rid of initialState. so a component really is just a render and update.

ashaffer commented 8 years ago

@joshrtay That's true. You can actually replace all of deku's current lifecycle hooks with update calls.

joshrtay commented 8 years ago

In addition or instead of an initalize event you could have a props event or propsUpdate event that receives props. Then update and render just need state.

The problem with it is that it would increase the amount of boilerplate necessary for a component. All components would basically require a trivial update.

function render(state, handler) {

}

function update(state, name, ...args) {
  switch (name) {
    case 'props':
      return { ...state, props: args[0] }
  }
}
joshrtay commented 8 years ago

I also think you may want to consider passing an event as a single object instead of a name and a set of args. So the previous example would become:

function update(state, evt) {
  switch (evt.name) {
    case 'props':
      return { ...state, props: evt.data }
  }
}

Seems a little cleaner to me and has the added benefit allowing for a bubbling bool on the evt that controls bubbling.

anthonyshort commented 8 years ago

@joshrtay One minor "downside" to that is that you can't curry the handler to use it nicely within the elements:

<div onClick={handler('completeTodo')}>

You'd need to do something like:

<div onClick={e => handler({ type: 'completeTodo' })}>

but you're right, having an action as a single object like an event feels more correct.

I'm going to try and use this to put together a mini app over the weekend that tries to do some of the more obscure things that we handle in our real app now.

joshrtay commented 8 years ago

hadn't thought of that. that is unfortunate.

also handler should probably be renamed to trigger. you trigger an update.

joshrtay commented 8 years ago

@anthonyshort you could still curry handler/trigger if it's not a direct call to update. right?

function trigger (type, payload) {
  component.update(entity.state, {type: type, payload: payload})
}
ashaffer commented 8 years ago

@joshrtay Doing that does allow currying to possibly be more effective. It seems like there's two plausible options here. I just updated my branch to do the following:

  function triggerEvent (entity) {
    return function () {
      var curried = Array.prototype.slice.apply(arguments)

      return function () {
        var args = curried.concat(Array.prototype.slice.apply(arguments))
        // ...
      }
    }
  }

Which allows you to do things like <TextInput onChange={trigger('set', 'username')} />. But it has the downside of requiring you to do this if you wanted to call trigger outside of there:

<TextInput onChange={handleChange} />

function handleChange(e) {
  trigger('set', 'username', e.delegateTarget.value)()
}

Which is kind of lame. In your approach, you could curry only the first argument, and then pass other parameters in your event object, like: <TextInput onChange={trigger({event: 'set', field: 'username'})} />

With either approach, you could also do neat stuff like

<TextInput onChange={compose(extractValue, trigger('set', 'username'))} />

function extractValue(e) {
  return e.delegateTarget.value
}
ashaffer commented 8 years ago

So i've encountered a slight issue. Not sure what the best approach to solving it is, but I think it's a pretty general problem.

So a form has a validity state, but in theory at least, its validity shouldn't really be a part of your official state object, since it is a computed property of the values currently in the form. So if you don't want to put the validity of the form in your actual state object, then you have to recompute it (and duplicate all the associated code) in both your render and update functions if you want to say, highlight things in red when they are wrong, but also block form submissions when the form is invalid (render highlights, update blocks submissions). Performance wise this is mostly probably negligible, but it isn't very DRY.

It seems like there are a few possible ways to address this:

Anyone have any thoughts on the best of these, or other ideas entirely? I'm kind of leaning towards the last one, at the moment.

EDIT: It occurs to me that most forms have a server-side validation component as well, and that that is an external data source, so validity is probably actually a legitimate component of state. But I think this problem still kind of exists in general.

ashaffer commented 8 years ago

In regards to the currying of trigger, i'm starting to think that the right solution is for deku not to try to solve it. Since sometimes you may want to pass many arguments, and sometimes you may want some other function in render that decides whether to execute a trigger or not, and you don't want to have to do something awkward like trigger('submit')(), or pass more arguments than you need.

So I think the thing to do is let the user worry about it, and suggest they do something like this:

function render(state, trigger) {
  const triggerer = curry(trigger)

  return (
    <form onSubmit={submit}>
      <TextInput onChange={triggerer('set', 'username')} />
    </form>
  )

  function submit (e) {
    if (state.valid)
      trigger('submit')
  }
}

For a slightly special defintiion of 'curry' that doesn't attempt to pre-determine the arity, and just works like this:

function curry (fn) {
  return function (...outer) {
    return function (...inner) {
      return fn(...outer.concat(inner))
    }
  }
}

Really they just ought to add explicit currying to the ES7 bind syntax proposal. It's silly that we have to do things like this, rather than just trigger::('set', 'username')

ashaffer commented 8 years ago

New idea: Every component receives every action (optionally with bubbling used to decorate actions, like snabdom), action types are symbols, and request/response patterns are also mediated by symbols placed on the actions themselves

Signup form:

const failure = Symbol('failure')

const propTypes = {
  createUser: {source: 'createUser'},
  redirectToLoggedIn: {source: 'redirectToLoggedIn}
}

function render () {
  return (
    <form onSubmit={trigger(createUser, {onSuccess: redirectToLoggedIn, onFailure: failure})}>
      // ...
    </form>
  )
}

function update (state, action) {
  switch(action.type) {
    case failure:
      return assign({}, state, {error: action.error})
  }
}

Actions:

export default {
  createUser: Symbol('createUser'),
  // ...
  redirectToLoggedIn: Symbol('redirectToLoggedIn')
}

app:

app.set('createUser', actions.createUser)
app.set('redirectToLoggedIn', actions.redirectToLoggedIn)

handlers:

export default {
  [actions.createUser]: function (action) {
    api.user
      .create(action.user)
      .then(trigger(action.onSuccess), trigger(action.onFailure))
  },

  // ...
  [actions.redirectToLoggedIn]: function (action) {
    app.set('currentRoute', '/home')
  }
}

This gives you all the power of Elm's mailboxes with plain js, and it allows the component to remain 100% pure without sacrificing much flexibility. In terms of deku's core, the code is dead simple - every component receives every action, and naming conflicts are a non-issue because of symbols. Components could then also export the symbols necessary to communicate with them, so that the components can talk to each other in a pure way when necessary.

hden commented 8 years ago

Some bikesheds:

  1. If you would like to serialize and playback actions (e.g. time travel in redux), Symbols are not easily serializable, probably better keep it optional.
  2. There are so many existing solutions for message passing e.g. FRP event stream, ES7 observable, CSP channel, Actor mailbox, what's the rationale behind actions?
anthonyshort commented 8 years ago

@hden Totally agree. This is just discussion to look at possible ways of doing it. I'd like to just find a simple way to manage state so we don't need to use imperative calls like setState. I'm just not sure how this would look just yet so I'm hoping people who know way more about functional programming can help out :)

ashaffer commented 8 years ago

The simplest thing to do might just be to use actions/update for local state updates, and then use context to do communication across components as was previously planned? I think that would pretty elegantly address all the issues in a clean/functional way.

ashaffer commented 8 years ago

I think trigger/handler can also subsume the role of sources, and be used to handle request/response scenarios for api calls:

import dispatch from 'lib/fluxxy-thing'

function beforeMount (component, handler) {
  dispatch('subscribeToCurrentUser', handler('updateCurrentUser'))
}

function render (component, handler) {
  return (
    <form 
      onSubmit={handler('createUser', {success: handler('success'), error: handler('error')})}</form>
  )      
}

Where the dispatch in beforeMount just sets up a pipe between this component's update function and some external data source.

hden commented 8 years ago

Probably better fit elsewhere, but If you consider a component to be a transducer that:

It's like turning a component inside-out, all the states are nicely contained in the observable / channel, which is very similar to cycle.js.

Maybe we can start from allowing handlers to return a stream / observable / channel bound to certain parameters?

ashaffer commented 8 years ago

@hden That is an interesting idea but how would you handle components generating their own events (click/input/etc)?

hden commented 8 years ago

@ashaffer Events are generated by user / DOM / clocks (e.g. setTimeout) / remote state (e.g. database). A component merely respond to the events by emitting a virtual element.

ashaffer commented 8 years ago

@hden Ya, that is true, but I guess what i'm asking is how you are imagining the syntax for something like that would look.

hden commented 8 years ago

Unlike cycle.js, we probably want to start with a minimum API and let user choose their FRP implementation. e.g. bacon.js, rxjs, most

// stolen from cycle.js
let component = (eventualSelect) => {
  // eventually select a element from rendered DOM
  let eventStream = eventualSelect('.field', 'input')

  return eventStream
    .map(ev => ev.target.value)
    .startWith('') // initial value
    .map((name) => {
      return (
        <div>
          <label>Name:</label>
          <input className="field" type="text" />
          <hr />
          <h1 className="header">{ 'Hello' + name }</h1>
        </div>
      )
    })
}

EDIT: A similar concept has been posted by Allen Kim http://codrspace.com/allenkim67/a-simpler-web-architecture-using-flux-csp-and-frp-concepts/

joshrtay commented 8 years ago

@anthonyshort Have you thought about removing state from deku? It seems like everyone has different opinions on how state updates should be handled and even on whether components should manage local transient state at all.

Instead you could add a setupContext hook to deku. The setupContext hook would receive context and markDirty as parameters. That way if components want local state they can set it up themselves and they can choose their preferred approach. Using redux this might look like:


import { createStore } from 'redux';

function setupContext (context, markDirty) {
  let store = createStore(update)
  store.subscribe(markDirty)
  context.dispatch = store.dispatch.bind(store)
  Object.defineProperty(context, 'state', store.getState.bind(store)) 
}

function render ({props, state, dispatch}) {
  return (
   <div>
    <button onClick={emit('DECREMENT')}>-</button>
    <div>{state}</div>
    <button onClick={emit('INCREMENT')}>+</button>
   </div>
  )

  function emit(type) {
    dispatch({type: type})
  }
}

function update (state = 0, action) {
  switch (action.type) {
  case 'INCREMENT':
    return state + 1;
  case 'DECREMENT':
    return state - 1;
  default:
    return state;
  }
}

If update is exported with the component, then you could move the setupContext to a virtual-element wrapper and virtual-element implementations could specify how state updates are handled.

anthonyshort commented 8 years ago

@joshrtay that's what I think we might just need to do. Make components completely stateless and provide some easy ways for people to send in there own dispatcher/handler/way to trigger actions. We could do this at the top level, when render is called, and allow people to pass in a dispatch function that will be given to all components:

var store = createStore(reducers)
render(<App state={state} />, store.dispatch)
function render (component, dispatch) {
  return <button onClick={ dispatch({ type: 'setState', id: component.id, data: { clicked: true }}) } />
}

That would allow people to keep using a pattern that is somewhat familiar. Or they could just pass it down like you suggested.

render(<App state={state} dispatch={store.dispatch} />)

This starts to look a lot like the Elm Architecture. Deku is just a side-effect that is performed after the application state has been mutated, but doesn't manage the state itself. Then you can use something like Redux to manage your application state.

There are a couple of concerns with it that I brought up over here: https://github.com/paldepind/noname-functional-frontend-framework/issues/14

  1. You need to patch that dispatcher all the way down
  2. Local component state needs to find a place to live, eg. whether a select box is open.
  3. You need to pass state through components that don't care about it.

I think 1. is probably ok. Every component should have a way to trigger actions in the system. If you don't pass it down, it can't change the state in any way, which could be nice.


@hden I think we'll probably avoid requiring any sort of observable interface. The concept of returning a stream of vnodes is nice (like cycle), but in this case I'd want to lean on the side of developer experience and not require them to do any work with streams. They really just need to define that final map function.

We could make it function that way behind the scenes but there's something really nice to just being given a state object and returning a vnode. The trade-off for this is that events are defined inside of the vnode instead of as streams.


These are some things we need to cover in any solution, and I think we get these with the above solution:

Other notes:

Some reading I've been doing lately around Elm that might be of interest:

ashaffer commented 8 years ago

@anthonyshort I think that's right. Reusable components should never manage their own state, as a rule of thumb. This is probably a good heuristic whether or not deku handles state in its core, but becomes even moreso if it doesn't.

The thing that makes the most sense to me, but ideally wouldn't be prescribed by deku, is that the component tree's state is just one of the properties on the global, singular state atom. E.g. state.ui, and the updates are handled by reducing functions - you could create a neat binding to redux by just composing around virtual-element and combining all your update functions into a single state.ui reducer. But that could totally be an optional 3rd party library.

anthonyshort commented 8 years ago

Here's where it's currently up to: https://github.com/dekujs/deku/tree/1.0.0

Removed setState, shouldUpdate, and cleaned up a lot of things. The API is still the same, minus setState but I'm going to see if we can make it backwards compatible. We need to settle on an API soon so we can stop making breaking changes.

joshrtay commented 8 years ago

Cool. Looking much cleaner. What are you thinking in terms of a global dispatch vs passing dispatch down in props?

Also, have you considered event bubbling instead of a global dispatch? I know you mentioned that you're worried about tracing events around, but I find that when events bubble up they're not too difficult to trace - don't really have a problem with tracing dom events. This would give you the power to decorate events with contextual information and it keeps data flow really simple. Data flows down through props. Data flows up through events. Events that reach the root component can be hooked up to stores.

ashaffer commented 8 years ago

Looks good, the core is now under 1k SLOC!

+1 on what @joshrtay said about event bubbling. I think an API that looks like render ({props, emit}) where emit just bubbles up the tree, and some higher-level component with direct knowledge of the primary state store (e.g. redux) dispatches messages to be statified. In this way, the entire UI is just another observable generating events that cause state updates.

Assuming that route is taken, the issue that remains is how to tell individual components about the state they're interested in. The approach that the elm/snabbdom people seem to be taking is that you just pass everything down and it's really not as bad as it seems. That seems plausible to me, but maybe we should try to come up with some strategies/patterns for implementing that in vaguely real-world scenarios? Or just some PoC's that attempt to demonstrate that it can be done in a sane way.

ashaffer commented 8 years ago

For extreme purity and cleanliness, you could not even have a dispatch/emit function, and just let DOM event handlers return new events. E.g.

function render (props) {
  return (
    <div onClick={() => return {type: 'REMOVE_PROJECT'}}>
      Remove Project
    </div>
  )
}
joshrtay commented 8 years ago

@ashaffer How would listening to custom events work in components higher up the tree? And how would you supply a catchall?

Custom:

render(<App onRemoveProject={removeProject} />)

Catchall:

render(<App onAny={dispatch} />)
joshrtay commented 8 years ago

@ashaffer Also, how would you stop propagation? Just a stopPropagation bool on the event object?

ashaffer commented 8 years ago

@joshrtay Your custom example is what I was thinking. Stopping propagation would be weird in the version without emit/dispatch, since the return value of the handler is supposed to be a new event. It might mean that that approach isn't quite right. You could put stopPropagation on the event, but that's not very pure, though maybe that's ok in this case.

As for the catch-all issue, I hadn't thought about that, but I suppose you'd want to be able to generically pipe all events into a state store of some kind. onAny seems fine, or perhaps the top-level component is just an emitter or observable.

joshrtay commented 8 years ago

I think component.id is still going to be necessary to do a setState like action. Didn't see it in the code base.

function setState (componentId, state) {
  return {type: 'setState', id: componentId, data: state}
}

function render ({id, props}) {
  return (
    <div onClick={() => setState(id, {clicked: true})}>
      Click
    </div>
  )
}