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

[Meta] v2 API thoughts #337

Open rstacruz opened 8 years ago

rstacruz commented 8 years ago

Hey, i've had a chance to prototype the v2 api and I'd like to share my thoughts on it.

Stateless is awesome

I really love how Deku's component API isn't made with classes. This was one of the things I disliked about React: if components are classes, when are they constructed? How can I access the component instance? Thinking outside classes really drives in the functional and immutable aspect of the API design.

But removing setState was a bad idea

The biggest change from v2 is the removal of state and instead relying on Redux. I think this can cause a lot of headaches.

React's rendering works something like this:

ReactDOM.render = (element) => {
  while (true) {
    tree = render(element) // render components to a virtual dom tree, recursively

    if (someone called setState()) {
      applyStatesToComponents() // and then render again
    } else {
      break
    }
  }

  applyChangesToDOM(tree)
}

Effectively, this means 30 components can call setState() in one render pass, but re-rendering will only happen once. This is good, because it means we don't re-render 30 times if 30 components want to change states.

Using Redux for states

In contrast, deku v2 asks you to dispatch a store action instead. If 30 components want to change states. This means store.subscribe(() => { ... })'s callbacks will be called 30 times, which means re-rendering 30 times.

Also, this dispatching can happen /while/ the tree is rendering, causing problems. Consider this naive example:

export function onCreate({ path, dispatch }) {
  dispatch({ type: 'UI_SET_STATE', path, values: { loaded_at: +new Date() } })
}

This is a bad idea, because it means a re-rendering will happen while the onCreate action is executed. You can mitigate this by using requestAnimationFrame or process.nextTick or similar, so that they happen after rendering:

export function onCreate({ path, dispatch }) {
  process.nextTick(() => {
    dispatch({ type: 'UI_SET_STATE', path, values: { loaded_at: +new Date() } })
  })
}

But this means DOM updates might happen twice, and the rendering is now needlessly asynchronous.

Use cases of states

Here are some use cases for setState:

...these transient state flags will only polute the Redux state.

rstacruz commented 8 years ago

A bias towards Redux is not necessary

The deku v2 API looks like this:

import { dom } from 'deku'

const store = /* a redux store-like object */

const render = dom.createRenderer(document.body, store.dispatch)
render(<MyApp />, store.getState())

When used with Redux, it'd be more like this:

const store = createStore(...)

const render = dom.createRenderer(document.body, store.dispatch)
const update = () => { render(<MyApp />, store.getState()) }
store.subscribe(update) // re-run render() when new things happen
update()

Passing dispatch and context

As far as the user is concerned, all she needs to expose to Deku v2 is the dispatch and context, which are arbitrary objects that will be passed onto render(), onCreate(), and so on.

export function render({ context, dispatch }) { ... }
export function onUpdate({ context, dispatch }) { ... }
export function onCreate({ context, dispatch }) { ... }

Making context arbitrary

Instead of having a bias towards the names context and dispatch, why not let the user pick their own names, in case they don't want to use Redux? All it really is is just a mechanism to pass down objects all the way down the render tree.

render = dom.createRenderer(document.body)
const update = () => {
  render(<MyApp />, { state: store.getState(), dispatch: store.dispatch, foo: bar })
}

// elsewhere, pass that `context` object as splatted out into the `model` argument:
export function render({ state, dispatch, foo }) { ... }
export function onUpdate({ state, dispatch, foo }) { ... }
export function onCreate({ state, dispatch, foo }) { ... }
troch commented 8 years ago

In contrast, deku v2 asks you to dispatch a store action instead

it doesn't need to be an action, and it doesn't need to be sent to a store. The point of dispatch is to channel all UI actions to a single place. It can then act as a proxy, dispatching to more than one place.

rstacruz commented 8 years ago

The point of dispatch is to channel all UI actions to a single place.

States are a bit more than this. For instance, if I have an <Image> component and I want to track how long it took to load, I'd probably do:

export function initialState () {
  return { startedAt: new Date() }
}

// if the event handler was given access to state
function onLoad ({ state }) {
  var elapsed = +new Date() - state.startedAt
}

With the new API, this isn't possible. You can probably trigger a dispatch at onCreate instead:

export function onCreate({ dispatch, path }) {
  // hypothetical non-redux dispatch() that saves UI states
  dispatch(path, { loadedAt: +new Date() })
}

