React: `render` as a function of props and state

[chenglou]

This comes from my discussion with several people around animation, but I feel like it's important enough to warrant its own issue.

We already insist that render should be a pure function of this.props and this.state. How about taking this further and pass those in CompositeComponent? This way we can write:

render: function(props, state) {
  return <div>{props.something}</div>;
}

While most of the time we'll still call render(this.props, this.state) internally, with this we will be able to do render(somePrevProps, someTestState) from React or even from the user. Being able to "test the water" would be very valuable in the future. For example:

componentWillReceiveProps: function(nextProps) {
  var result1 = this.render(this.props, this.state);
  var result2 = this.render(nextProps, this.state);
  // Collection of changed stuff. Good for preparing mounting/unmounting animations or such logic.
  React.reconciler.getDiff(result1, result2);
  this.setState({...});
}

I've heard this can also solve some problems with pendingState? Not too familiar with the issue.
@jordwalke @sebmarkbage @petehunt

[sebmarkbage]

As you mentioned in the end. This API change shouldn't be considered in isolation, but with all other life cycle events and how this correspond to prev/current/next at every stage.

[sophiebits]

cf.

122 Pending state updates may be confusing

629 Store pending state updates directly in this.state

[syranide]

Though kind of unrelated, I personally would :+1 this in isolation, in a lot of cases this.props.whatever becomes quite tedious and a visual annoyance. Being able to avoid this. without having to declare them locally at the start of every render would be nice.

[sebmarkbage]

Note that there's a potential hazard here if this.state were the pending state. Callbacks that refer to this can be updated with new references locally, but closures cannot. Closures cannot get access to pending state. Likewise this issue can occur if we had partial reconciliation.

render() {
  return <div onClick={() => doSideEffect(this.state.x)} />;
}
// is not equivalent to
render() {
  var x = this.state.x;
  return <div onClick={() => doSideEffect(x)} />;
}
// or
render() {
  var state = this.state;
  return <div onClick={() => doSideEffect(state.x)} />;
}
// or
render(state) {
  return <div onClick={() => doSideEffect(state.x)} />;
}

I think that it's possible to have the same issue with this.props and this.refs but don't have a case off the top of my head.

[syranide]

@sebmarkbage Oh yeah you're right, at least if we allow the user to replaceState the object itself (like we already do I assume), which is neat in that whatever the user provides it gets (like numbers). Also yeah, the exact same thing would be true of props if we don't reuse the same object.

[YurySolovyov]

+1 on that, what are general arguments against passing props and state to render function?

[nhunzaker]

+1 from here too. I really wish the drawbacks about pending state weren't an issue. Though my desires for this are purely aesthetic.

Edit: Though additionally, it would make PureRenderMixin more declaratively accurate in its name

[nhunzaker]

Maybe it's a terrible idea, but what if you passed this? Then you could use destructuring to get something similar:

render({ props, state }) {
    // do stuff
}

Maybe this is worse, but I wanted to get the idea on the table

EDIT: actually this works for me already:

render({ props, state } = this) {
    // do stuff
}

but that's hardly fantastic, I think

[chenglou]

Is this still alive? @sebmarkbage you can just pass a state reference to the callback and tell them to use that instead right? That's what tween-state does. Breaks a lot of code though.

[alexpods]

If you want to pass props and state to the render method, then I think you must pass context too. In this case It will look a little weird to me. For example if someone uses only context property:

// You must write `pass` and `state` arguments although you aren't using them.
render(props, state, context) {
    return <span>{context.name}</span>
}

[probablyup]

I'd love to get this conversation going again. It's conceptually very clean to provide render(props, state, context). context perhaps could be dropped, but if there is a plan to majorly change what props and state are at this point, it would break every React install out there... so this seems like a safe enhancement.

I'm +0 on context, because it's not a well-documented API and adding it means the team would have to fully commit to it.

As to the comments above, state should be whatever you would be able to access at the time by using this.state - so it would not include pending state transactions.

[syranide]

@yaycmyk It's a bad idea in some cases because you're then referencing props and state from a certain point in time and not necessarily what they are currently set to. https://github.com/facebook/react/issues/1387#issuecomment-41931037

[probablyup]

@syranide Hmm, yeah but that's a matter of JS knowledge about referential equality.

[chenglou]

@yaycmyk it's not about referential equality. It's just reference. Pointing to an old copy gives you stale data no matter what kind of equality you're using.

[probablyup]

My comment was that it would require the implementor to know that a cached version of an object in a variable may not reflect the current "version" of that object, hence the referential equality statement.

[jimfb]

@yaycmyk Yeah, let's call that "data management". The term "referential equality" implies something very specific, and this is not that.

[probablyup]

Fair enough.

[nippur72]

I would like to add that preact already implements this feature. They say it leads to

• easy destructuring
• easy to switch from stateless to stateful component

[steve-taylor]

Haven't tested this, but it might work:

Higher order component:

const preactify = (component) => class extends component {
  render() {
    return super.render && super.render(this.props, this.state, this.context);
  }
};

