facebook/react-devtools

"Highlight Updates" gives false positives for a performance tuned React app

nareshbhatia opened this issue · 3 comments

I have created a minimal application that displays a list of 10 todo's using Redux. One of the todo's is updated every second. The app has been optimized using recommendations in Redux Performance FAQ. Indeed when I run the app, I can see in console logs that the list is rendered only once at application startup, and only one todo is rendered every second. However, React DevTools highlights the entire list every second.

I have created a minimal example to demonstrate the issue: https://github.com/nareshbhatia/redux-performance. Could someone please look at the example and confirm if this is a real issue with React DevTools or just a cockpit error!

You are preventing your components from re-rendering, but connect() wrappers generated by React Redux still re-render. I think you might have misinterpreted the advice (not sure because I haven’t read that FAQ): what you need to do instead is to make sure mapStateToProps result is shallowly equal for things that don‘t change.

Here is a diff to give you an idea:

diff --git a/src/components/todo-view.js b/src/components/todo-view.js
index 9559a04..a3c0ac6 100644
--- a/src/components/todo-view.js
+++ b/src/components/todo-view.js
@@ -2,24 +2,12 @@ import React from 'react';
 import { PropTypes } from 'prop-types';
 
 export class TodoView extends React.Component {
-    static propTypes = {
-        todoId: PropTypes.number.isRequired,
-        todoMap: PropTypes.object.isRequired
-    };
-
-    shouldComponentUpdate(nextProps) {
-        const { todoId, todoMap } = this.props;
-        const { todoMap: nextTodoMap } = nextProps;
-        return todoMap[todoId] !== nextTodoMap[todoId];
-    }
-
     render() {
-        const { todoId, todoMap } = this.props;
-        console.log('-----> TodoView.render():', todoId);
+        const { todo } = this.props;
 
         return (
             <tr>
-                <td >{todoMap[todoId]}</td>
+                <td >{todo}</td>
             </tr>
         );
     }
diff --git a/src/containers/todo-view-container.js b/src/containers/todo-view-container.js
index b3930f0..ad3e37f 100644
--- a/src/containers/todo-view-container.js
+++ b/src/containers/todo-view-container.js
@@ -1,8 +1,8 @@
 import { connect } from 'react-redux';
 import { TodoView } from '../components/todo-view';
 
-const mapStateToProps = state => ({
-    todoMap: state.todoMap
+const mapStateToProps = (state, ownProps) => ({
+    todo: state.todoMap[ownProps.todoId]
 });
 
 export const TodoViewContainer = connect(mapStateToProps)(TodoView);

cc @markerikson in case something in the FAQ could use a clarification

Thanks for the ping. I don't think there's anything specific that needs to be edited, but @nareshbhatia , feel free to file an issue on the Redux repo if you feel it's insufficiently informative.

Thanks @gaearon & @markerikson for your help. I think the docs are very clear. The only trick that I did not think of (as suggested by @gaearon) is to use ownProps to resolve the todo right inside the container (instead of passing the entire map into the view). This was the root cause of the inefficient rendering. I have fixed my repo based on your suggestion. Hopefully it is educational for others in the future.

Thanks again!