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

Returning actions vs. imperative dispatch #365

Open ashaffer opened 8 years ago

ashaffer commented 8 years ago

We discussed this briefly in the 2.0 release thread but i'd like to bring it up again so that other people can chime in if they want.

Example:

Imperative dispatch:

function render (props, {dispatch}) {
  return <div onClick={() => dispatch(increment())}>Increment</div>
}

Returning actions from events / lifecycle hooks:

function render () {
  return <div onClick={increment}>Increment</div>
}

The return value of all event handlers and lifecycle hooks is passed to dispatch automatically, and your components don't ever even need to know about dispatch.

I do understand the perspective that it might create a more difficult learning curve, but IMO deku is already for people looking for the next step up in conceptual purity and simplicity from React, and so should be more tolerant of abstract concepts.

If you give components access to dispatch directly, your event handlers and lifecycle hooks can become stateful, long-lived, and harder to test.

E.g.

function handleClick (dispatch) {
  setInterval(() => dispatch(updateAnimation())
}

Or

function handleClick (dispatch) {
  return () => {
    while (someStatefulCondition())
      dispatch(action())
  }
}

These things are hard to reason about and hard to test and right now are the sole source of functional impurity in deku. If you got rid of imperative dispatch, components would be 100% functionally pure / referentially transparent.

This would also make component composition / mixins more powerful, elegant, and idiomatic. If you wanted to proxy the actions emitted by a component in a mixin, right now you'd have to do that by wrapping the component and swapping out dispatch, which is not really the standard definition of composition. If you use return values, you can compose event handlers / lifecycle hooks in a more natural way.

Deku 2.0 is really nice and i'm considering using it instead of virtex for the pending rewrite of our app at my work, but this seems like a critical issue to me.

rstacruz commented 8 years ago

on a lateral note, what's the redux-y way of issuing AJAX requests? I currently have something to the effect of:

function fetchData () {
  store.dispatch({ type: 'data:fetch:start' })

  request('/data.json')
    .then((data) => store.dispatch({ type: 'data:fetch:done', data })
    .catch((error) => store.dispatch({ type: 'data:fetch:errror', error })
}

...which obviously won't work with the "returning actions" paradigm.

11111000000 commented 8 years ago

@ashaffer: Can you give us example repo with that approach? @rstacruz: maybe with redux-effect?

anthonyshort commented 8 years ago

I do really like that approach. It makes things really simple and pure. The only concerns I had with going that way way:

One really good thing about doing this is that it would be nearly impossible for someone to do something stupid in their component. It pretty much enforces best practice.

Could we implement both? It seems like that could be harmless and allow some testing.

ashaffer commented 8 years ago

@rstacruz I use redux-effects for composing and managing side-effects in a pure way. For http requests specifically, there is redux-effects-fetch.

The primary redux-effects repo is just the thing that orchestrates your side effects, and it can be swapped out with alternatives if you don't like the bind syntax it ships with, such as redux-gen, redux-flo, or redux-saga (using redux-effects with redux-saga, is, imo the most principled possible approach - but does introduce an additional layer of indirection which i'm not sure is necessary).

@11111000000 If you want to see some examples, check out the examples in vdux, a virtual dom library / micro-framework I created that was heavily inspired by deku, virtual-dom, and redux.

@anthonyshort Implementing both sounds great to me. At least in the beginning - I think once you want to start growing a reusable component ecosystem it probably makes sense to standardize on one or the other, but for a while I think both would be just fine.

As for making things difficult later - I think there is some conceptual trickiness there, however using something like redux-effects or redux-saga, you should be able to compose arbitrarily complex operations that are triggered by the actions you return, and have all that stateful messiness live inside your redux middleware.

queckezz commented 8 years ago

@11111000000

Using redux-effects and redux-effects-fetch you can define your actions declarative without imperative dispatching of actions.

import { bind } from 'redux-effects'
import { fetch } from 'redux-effects-fetch'
import { createAction } from 'redux-actions'

const gotData = createAction('data:fetch:done')
const setError = createAction('data:fetch:error')

const beforeMount = () => {
  // returns plain old object
  return bind(
    fetch('/data.json'),
    ({value}) => gotData(value),
    ({value}) => setError(value)
  )
}

const reducer = handleActions({
  [gotData]: (state, data) => { ...state, data },
  [setError]: (state, error) => { ...state, error: error.message }
})

redux-effects will take care of when to dispatch the next action via composition defined in the action. So the action returned from beforeMount will look something like this:

{
  type: 'EFFECT_COMPOSE',
  payload: {
    type: 'FETCH'
    payload: { url: '/data.json', method: 'GET' }
  },
  meta: {
    steps: [
    [
      // success
      ({value}) => gotData(value),
      // failure
      ({value}) => setError(value)
    ]
  }
}

•The thing @rstacruz mentioned. It would push a lot of that stateful logic outside of the component, but I was unsure about it leading to a dead end later that would make something else difficult. This is mostly because I don't have enough experience using Redux for side-effects.

Well I sort of think the same way. I think sometimes you just can't always synchrounously return actions. I had a discussion with @ashaffer a couple of days ago about requestAnimationFrame(). It's not possible to return something while in a raf but you still may want to update state. Well maybe it is, but I haven't figured it out yet. I'll just don't know how it can be done declaratively. So dispatch seems the best option here for me personally.

const frame = null

const loop = dt => {
  const selectedState = select(getState())

  if (isEmpty(selectedState)) {
    frame ? cancelAnimationFrame(frame) : return
  }

  // No way I can return here
  dispatch(update(
    dt,
    selectedState, 
    renderContext
  ))

  frame = requestAnimationFrame(loop)
}

loop()

I think the best option is to support both but encourage declarative dispatching over the imperative one.

ashaffer commented 8 years ago

Well I sort of think the same way. I think sometimes you just can't always synchrounously return actions. I had a discussion with @ashaffer a couple of days ago about requestAnimationFrame(). It's not possible to return something while in a raf but you still may want to update state. Well maybe it is, but I haven't figured it out yet. I'll just don't know how it can be done declaratively. So dispatch seems the best option here for me personally.

Using redux-effects-timeout the return value of your raf will be dispatched back into redux. E.g.

import {raf} from 'redux-effects-timeout'

function handleClick () {
  return raf(doSomethingLater)
}

The return value of doSomethingLater will get dispatched back into redux. The issue we were having in your thread had to do with the fact that I wasn't dispatching virtex's local actions back through the middleware stack like normal actions, which caused them not to work correctly with this approach. But that problem is unique to what i'm doing in virtex-local.

ashaffer commented 8 years ago

I should also add that redux-multi makes this approach much more manageable, by allowing you to return multiple actions as an array, e.g.

import {bind} from 'redux-effects'
import {fetch} from 'redux-effects-fetch'
import {createAction} from 'redux-actions'

function render () {
  return <button onClick={fetchItems}>Fetch items</button>
}

function fetchItems () {
  return [
    fetchingItems(),
    bind(fetch('/items'), receivedItems)
  ]
}

const fetchingItems = createAction('fetchingItems')
const receivedItems = createAction('receivedItems')
Yomguithereal commented 8 years ago

The nice thing with the current dispatch is that one does not have to follow the redux action way. For instance, in #362, I need the dispatch function to be able to receive as many arguments as needed because it suits my way of handling action better and untie my reducers from a centralized handler (which is harder to hot-reload and is not really efficient).