Skip to content
This repository has been archived by the owner on Sep 21, 2022. It is now read-only.

Attempt to improve "Returning state" section #36

Open
Kureev opened this issue Jun 15, 2015 · 12 comments
Open

Attempt to improve "Returning state" section #36

Kureev opened this issue Jun 15, 2015 · 12 comments

Comments

@Kureev
Copy link

Kureev commented Jun 15, 2015

What about if we can get rid of this:

render(state = { count: 0 }, props = {}) {
  return (
    <div>
      <span>Clicked {state.count} times!</span>
      <a href="javascript:void(0)" 
        onClick={() => render({ count: state.count + 1, }, props)}>
        Click to increment!
      </a>
    </div>
  );
}
@jordwalke
Copy link
Member

jordwalke commented Jun 15, 2015

👍

The trick is that render only returns a data structure of React elements. Something somewhere needs to know to take the return value from onClick and reconcile it with the previous exact structure that was present for this particular instance.

I'm working on a solution to this problem right now, and I came up with an updater binder as the third argument to render.

render(state, props, updater) {
  return (
    <div>
      <span>Clicked {state.count} times!</span>
      <a href="javascript:void(0)" 
        onClick={updater((e, {props, state, updater}) => {count: state.count + 1})}>
        Click to increment!
      </a>
    </div>
  );
}

BTW: What syntax is that for default arguments?

@Kureev
Copy link
Author

Kureev commented Jun 15, 2015

@jordwalke Ah, got it. I also like your way with updater. My goal was to avoid using this and compose components from the export functions (but it's already in the section 👍 ).
So, I know it's too early to talk about it, but do you have any optimistic prognoses about implementing this?

About the syntax - it's in the ECMA-262 proposal

@chicoxyzzy
Copy link

what about context?

@Kureev
Copy link
Author

Kureev commented Jun 15, 2015

@chicoxyzzy I'd implement a context as one of the arguments:

render(state, props, ctx, updater) {
  //...
}

@chicoxyzzy
Copy link

IMO it's better to make context optional. Because of it's rare use. You don't need context if you don't have contextTypes for your component. BTW as I understand updater is also optional. So it might look like

render(state = { foo: 0, bar: 'hello' }, props = { length: 100500 }, context = null, updater = defaultUpdater ) {
  //...
}

@Kureev
Copy link
Author

Kureev commented Jun 15, 2015

@chicoxyzzy What means "optional"? If you want, you can omit all attributes, use it like this:

render() {
  return <div style={{ color: 'red', }}>*</div>;
}

If you're talking about order of arguments, then I guess state, props, updater, context

@chicoxyzzy
Copy link

I mean that it should be handful to omit some arguments (i.e. state, context, updater) if you don't need them in render function. Also I think order should be props first, then state, updater and context.

@kibin
Copy link

kibin commented Jul 5, 2015

I think that render should take an object in which you can pass all the parameters. That way you can easily omit all that you don’t need:

  render({ state, props, updater, context, whatever }) {
    // ...
  }

@FezVrasta
Copy link

I don't quite get what would be the purpose of the updater

@jordwalke
Copy link
Member

@FezVrasta It's kind of like doing .bind(this). With the returning-state pattern, there's no "this", and no reference to an actual component instance (which is actually one of the appeals). The updater(fn) event handler binder ensures that the state in the right location in the UI tree is updated with the return value of fn.

@FezVrasta
Copy link

FezVrasta commented Dec 25, 2016

How preact achieves it then?

Once you have this code, everything should just work:

render(props, state) {
  return (
    <button onClick={() => this.setState({ foo: 'bar' })>{state.foo}</button>;
  );
}

this is still used for setState, but who cares?

Still better than:

render() {
  const { foo } = this.state;
  return (
    <button onClick={() => this.setState({ foo: 'bar' })>{foo}</button>;
  );
}

@jordwalke
Copy link
Member

@FezVrasta With the this example you provided, there's some things that the updater pattern (imho) handles better. In your example, state can be read off of this, which implies that state must always be mutated to contain the new state. onClick={() => this.setState({ foo: this.state.foo + 1 })}

Mutation makes many things more difficult including (eventually) concurrency/parallelism. If you change your example slightly to use the alternative React setState API that accepts a callback, it's improved because it doesn't encourage relying on stale state fields that must be mutated in order to be kept up to date:

 <button onClick={
    (e) => this.setState((prevState, props) => ({ foo: prevState.foo + 1 })
  }>
    {foo}
  </button>;

This alternative React setState API has the benefit that the system can provide you the "freshest" props in the callback, which has many benefits. Then it becomes clear that this is actually very similar to the updater API. It has the same benefits such as not requiring mutation of this or this.state, and discourages trapping stale values in the event handling closure. updater just allows you to create your callback in this form (e, {props, state, updater}) => nextState which also would let you "chain" updaters by using the updater again even if the event handler was defined far away from the render function.

Note: in my original updater API I forgot to include the arguments to the callback (I just added them now).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants