composability-in-lifecycle-hooks.md

Logic composability problems of lifecycle hooks in React

Suppose I have these components in my project:

class MessageHeader extends React.Component { /* ... */ }

class NiceButton extends React.Component { /* ... */ }

class FridgeContents extends React.Component { /* ... */ }

All of these components are presentational, and take a timestamp: number as a props. I pass this timestamp prop to them, but only once, so basically it's like a constant.

Inside these components, I use the function humanTime which does a presentational thing: converts 1512767145540 to 2 minutes ago. I want to periodically recall this function so that it changes the rendering to 3 minutes ago and so forth. For simplicity, assume I'm just calling humanTime(123) inside render methods.

Solution zero

The dumbest way of solving this is implementing that logic in every component's lifecycle hooks:

 class MessageHeader extends React.Component {
   constructor(props) {
     super(props)
   }
  
+  componentDidMount() {
+    this.interval = setInterval(() => this.forceUpdate(), 30e3);
+  }
  
+  componentWillUnmount() {
+    clearInterval(this.interval);
+  } 
  
   render() {
     // calls humanTime(123)
     // ...
   }
 }
 
 class NiceButton extends React.Component {
   constructor(props) {
     super(props)
   }
  
+  componentDidMount() {
+    this.interval = setInterval(() => this.forceUpdate(), 30e3);
+  }
  
+  componentWillUnmount() {
+    clearInterval(this.interval);
+  } 
  
   render() {
     // calls humanTime(123)
     // ...
   }
 }
 
 class FridgeContents extends React.Component {
   constructor(props) {
     super(props)
   }
  
+  componentDidMount() {
+    this.interval = setInterval(() => this.forceUpdate(), 30e3);
+  }
  
+  componentWillUnmount() {
+    clearInterval(this.interval);
+  } 
  
   render() {
     // calls humanTime(123)
     // ...
   }
 }