...But for any dispatch to be useful, it needs to re-render the tree (ie, call render() again), which you don't need at that point. This is just one of the pitfalls of not having component states.

anthonyshort commented 8 years ago

Effectively, this means 30 components can call setState() in one render pass, but re-rendering will only happen once. This is good, because it means we don't re-render 30 times if 30 components want to change states.

The idea is that Deku is lower-level than that. You could use something like raf-debounce to make sure the render function is only called once per frame.

I think what would be nice is to have a higher-level module that handles a lot of the wiring and setup for you. It's just outside of the scope of Deku itself. Sort of like how Elm has the StartApp module, you could have a similar thing that hooks up Deku, Redux, rendering loop, handles side-effects, handles adding local state middleware etc.

This is similar to @ashaffer's vdux:

start(app, node, [...middleware])

these transient state flags will only polute the Redux state.

True, but you also don't have to store the state in Redux. It's just a way to get actions from the UI. You could do something like:

let state = {}

let render = createRenderer(node, action => {
  if (action.type === 'update state') {
    state[action.path] = { ...(state[action.path] || {}), ...action.state }
  } else {
    store.dispatch(action)
  }
  scheduleRender()
})

render(<App />, {
  state,
  store: store.getState()
})

You could even go so far as to have two updates, use Redux to update the context, and use the Elm-style way of passing down an update function for local state. This would allow you to have reducers within each component for their own local state, and use dispatch when you want to change something on the context.

You could also wrap components in a higher-level component that manages pulling UI state from the context and handing it to the component. You'd get an updateState function passed in to the model. It could also manage clearing the state when it's removed, debouncing, adding the correct shouldUpdate etc. That way it would be mostly transparent to users.

Instead of having a bias towards the names context and dispatch, why not let the user pick their own names, in case they don't want to use Redux?

The only reason I made dispatch it's own thing is that I wanted to make the idea of UI actions a first-class citizen. The dispatcher shouldn't ever change over the lifetime of the program, so it's different than context. It's just a port the UI can use to send data, similar to requesting a url, so that you don't couple the state logic with the view logic.

troch commented 8 years ago

...But for any dispatch to be useful, it needs to re-render the tree (ie, call render() again), which you don't need at that point. This is just one of the pitfalls of not having component states.

The problem is not dispatch or component states. You can keep local state using your own registry and identifying components by their path. The problem is more that you don't have any mechanism re-render a specific sub-tree or leaf.

It is particularly useful for images like mentioned, and also content which needs to change without a data change (when time rather than data is the trigger): look at https://github.com/troch/react-timer-hoc for example.

Maybe I am looking at it the wrong way? As you said, deku is quite low-level. Could an application tree have many deku sub-trees?

anthonyshort commented 8 years ago

An example of the higher-level component that handles state:

function stateful (Component) {
  let state = {}
  let pendingState = {}

  function updateState (path, dispatch) {
    return state => {
      pendingState[path] = state
      dispatch({
        type: 'state changed'
      })
    }
  }

  function onCreate ({ path }) {
    // you could even do a Component.initialState call here
    state[path] = {}
  }

  function render ({ path, props, dispatch }) {
    state[path] = {...state[path], ...pendingState[path] }
    return <Component state={state[path]} updateState={updateState(path, dispatch)} {...props} />
  }

  function onRemove ({ path }) {
    delete state[path]
  }

  function shouldUpdate ({ path }) {
    return path in pendingState
  }

  return {
    render,
    onCreate,
    onRemove,
    shouldUpdate
  }
}

Then in your component:

let App = {
  render: ({ props }) => {
    return <button onClick={props.updateState({ text: 'world' })}>{props.state.text || 'hello'}</button>
  },
  onCreate
}

export default stateful(App)

Then you just need to handle the state changed action at the top-level of your app so that you re-render. It will re-render everything, but only if things have actually changed.

This way is a little too magic-y for me, and I'd probably go the Elm route for local state and just pass down an update function that gets reduced the whole way up.

anthonyshort commented 8 years ago

For the record, even in v1, and in React, the whole tree is just re-rendered on a state change. All it did was just trigger the top-level render function action. Components work as thunks to prevent rendering branches of the tree. With a really basic shouldUpdate in there, it should skip re-rendering the majority of the tree when it doesn't need to.

Remember the only thing you're actually skipping is the diffing. It won't actually apply any changes to the dom and the diffing should be rather quick. You can manually speed up the diffing too by splitting your render function into smaller functions and memoizing them.

