props-changing.md

Suppose a parent renders <ChildComponent callback={(value)=>setState(value)} />. Suppose the parent re-renders. The pointer values of the callback prop will be different (ie. not triple-equals-equal) but the callback prop's value is conceptually the same for all intents and purposes. This is a "new value" despite being the "same value". You run into the same problem when the parent creates an object <ChildComponent data={{foo: 'bar', bar: 'noise'}} /> (pointers differ, despite it being the "same" object from a value-type perspective).

You also run into the reverse problem due to mutability. Suppose I say:

var value = {foo: 'bar', bar: 'noise'};
ReactDOM.render(<ChildComponent data={value} />, ...);
value.bar = 'drinks';
ReactDOM.render(<ChildComponent data={value} />, ...);

As you can see, the props have clearly "changed" (ie. componentWillReceiveProps should get called, so the ChildComponent can respond accordingly), but the values are triple-equals-equal.

In general, there is no way to solve this, except to always call componentWillReceiveProps any time the values might have changed.

If javascript always used value types (like they do for integers; two renders of integers or strings are always "equal" if they are equal from a value-type definition) for all data types, than this problem would not exist. Also, mutation creates an issue, because the meaning of a value can change (when it is mutated) but is still triple-equals equal to the original prop. But because javascript uses reference equality and supports mutation, you can't rely on triple-equals to tell you if two things are conceptually the same.

I believe the counter argument is around efficiently in checking to see if things actually changed (eg. focussed on the example of simplifying the API around a past conditions) but that the counter argument is not against the principles in general (an example would help in clarifying why the principles are not possible to achieve under all circumstances).

Let's remove the past tense in the exercise (eg. propsChanged vs receiveProps) and go back to the principles (and adding C raised by @akinnee-gl in facebook/react#3279):

(A) simplify code structure / reduce boilerplate
(B) keep props (inputs) separate from state (internal)
(C) allow for shouldComponentUpdate to control the rendering of a component

In this solution space (or a refined / augmented one), is there a way to achieve these principles?

For example (called with an identical signature both at initialization and with any prop changes to reduce code paths):

