acdlite / recompose

A React utility belt for function components and higher-order components.
MIT License
14.76k stars 1.26k forks source link

withController/withViewModel #543

Open cztomsik opened 6 years ago

cztomsik commented 6 years ago

What do you think about something like this?

// alternative to withStateHandlers()
const withController = (ctrl) =>
  (Comp) =>
    class extends React.Component {
      constructor(props) {
        super(props)
        this.inst = Object.create(ctrl)
        this.inst.init(props)

        this.actions = {}

        for (let k in ctrl) {
          this.actions[k] = (...args) => {
            ctrl[k].apply(this.inst, args)
            this.forceUpdate()
          }
        }
      }

      render() {
        return <Comp {...this.props} {...this.inst} {...this.actions} />
      }
    }

const enhance = withController({
  init({initialCount = 10}) {
    this.count = initialCount
  },

  dec() {
    this.count--
  },

  inc() {
    this.count++
  }
})

const Counter = ({count, dec, inc}) =>
  <div>
    {count}
    <button onClick={dec}>--</button>
    <button onClick={inc}>++</button>
  </div>

export default enhance(Counter)

the idea is:

yes, it's not idiomatic react code, but it's very clear what is going on - it almost feels like poorman's mobx

EDIT: maybe it could accept class, so it would look even more angular/vue/polymer-like

istarkov commented 6 years ago

Hi thank you,

What the reason to hold state in this and use forceUpdate instead of state? It doesn't look good, see React documentation

Normally you should try to avoid all uses 
of forceUpdate() and only read from this.props 
and this.state in render().

Also as I know such hacks will be a problem in a near future, as all the state out of React state will not be supported by some new features (like memory management etc)

How it differs from withStateHandlers - removal of one argument in favour of this? It gives shorter method description but longer props accessor.

Also I don't like to say words like declarative - imperative
but all that this.blabla = blublu looks very imperative ;-) for near functional library.

So at a first sight we will not accept this.

cztomsik commented 6 years ago

because it's very clear what's going on, unlike withState/withStateHandlers

immutability is great to certain degree, but forcing it leads to unmaintainable mess (compare decent redux vs. mobx applications)

istarkov commented 6 years ago

Be clear in your words - very clear for ... ? For you - yes, for me - no, for other peoples I don't know ;-) I didn't use mobx but used redux a lot in very big projects and saw no unmaintainable mess so I don't understand what are you talking about. I see nothing bad in immutability and for me it forces you to write more readable and maintainable code, forcing to use immutability is in a way of react.

cztomsik commented 6 years ago

I really don't want to get more into this, but... Try to do doubly-linked list with immutable structures (it's hard). Or try to update object referenced from other objects, it's near to impossible.

Obviously, it's not great fit for everything, and please don't get me wrong, immutable structures are great for a lot of things (query builders) but I've done quite a lot of applications (one with hundred different screens, every with its own unique stateful logic) in my career and I know that mutable state is not something we should be blindy afraid of.

React is not strictly immutable, vdom is. MobX is also quite a lot about mutable state and it's usually used with React

And I don't think recompose is strictly functional either - being utility belt alone does not imply anything like that.

mpyw commented 6 years ago

Mutability is sometimes required because of constraints on the implementation. (e.g. Animated Component in React Native)

PinchGestureHandler · React Native Gesture Handler

Immutable animation causes serious impact on React Native.