Function.bind used in event binding will always re-render because it is not pure

Question

Working on render performance on React, wonder what is the best way to tackle this performance issue. (The code is overly simplified for clarity)

var TodoList = React.createClass({
    getInitialState: function () {
        return { todos: Immutable.List(['Buy milk', 'Buy eggs']) };
    },
    onTodoChange: function (index, newValue) {
        this.setState({
            todos: this.state.todos.set(index, newValue)
        });
    },
    render: function () {
        return (
            this.state.todos.map((todo, index) =>
                <TodoItem
                    value={todo}
                    onChange={this.onTodoChange.bind(null, index)} />
            )
        );
    }
});

Assume only one single todo item has been changed. First, TodoList.render() will be called and start re-render the list again. Since TodoItem.onChange is binding to a event handler thru bind, thus, TodoItem.onChange will have a new value every time TodoList.render() is called. That means, even though we applied React.addons.PureRenderMixin to TodoItem.mixins, not one but all TodoItem will be re-rendered even when their value has not been changed.

I believe there are multiple ways to solve this, I am looking for an elegant way to solve the issue.

David L. Walsh · Accepted Answer · 2015-12-11 06:04:39Z

1

When looping through UI components in React, you need to use the key property. This allows for like-for-like comparisons. You will probably have seen the following warning in the console.

Warning: Each child in an array or iterator should have a unique "key" prop.

It's tempting to use the index property as the key, and if the list is static this may be a good choice (if only to get rid of the warning). However if the list is dynamic, you need a better key. In this case, I'd opt for the value of the todo item itself.

render: function () {
    return (
        this.state.todos.map((todo, index) => (
            <TodoItem
                key={todo}
                value={todo}
                onChange={this.onTodoChange.bind(null, index)}
            />
        ))
    );
}

Finally, I think your conjecture about the nature of the onChange property is off the mark. Yes it will be a different property each time it is rendered. But the property itself has no rendering effect, so it doesn't come into play in the virtual DOM comparison.

UPDATE

(This answer has been updated based on the conversation below.)

Whilst it's true that a change to a non-render based prop like onChange won't trigger a re-render, it will trigger a virtual DOM comparison. Depending on the size of your app, this may be expensive or it may be trivial.

Should it be necessary to avoid this comparison, you'll need to implement the component's shouldComponentUpdate method to ignore any changes to non-render based props. e.g.

shouldComponentUpdate(nextProps) {

    const ignoreProps = [ 'onChange' ];

    const keys = Object.keys(this.props)
        .filter((k) => ignoreProps.indexOf(k) === -1);

    const keysNext = Object.keys(nextProps)
        .filter((k) => ignoreProps.indexOf(k) === -1);

    return keysNext.length !== keys.length ||
        keysNext.some((k) => nextProps[k] !== this.props[k]);

}

If using state, you'll also need to compare nextState to this.state.

edited Dec 11, 2015 at 6:04

answered Dec 10, 2015 at 4:24

David L. Walsh

24.9k10 gold badges63 silver badges50 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

Compulim Over a year ago

I over-simplified the code to omit the "key" for clarity. Even though "key" is added, the "onChange" is still a new instance every time render is called. In our real world scenario, our TodoItem is complicated. Therefore, we are seeing 20-30 ms for every TodoItem.render. Even though it is not rendered to browser DOM, the virtual DOM performance impact is still not negligible.

David L. Walsh Over a year ago

OK I see now. In that case, you probably want to roll your own shouldComponentUpdate in TodoItem to compare every prop except onChange.

Compulim Over a year ago

Okay, let's assume I return false on shouldComponentUpdate for onChange. If onChange handler really need to be updated, will this break?

David L. Walsh Over a year ago

No, I don't believe so. The prop will still be received, it just won't trigger a re-render.

Compulim Over a year ago

Tested, you are right. Even shouldComponentUpdate return false, this.props will be updated with nextProps. That means we should implement shouldComponentUpdate that ignore changes for /^on[A-Z]/. Could you please update your answer?

|

Collectives™ on Stack Overflow

Function.bind used in event binding will always re-render because it is not pure

1 Answer 1

6 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

6 Comments

Your Answer

Sign up or log in

Post as a guest

Related