receiveProps(prevProps) { // or maybe receiveProps(nextProps)
  if (!shallowEqual(_.pick(prevProps || {}, KEYS1), _.pick(this.props, KEYS1)) { // changed
   // do something
  }

  if (!shallowEqual(_.pick(prevProps || {}, KEYS2), _.pick(this.props, KEYS2)) { // changed
   // do something
  }
}

@kmalakoff Efficiency is an issue, but correctness is also an issue. It's physically not possible, due to javascript's language constraints, to do the equality checks that would be necessary to only call propsChanged if the props actually changed (as per above) - that's all I was saying in my comment in facebook/react#3279.

(A) is the objective, but it's not clear (yet) what the right solution is. That's the whole topic of facebook/react#3279. The first step to fixing a problem is understanding the problem/usecases.
(B) I don't see any win to separating them, if propsChanged still doesn't allow you to assume that the props changed. What is the point in separating them, if you need to do the did-change check anyway?
(C) That's already what shouldComponentUpdate is intended for; presumably it already (mostly) does that.

@jimfb thank you for helping move this forward!

---

(A) is the objective, but it's not clear (yet) what the right solution is. That's the whole topic of facebook/react#3279. The first step to fixing a problem is understanding the problem/usecases.

Makes sense. I've been contributing to the issue to promote an environment to help surface the problems / use cases, but also to see if there is a different way of thinking of the solution space in case people are anchoring too much on the current API. As an exercise, I've been thinking that formalizing and agreeing principles could create new insights on the problems / use cases.

From my own code, I documented the type of control flow pattern that the current API creates and I'm hoping to streamline it to a flow with less boilerplate. Repeated here:

componentWillMount() { this.actuallyCheckThePropsAndMaybeDoSomething(); }
componentWillReceiveProps(nextProps) { this.actuallyCheckThePropsAndMaybeDoSomething(nextProps); }

actuallyCheckThePropsAndMaybeDoSomething(props) {
  props = props || this.props;

  let relatedProps1 = _.pick(props, KEYS1);
  if (!shallowEqual(this.relatedProps1, relatedProps1) { // changed
   this.relatedProps1 = relatedProps1;

   // do something
  }

  let relatedProps2 = _.pick(props, KEYS2);
  if (!shallowEqual(this.relatedProps1, relatedProps2) { // changed
   this.relatedProps2 = relatedProps2;

   // do something else
  }
}

With this sort of use case in mind, I have tried to summarize some potential principles that may help resolve the issue.

---

(B) I don't see any win to separating them, if propsChanged still doesn't allow you to assume that the props changed. What is the point in separating them, if you need to do the did-change check anyway?

I explained the reasoning for separating props and state here:

As for (B), the main problem with the componentDidUpdate is like was just mentioned is that you can trigger repeated method calls if you set state due to property changes since it is called for both props (inputs) and (internal) state. These code paths seems to be better as decoupled because props are supplied from the outside and state is set in the inside, eg. I've tried / considered a fourth possibility of updated(prevProps, prevState) (a simplified name for componentDidUpdate since fewer lifecycle methods could allow for shorter naming) using componentDidUpdate to reduce boilerplate, but I find myself a bit dissatisfied with the potential redundant second update calls and that the logic seems to be quite independent in practice.

This principle should be looked at independently from the propsChanged example which you have successfully argued against.

As a clarification, I tried using componentDidUpdate, but found that the flow in practice naturally separated since one portion of logic dealt with external inputs (props) and the other changes in internal state (state). Think of it like cascading state changes...changes to the props may force the state to reset or be translated into basis state (inputs-phase) which in turn may cause derived state to update (derived-state phase).

I definitely agree that looking at use cases around having a single method for state and props could be a valuable exercise. Also, a single method definitely has the potential to be better since it is more flexible. Finally, the argument (taken from previous issue comments) around componentDidUpdate having the potential to be called multiple time in response to it's own changes would still happen if the state is set in the state-centric method (that said, looking at my current codebase, I have only exclusively used the props to change state since multiple state attributes can be updated simultaneously at the source of the change, eg. maybe the API discussion should mainly focus on props event if this is a potential principle).

---

(C) allow for shouldComponentUpdate to control the rendering of a component

I added this principle given that it is important to keep in mind in exploring solution space options. I also had taken it for granted (eg. implicitly included) but I made it explicit because @akinnee-gl explicitly raised it when clarifying the reasons against focussing on considering the updated stage to simplify the lifecycle hooks.

---

Do you believe that clarifying principles in addition to raising problems / use cases has good potential to move this forward?

If so, is there an API design that better achieves these principles? or do the principles need to be updated? or is there a line of argument that explains that we should only be looking at sustaining vs also considering disruptive (to draw on an innovation framework) API changes?

I might add one more principle (I was also using it implicitly):

(D) simplify method naming

My thinking is that the names are very verbose and so I end up copying any pasting instead of typing which signals that there could be room for improvement. Since we are moving towards an ES6 world and there are compilers like Babel, let's assume the use of React.Component and simplify the naming.

Here are some proposals to consider:

(1a) Very minimal API - it might be clearer to use the word render instead of update:

willMount()
didMount()
receivedProps(prevProps | null)
shouldRender(nextProps, nextState)
render()

(1b) Very minimal API:

willMount()
didMount()
receivedProps(prevProps | null)
shouldUpdate(nextProps, nextState)
render()

(2a) Slightly more expanded API - it might be clearer to use the word render instead of update:

willMount()
didMount()
receivedProps(prevProps | null)
shouldRender(nextProps, nextState)
beforeRender(prevProps | null, preState | null)
render()

(2a) Slightly more expanded API:

willMount()
didMount()
receivedProps(prevProps | null)
shouldUpdate(nextProps, nextState)
didUpdate(prevProps | null, preState | null)
render()

I'm actually not sure if the other methods are necessary so it should be analyzed and based on the findings, the principles could be updated.

(D) simplify method naming

Please see reactjs/react-future#40 (comment) and reactjs/react-future#40 (comment). For the purposes of this discussion, we should use the full/complete/existing lifecycle names; otherwise everyone picks their own names for their methods in their comments, and it becomes literally impossible to follow the discussion.

Overall, I think you're trying to squeeze too many changes into a single issue. For instance, the lifecycles take in props+state in case you want to compare your props to your state in order to determine something (as people clearly do in the issue facebook/react#3279). You're welcomed to continue to think about these things, but note that a lot of design went into these APIs, and there are several concerns that you've been overlooking in your proposals (like the value-type and mutability points, or the reasons that the lifecycles often take multiple parameters, etc - primarily because you don't know the history of these design decisions). Since we've completely covered the original point of this gist, I'm going to bow out of this discussion here, but feel free to continue the discussion here for the benefit of others. We'll continue to watch these threads and maybe jump in from time to time.

@jimfb I believe this gist was most likely started due to a misunderstanding in terms of the spirit of the line of solutions that I was trying to put forward to solve the issue. My point was not strongly rooted in having an update method added (it was just an example around consolidating the prop methods), but more importantly I was trying to move the discussion away from being anchored or overly-focussed on defending the addition of a new method to considering a more broad set of solutions (such as reducing multiple code paths, standardizing the method signatures, replacing the current prop method with another, etc) and to include the feedback from other contributors into the principles that could be used to evaluate solutions.

I took the time to explain to you in this gist why focussing on the value-type and mutability points and focussing on the update method example was a bit of a red herring (choosing the update lifecycle hook was not a significant point in my analysis) and I took this opportunity to put forward how other improvements triggered by the prop API improvements might fit into the bigger picture since it is quite possible that the best solution may in fact lead down the path of a larger change.

I acknowledge that my understanding of all of the issues that went into the React API is not as deep as people closer to it, but on the other hand, I am a very experienced software architect and open source library maintainer who deals with API design tradeoffs on a regular basis and I've been using React as a user for a while now so I have been considering what changes could be made to improve productivity.

It is a bit of a shame that the approach taken by the Facebook team has been to narrow the potential solution space being considered for resolving the issue and to classify related considerations as being out of scope. Unfortunately, it feels like it has been and will be a poor ROI for my time to continue with this discussion under these circumstances so I will also bow out.

At least there is a record of principles that the React team could consider. As you can see from these principles and potential solution that I have tried to promote discussion on, I believe the React API can and should be simplified. I hope you all consider ways to simplify the API and to make it even better for all!