I'm going to put together some docs for all of this so people can more easily make a choice :)

rstacruz commented 8 years ago

Interesting.

I guess passing something other than a Function to dispatch can work:

state = {}
render = createRenderer(document.body, { state: setPathState, store: store.dispatch() })
update = debounce(() => render(<App />, { state, store: store.getStore() })

let setPathState = (path) => (values) => {
  state[path] = { ...(state[path] || {}), ...values }
  update()
}

And in your components:

render ({ dispatch, path }) {
  const setState = dispatch.state(path)
  return <button onClick={onclick(setState)}>
}

let onclick = (setState) => (path) => {
  setState({ clicked: true })
}

A bit long winded but I'll take it :) We do miss the ability to set an initialState, though. The docs seem to really push Redux in there though, as if passing an object as a dispatcher is something non-kosher.

troch commented 8 years ago

There is still something I trip over: in React the whole tree is re-evaluated on each setState / mutation, but isetState marks nodes as dirty, forcing their render function to be re-evaluated.

Without having the ability to mark nodes dirty, does it prevent optimizations? Are shouldUpdate or memoized render functions possible for non-leaf components? A component can only compare what it uses (props & context) but cannot account for data dependencies of components down its path.

For example, with redux and react, connect is used to slice some parts of the state and pass them to a connected component. It behaves like a pure component, shallow comparing props and sliced / derived pieces of state. However it is possible to nest them, thanks to dirty marking.

anthonyshort commented 8 years ago

cannot account for data dependencies of components down its path

True. I didn't think of that. You could still mark nodes as dirty using their path. When a component is checking to see if it should update, it could check to see if there is any pending state in any deeper paths, which is pretty easy because you know 0.1.0.4.6 is within 0.1.

The downside is that if you have a 'pure' component at the top level with a pure shouldUpdate, it will skip the whole sub-tree. So you'd need to set the shouldUpdate for every component higher in the tree to be checking some pending state object.

I wonder if the shouldUpdate function could just be declared at the top level on the renderer, so that it affects all components. It is a renderer-level piece of functionality.

function shouldUpdate (component, prev, next) {
  // if the state has change for this component or any components within this `path`
  // then return true. If prev and next have different props, children, or context, also
  // return true.  
}

let render = createRenderer(el, dispatch, shouldUpdate)

Then you could apply a 'pure' update function as well as a check for state changes in a single spot for your entire UI.

troch commented 8 years ago

Another idea, going down the path of memoized render functions:

rstacruz commented 8 years ago

I managed to hack together a setState replacement like so:

function render ({ props, path, context, dispatch }) {
  let state = context.ui[path]
  let setState = (data) => { dispatch({ type: 'ui:set', path, data }) }

and in Redux:

const ui = createReducer({
  'ui:set': (state, { path, data }) => {
    return { ...state, ui: { ...state.ui, [path]: { ...state.ui[path], ...data } }
  }
})

and in App:

    let render = dom.createRenderer(
      document.getElementById('_app'),
      this.store.dispatch)

    let update = () => { render(<AppRoot />, this.store.getState()) }
    this.store.subscribe(debounce(update, 0))
rstacruz commented 8 years ago

ohh, not bad.

An example of the higher-level component that handles state: https://github.com/dekujs/deku/issues/337#issuecomment-168034492

Mind if I make an npm package out of that concept?

ashaffer commented 8 years ago

@rstacruz You may want to take a look at state-lens and redux-ephemeral. The state lens thing is just sort of a rough idea, but it works.

rstacruz commented 8 years ago

@ashaffer, nice work on those! I like it. However, if you're using, say, Immutable.js or Baobab or Scour as your redux state, it isn't as easy to integrate.

ashaffer commented 8 years ago

@rstacruz Ya, I know that's a problem. I'm planning on allowing you to mixin getProp/setProp to redux-ephemeral to support that use-case. I'll try to get that up today or tomorrow.

rstacruz commented 8 years ago

@anthonyshort: the problem with initialState on onCreate in your example, though, is that it triggers after the first render... so your first render has no state.

  function onCreate ({ path }) {
    // you could even do a Component.initialState call here
    state[path] = {}
  }
rstacruz commented 8 years ago

Here you go!

https://www.npmjs.com/package/deku-stateful

Using it now... works like a charm. (I've also updated decca to be completely 100% compatible with deku's v2 API—that is, no states.)