Yes, this is a solution, but a pretty horrible one when it comes to DRY (Don't Repeat Yourself) and maintenance and legibility of purpose

Desired solution

I want to compose this logic into my three (or many) components, so I can simply write

const MessageHeader2 = withPeriodicRefresh(MessageHeader);
const NiceButton2 = withPeriodicRefresh(NiceButton);
const FridgeContents2 = withPeriodicRefresh(FridgeContents);

And then just use those components, which now have that setInterval/forceUpdate logic. I don't necessarily want to insert any child component, nor a wrapper component. I just want to add this logic into all those components. Mutating the input component is one solution, but that's not the best for composability and predictability.

So ideally, I just want MessageHeader2 to be a new component basically behaves like MessageHeader but has periodic refreshing built into it. It puts all that Solution Zero stuff into a utility and I reuse that. Simple.

Imperfect solution 1

Inheritance: MessageHeader2 extends MessageHeader. This works, but Java has taught us that at some point you will get stuck wishing to do multiple inheritance, which isn't reliable due to the Diamond Problem In Inheritance.

Imperfect solution 2

Wrapper component or higher-order component

class PeriodicRefresh extends React.Component {
  constructor(props) {
    super(props);
  }
  
  componentDidMount() {
    this.interval = setInterval(() => this.forceUpdate(), 1e3);
  }
  
  componentWillUnmount() {
    clearInterval(this.interval);
  } 
  
  render() {
    return this.props.render();
  }
}

And its usage:

<PeriodicRefresh render={() => <MessageHeader name={"World"} />} />,

This actually doesn't work because the MessageHeader may legitimately have an optimized shouldComponentUpdate:

class MessageHeader extends React.Component {
  constructor(props) {
    super(props)
  }
  
  // HERE
  shouldComponentUpdate(nextProps) {
    return nextProps.name !== this.props.name;
  }
  
  render() {
    return <h2>Hello {this.props.name} {humanTime(123)}</h2>
  }
}

Which blocks the forceUpdate from PeriodicRefresh because now there is a parent-child component boundary. This is all "logical" and abides by the design principles, but by introducing a parent-child boundary, I lose the capability of calling forceUpdate (which is to bypass shouldComponentUpdate) as a feature in my composable utility. So a wrapper component is not even a solution at all.

Imperfect solution 3

Introduce a new prop whatever to MessageHeader, the prop would cause the re-render. A wrapper component would pass this prop to the MessageHeader.

There are three problems with this approach:

• It requires us to update MessageHeader shouldComponentUpdate, which is not composability (see section "Desired solution")
• It still requires copy-pasting code to MessageHeader, NiceButton, FridgeContents
• It is not clear, to a colleague or to your future self, what is the prop whatever for, since its contents aren't actually used for the rendering

Imperfect solution 4

Recognize that Date.now() inside the library humanTime is the source of updates, extract that from the library, pull it all the way up the tree to somewhere in state management architecture or in props.

I can explain why that is an imperfect solution. To begin with, this whole problem is perfectly solvable by sprinkling setInterval with forceUpdate in lifecycle hooks (see Solution zero above). That's a low-cost solution, but it's a bad idea for code reusability and maintenance.

A rearchitecture like this solution num 4 involves insight into third-party libraries, rework of component hierarchy and their props, and/or rework near state management or controller components. That's a high-cost solution.

It should be cheap to do the right thing, and expensive to do the wrong thing, but in this case solution num 4 turns it around, and makes solution zero more attractive to developers. That's probably what often happens in the end of the day.

Imperfect solution 5

refs

function withPeriodicRefresh(Comp) {
  return class extends React.Component {
    constructor(props) {
      super(props);
    }
  
    componentDidMount() {
      this.interval = setInterval(() => {this.child.forceUpdate()}, 1e3);
    }
  
    componentWillUnmount() {
      clearInterval(this.interval);
    } 
  
    render() {
      return <Comp {...this.props} ref={child => this.child = child} />
    }
  }
}

This actually works.

But using refs is not recommended, said Andrew Clark on Twitter. https://twitter.com/acdlite/status/939219265332289536 I can understand how refs are "not so nice" for updating children, but in this case I didn't want to introduce children in the first place. One could say this is a perfectly valid use case for refs, but there is documentation-induced shame for using them, or just labeling this as an escape hatch.

Apart from the "against best practices" issues of using refs, there is also some subtle limitations: https://reactjs.org/docs/refs-and-the-dom.html#refs-and-functional-components

// This is a functional component, it doesn't support refs
function MessageHead(props) {
  return <h2>Hello {props.name} {humanTime(123)}</h2>;
}

const MessageHeader2 = withPeriodicRefresh(MessageHead);

Suddenly we stop losing predictability. Some components behave fine when we pass them to withPeriodicRefresh, and other components don't work at all.

Conclusion

So yeah, I am not aware of any solution for implementing withPeriodicRefresh(Foo) (from "desired solution" section) which ticks all these boxes:

• Works no matter what component you pass as input Foo (predictability)
• Requires no changes to the implementation of Foo (separation of concerns & unleaky abstraction)
• Requires no new prop on Foo (unleaky abstraction)
• Is cheap compared to solution zero

HOCs and forceUpdate feel like the wrong approach. Since the displayed time needs to get updated at an interval, it can be extracted to a component which manages updating itself. The updating part can be done by either forceUpdate or setState, where setState feels like the better solution https://twitter.com/satya164/status/939283974924337153

I would go with what @gaearon posted. But, if you really need this to work, and you don't have any control over the implementation of the "children" components, something like this should work:

function withPeriodicUpdate(Component) {
  return class PeriodicRefresh extends React.Component {
    constructor(props) {
      super(props);
      const wrapper = this;
      this.Component = Component;
      if (Component.prototype.shouldComponentUpdate) {
        const sCU = Component.prototype.shouldComponentUpdate;
        // Create a new component with a new sCU
        this.Component = class extends Component {
          shouldComponentUpdate() {
            return wrapper.willUpdate || sCU.apply(this, arguments);
          }
        };
        // TODO: hoist statics
      }
    }

    componentDidMount() {
      this.interval = setInterval(() => {
        this.willUpdate = true;
        this.forceUpdate();
        this.willUpdate = false;
      }, 1e3);
    }

    componentWillUnmount() {
      clearInterval(this.interval);
    }

    render() {
      return <this.Component {...this.props} />;
    }
  };
}

Not the most elegant of solutions, but ticks all of your items in the checklist and only adds overhead if it needs to (if sCU is implemented).

Easy
https://codesandbox.io/s/xr2n0oykyo?module=%2Fsolution.js

As the minimal refresh time is almost always constant and the same, we don't need to manage update of time-based components nowhere apart from components themselves. The only one foreign thing that they will need is time clock generator, and it definitely could be common for all such components.

The interesting conclusion is that our clock might come not only from props, thereby bypassing shouldComponentUpdate hook.

My solution is to use recompose and streams to delegate him all low-level logic and make the component in a few simple lines. Even more, it will not be blocked by shouldComponentUpdate and I proved this in a link above

In this example, I will use most but recompose can work even with your library xstream right out from the box

import { periodic, from } from 'most'
import { componentFromStream } from 'recompose'

const clock$ = periodic(1e3)

const LiveTime = componentFromStream(
  props$ => from(props$).combine(
    ({ component: C, ...props }) => <C {...props} />,
    clock$,
  ),
)

• Works no matter what component you pass as input Foo (predictability) My solution really don't care what to render
• Requires no changes to the implementation of Foo (separation of concerns & unleaky abstraction) Honestly, I added flow types to MessageHeader, shame on me
• Requires no new prop on Foo (unleaky abstraction)
• Is cheap compared to solution zero Expenses of man/hours are near zero, and even if you'll found performance issues, feel free to make PR to recompose and JSS

Thanks for the replies, I liked @nirrek's solution:

function withPeriodicRefresh(Comp) {
  return class extends React.Component {
    constructor(props) {
      super(props);
    }
  
    componentDidMount() {
-     this.interval = setInterval(() => {this.child.forceUpdate()}, 1e3);
+     this.interval = setInterval(() => {this.child ? this.child.forceUpdate() : this.forceUpdate()}, 1e3);
    }
  
    componentWillUnmount() {
      clearInterval(this.interval);
    } 
  
    render() {
      return <Comp {...this.props} ref={child => this.child = child} />
    }
  }
}

But obviously @satya164's solution is the right way, as it avoids forceUpdate and refs.

class HumanTime extends React.Component {
  constructor(props) {
    super(props);
    this.state = this.getStateFromProps(props);
  }
  
  getStateFromProps(props) {
    return {formattedTime: humanTime(props.time)};
  }
  
  componentDidMount() {
    this._timer = setInterval(() => this.setState(this.getStateFromProps(this.props)), 1e3);
  }
  
  componentWillUnmount() {
    clearInterval(this._timer);
  }
  
  render() {
    return this.state.formattedTime;
  }
}

Lessons learned and feedback

On one hand this is just a silly problem with a silly solution, but on the other hand, this is how outsiders (to the React community) or beginners might typically approach a problem, and my experience might provide feedback.

Here's what I learned from comments in Twitter:

render() function shouldn't call any special function, it should just use props and state

And as a feedback: that could be made more obvious through API design, instead of through community best practices. E.g. here is how I experienced the render() API:

// no arguments, okay, so it means I should use things from elsewhere, like `this` or closure
render() {
  // here is typically some empty space for me to fill in some code
  // maybe I could use this space
  return // ... finally, I must return react elements
}

The lack of arguments induced me to think that it's okay to use either imported functions inside render, as well as this.props etc.

Here is one way API design and documentation can psychologically push you to the right direction:

// Oh so this is a function with two arguments: props and state
// I should use those two things.
// Oh, it's an arrow function, I can't use `this`.
// Hmm, the right side of the arrow just gives some JSX, so it should be
// a simple mapping from props&state to JSX, nothing else
render = (props, state) => (
  <JSXHere />
);

So that if I would do

render = (props, state) => {
  // here is some space for other code
  return <JSXHere />
};

It would feel like a deviation of what I saw in the docs and it wouldn't feel correct. Also it costs me more typing, so it's hinting me that I'm deviating from the normal use case.

Lesson 2: you should really use setState, not forceUpdate

My experience was: I noticed humanTime() was the source of problems, but at my disposal I had some tools to solve it: componentDidMount (this is common stuff, feels correct to use it) and forceUpdate (looks like a short name, looks like what I need, the docs don't signal it with a big warning, so I'll use that) which led me to do

componentDidMount() {
    this.interval = setInterval(() => this.forceUpdate(), 30e3);
}

while thinking "that is perfectly acceptable use of the API". I wasn't aware that I was deviating from the normal flow of development. Perhaps if the name would be dangerouslyForceUpdate or shamefullyForceUpdate or forceUpdateIgnoringState or any kind of warning, then I would know that I'm deviating from well supported workflows. Interestingly, the API dangerouslySetInnerHTML and "_DO_NOT_USE_OR_YOU_WILL_BE_FIRED" and shamefullySendNext (from my library xstream) have worked really well to signal to developers that these are not good APIs. People have found other ways of solving their problems through the correct workflow.

So yeah, those are my two cents on how this stuff could be made better.

@staltz doesn't force already means it's dangerous like force push in git?

If you'd like the create stateful component using a more functional api, I'm working on React Stateful Component. It guides you into having a pure render function by avoiding the usage if this and by passing props and state as parameters. It's inspired on ReasonReact's api and essentially a lightweight wrapper around React's class components. Not entirely finished yet though.

@staltz <HumanTime time={time} /> also has another advantage. Depending on how far away props.time is from current time, you can set the interval accordingly.

If the difference is less than a few minutes, you can make the setInterval happen every 1 second. If it is more than 2-3 hours, you can make the setInterval execute every 30 minutes. If it is more than a day old, you can set a precise timeout of when to update, instead of setInterval. A post that was created '3 days ago' is going to change to '4 days ago' after a certain time, you can initiate setTimeout for that specific time.