or component base class:

class BaseComponent extends React.PureComponent {
  render() {
    return super.render && super.render(this.props, this.state, this.context);
  }
}

I'm not really a fan of deep inheritence hierarchies. This just demonstrates that you can fairly easily get around this feature not (yet) existing in React. You could just as easily wrap and delegate, although it would be a bit more verbose and less performant.

I'm all for React changing to pass props, state, and context to the render function. It's consistent with functional components and doesn't look like it would break backwards compatibility.

Happy to create a PR when I get some time.

[syranide]

@steve-taylor https://github.com/facebook/react/issues/1387#issuecomment-191280035

[steve-taylor]

@syranide render shouldn't have any side effects. As long as you don't break the rules, you're safe. In terms of event handler callbacks, they will be recreated each render cycle, so their reference to props, state, and context should always be current. If you have an inline handler that does something like setTimeout or a web service call with an inline callback (which will be executed in subsequent ticks and, potentially, subsequent renders), then you're doing something wrong and should, at the very least, stop inlining such handlers. (Besides, when you have handlers that do something in a subsequent tick, you should abort them in various lifecycle handlers in case you do something a bit stupid like calling setState on an unmounted component or let stale web service callbacks pile up.)

[ngryman]

Please consider this addition, as long as the documentation properly warns about the difference of referencing this.state vs the state argument, I think it could bring some valuable sugar.

Let me sum up the pros / cons:

Pros

1. It removes code clutter in every stateful render function.
2. It makes super easy to transform a stateful -> pure or pure -> stateful component.
3. (IMHO) It makes the API consistent with some lifecycle events.

Cons

1. Decouples the render-time state with the current state.

I tend to be agree with @steve-taylor, and would say that render should not have closures but be pure. It's all about documentation and compromises here. If the docs specify clearly that closures in render is a bad practice but also explain the difference between referencing this.state and state argument, I think it would make everybody happy. Don't forget Pareto here 🤓

[jamesplease]

In the first comment, this snippet is suggested as a use case for this API:

componentWillReceiveProps: function(nextProps) {
  var result1 = this.render(this.props, this.state);
  var result2 = this.render(nextProps, this.state);
  // Collection of changed stuff. Good for preparing mounting/unmounting animations or such logic.
  React.reconciler.getDiff(result1, result2);
  this.setState({...});
}

I’m not sure that this would work if the idea here is that this would be internal React code. Developers should still be able to use this.props inside render rather than the props argument, right?

I don’t think it’s a good idea to assume that the first argument to render is how the developer is accessing props inside of render. The documentation could strongly recommend it, but it still seems like an odd restriction to place in render. If that example code was for app code, though, then never mind!

[gaearon]

This snippet was meant as an example of the app code. It’s three years old though and I don’t think it reflects our thinking :-)

[jamesplease]

Thanks for the clarification @gaearon :v:

[dispix]

My question after reading this thread is: should destructuring props (i.e. const { users } = this.props at the beginning of the render method) be considered bad practice ?

From what I read const { users } = this.props has no absolute guarantee to be the same as this.props.users, but there is a lot of examples that use this as a way to make the code more readable so I'm kinda lost on what's the best practice here.

[ivan-kleshnin]

@dispix const {users} = this.props produces the same users as this.props.users. Always.

[dispix]

My bad, I assumed this comment was also applicable to props. Although, is it a bad practice to write const { users } = this.state ?

[ivan-kleshnin]

@dispix I write let {x, y, z} = foo all the time because it doesn't take a new line to add a new variable.
Saying that - both your questions are really about JS and don't belong here.

[LandonSchropp]

This approach creates a nice symmetry with functional components.

function Person({ name }) {
  return <li>{ name }</li>;
}

class People extends React.Component {
  render({ people }) {
    <ul>
      { people.map(Person); }
    </ul>
  }
}

In this example, Person and People share very similar interfaces. This makes it easier to mix stateful and stateless components, as means less work if you decide to convert one type of component to the other.

[jasford]

In this example, Person and People share very similar interfaces. This makes it easier to mix stateful and stateless components, as means less work if you decide to convert one type of component to the other.

Yes! I often start with a stateless component function and then later find the need to convert it to a stateful component class. The find/replace for this.props becomes a bit tedious. Being able to write the render() method of a component class using the same interface as a stateless component would be super nice.

[gaearon]

See the discussion of this in https://github.com/reactjs/rfcs/pull/4.
I left a response on https://github.com/reactjs/rfcs/pull/4#issuecomment-412637588.

We understand why it’s inconvenient and we’re going to address it. However, we won’t use this particular solution.

[gaearon]

(in response to a deleted comment about using a Babel transform)

While I understand why you’re doing it I strongly recommend against using a plugin like this because it changes the semantics of otherwise valid JavaScript code. That tends to cause many problems over a longer term: the code breaks in different environments, you can’t copy-paste it to other codebases, it’s surprising to new team members, etc.