Skip to content

Instantly share code, notes, and snippets.

@jlongster
Last active May 7, 2021 17:58
Show Gist options
  • Star 18 You must be signed in to star a gist
  • Fork 1 You must be signed in to fork a gist
  • Save jlongster/febd2a397aff9501abec0c2d66075ec8 to your computer and use it in GitHub Desktop.
Save jlongster/febd2a397aff9501abec0c2d66075ec8 to your computer and use it in GitHub Desktop.

3 Gripes With React

I started using React 3.5 years ago, and I still love it. It was such a well-designed solution that not much has changed since then, only superficial stuff like naming. What I learned then is still wholly applicable today because it's such a good idea (although now you can choose from many other libraries). On top of that, we now benefit from an entirely new architecture (fiber) without changing much.

However, nothing is perfect. There are a few things I keep hitting as I work in large, complex codebases. React probably has other weak spots, but these are what I keep hitting.

Opaque references

React allows you to pass a ref property to a component to get the instance when it's created. When it's a native DOM element, like an input, you get the raw DOM element. When it's a React component, you get the React instance.

The problem is that refs are special properties and do not actually exist in the props of a component. Let's say you are wrapping an input to make a custom Input component. If you pass down the props like <input {...this.props} />, the ref is not passed down, which means when I use <Input ref={...} /> I will get a React instance, not the DOM element.

This is probably the correct default, as developers most likely don't mean to pass down ref and consumers of a component would accidentally get the wrong thing (causing very confusing bugs). However, I find myself needing to expose an innerRef property quite often that I manually pass down, allowing the consumer to bypass my wrapper and get the internal instance itself.

This is needed for Input because I want the user to use it like a native DOM element, but ref is broken for that case as they won't get an input element. Users need to use <Input innerRef={...} />.

The biggest frustration with this is when using somebody else's component that doesn't expose something like innerRef, but I need it. This is common when I need to do something special the div that it creates, but I don't have access to it.

I don't know what the solution is, but we should thinking about a way for React to help facilite this.

PureComponent, but only for props

Pure components are a tradeoff: they allow you avoid unnecessary rerenders, but at the cost of a shallow check of props and state. For fast components this shallow check may actually hurt performance.

I almost always find myself using pure components to guard against incoming prop changes only. If the state changes it should always rerender. I wonder if there's value in a PureComponent that only checks props (something like return shallowEquals(this.props, nextProps) || this.state !== nextState).

This would cut the time for shouldComponentUpdate in half on average and make it less of an issue (you can use it more freely without worrying about the overhead).

This could be something done entirely in user-land, nothing should change within React itself.

Complicated lifecycle hook componentWillReceiveProps

Probaby the most time-consuing grunt work is managing state changes in componentWillReceiveProps. There are probably better ways to manage this state, but it's easy to fall into this:

class MyComponent extends React.Component {
  constructor(props) {
    super(props);
    this.state = { messages: getNewMessages(props.messages) }
  }

  componentWillReceiveProps(nextProps) {
    // Make sure to process the messages from nextProps and
    // update the local state
  }
}

Maybe I need to step back and find a better pattern for this. But even if I did, my teammates will most likely write code like this. It's the most straight-forward at first, but it's hard to maintain. In real apps, the code for deriving the state is a lot more complicated, and what do to next in componentWillReceiveProps may even depend on the state of the component itself.

For example, I have an app that renders a list of messages and automatically display the oldest unread message. When all messages are read, a "all caught up!" message is shown. When new messages come in, I automatically show the oldest new message, but only if the user is on the "all caught up!" screen. Otherwise don't change anything.

This app-specific logic is inherently complicated; but I find myself mixing in this logic inside of componentWillReceiveProps with other code concerned with specific state changes and things to avoid performance pitfalls.

Maybe we need clearer guidelines for how to structure state transitions, because in those components it doesn't feel as simple as "f(state) -> UI`. We might already have those guidelines but I haven't read them!

This is just a braindump of things I've been thinking of improving. It may not be coherent (particularly the last one), but let me know if you have any ideas. It's safe to say that if these are my only gripes with React, it's a pretty damn good library.

@mlrawlings
Copy link

@jlongster I'm enjoying the microblogs 👍

Regarding componentWillReceiveProps, typically I use the render function to handle derived data:

class MyComponent extends React.Component {
  render() {
    var newMessages = getNewMessages(this.props.messages);

    return (
      <div>{newMessages.map(message => (<li>{message.text}</li>))}</div>
    );
  }
}

I only reach for componentWillReceiveProps when I need to do some imperative action based on incoming props (something often necessary when wrapping a vanilla js library as a React component).

If there becomes a performance concern with deriving the data as a part of rendering, you could look into memoizing the function or a flux-like model (redux). From the flux overview:

We originally set out to deal correctly with derived data: for example, we wanted to show an unread count for message threads while another view showed a list of threads, with the unread ones highlighted. This was difficult to handle with MVC — marking a single thread as read would update the thread model, and then also need to update the unread count model. These dependencies and cascading updates often occur in a large MVC application, leading to a tangled weave of data flow and unpredictable results.

@radubrehar
Copy link

radubrehar commented Oct 19, 2017

@jlongster thanks for microblogging.

I find the componentWillReceiveProps approach redundant - unless i'm missing something.
What I do is either have the messages in state or in props, but never mix the two - so controlled vs uncontrolled.

class MyComponent extends React.Component {
  constructor(props) { 
    super(props)
    this.state = {
       messages: props.defaultMessages || []
    }
  }
  render() {
    const messages = this.props.messages != null ?
        this.props.messages :
        this.state.messages

    return (
      <div>{messages.map(message => (<li>{message.text}</li>))}</div>
    );
  }
}
<MyComponent  /> // uncontrolled, no default messages 
<MyComponent  defaultMessages={[...]}/> // uncontrolled, with some default messages 
<MyComponent  messages={[...]}/> // controlled, with initial messages

@asantos00
Copy link

@mlrawlings even for those imperative actions your would normally use the recommended componentDidUpdate (cWRP shouldn't have that kind of side effects). I like the fact that you re using a function to manage the data, but don't like receiving it as a prop. This way you're passing "inner component" things to the outside (whoever defines the callback) should the outside component know how to format your inner state?

@joshwcomeau
Copy link

joshwcomeau commented Oct 19, 2017

For data-munging, like in your third point, I like to use an intermediary component.

const App = ({ rawMessages }) => (
  <FetchMessages rawData={rawMessages}>
    {messages => (
      <MyComponent messages={messages} />
    )}
  </FetchMessages>
);

I like to use function-as-children, but there's nothing stopping you from using a render prop, or a HOC, or cloneElement.

I think it's quite a nice pattern - you have a data-munging component that abstracts that concern without doing anything presentational, and your presentational MyComponent gets to live a blissfully ignorant life, oblivious to all the work that needs to be done to get its messages.

Random aside: I think the reason this seems unintuitive is because we still tend to think of React components as "Stuff that renders UI", instead of "A mechanism of encapsulation that may or may not render UI".

To your point about "even if I learn a different pattern, my teammates will continue to write code like this"... I hear what you're saying, and you're right in the short-term... but I think if we continue to develop and share good practices, it'll eventually spread wide enough to be the default way of doing things. Maybe it makes sense to cover render props earlier, when educating new React devs, so that new patterns like this aren't seen as so esoteric?

@mjackson
Copy link

Can you expand a little on why you need an component? AFAICT, (lower-case i) is one of the very few times I actually need a ref, and that's only so I can focus() it. I've never understood the use case for a custom . I'd be grateful if you would please say a little more about what you're doing with it.

@mjackson
Copy link

Ah dang, forgot to put my <Input> and <input> in backticks, so they got swallowed. Sorry, on my phone!

@faceyspacey
Copy link

faceyspacey commented Oct 20, 2017

From my experience, lifecycle handlers are a solution to performing too much data transformation in the view layer (i.e. parent components).

The more you can push such transformation out of the "component layer" and back to reducers + selectors, the less incoming component-to-component props you have.

What will happen is:

  • the data you depended on from props is now available to selectors
  • componentWillReceiveProps won't even have anything to respond to anymore
  • and then you can perform the same work of componentWillReceiveProps in another selector (without the worry of other unrelated props and state changes triggering the lifecycle hook)

In addition having to worry about unrelated incoming props is a bigger problem than it seems--your reducing/selecting is now being called when it's not appropriate, and you have to account for all those cases with confusing if/else logic.

Many an app ends in spagetti code because of doing too much at the component-level.

I've found the best use of component level state is when you have a small # of well-defined states, and the goal is to make a re-usable component, like a switch or modal. Once you're doing application-specific stuff there, you're usually on a path to hell. This ultimately is a major problem for the community because "component everything" and overuse of lifecycle hooks is where beginning React developers are ushered to. It's more accessible than Redux, but the runway it gives you is very short.

@jlongster
Copy link
Author

jlongster commented Oct 20, 2017

Can you expand a little on why you need an component?

@mjackson Focus is a big one, and it's so common that that itself justifies it to be honest. Other ones are animations; in some parts I use a JS-driven animation system which needs the DOM node to animate. In one case I'm using a 3rd party component and it was a pain to try to get a ref to animate it with.

It's a minor gripe, and one of those things that when you need it (and you need focus) it can be a pain.

@jlongster
Copy link
Author

As for the above replies, it's all a spectrum for where you put your logic. Putting it outside of the component makes the consumer do more, which is appropriate for some APIs, but not others. You can also sideload data and transform it through some kind of stream (as @faceyspacey is saying) and that is appropriate sometimes too. Sometimes it's best for the logic to be inside the component itself, and all other things are much uglier and lead to more boilerplate. I use all of these ways to manage state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment