Skip to content

Commit 56591ac

Browse files
authored
Merge pull request #6 from itReverie/improvingPerformance
Improving performance with react-addons-perf, PureComponents, etc
2 parents 608e646 + 1abb0d3 commit 56591ac

File tree

11 files changed

+7624
-1725
lines changed

11 files changed

+7624
-1725
lines changed

lib/components/App.js

Lines changed: 52 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,17 @@
33
import React from 'react';
44
import PropTypes from 'prop-types';
55
import pickBy from 'lodash.pickby';
6+
import Perf from 'react-addons-perf';
7+
8+
if(typeof window !== 'undefined') {
9+
window.Perf = Perf;
10+
}
611

712
import ArticleList from './ArticleList';
813
import SearchBar from './SearchBar';
914
import Timestamp from './Timestamp';
1015

11-
class App extends React.Component {
16+
class App extends React.PureComponent {
1217

1318
//****** I need the following lines to expose the CONTEXT
1419
//to make the context api work we need to define the context type
@@ -22,26 +27,46 @@ class App extends React.Component {
2227
store: this.props.store
2328
};
2429
}
25-
2630
//********
2731

28-
state = this.props.store.getState();
2932

30-
onStoreChange=()=>{
31-
this.setState(this.props.store.getState);
33+
//**** Subscribing to a portion of the state,
34+
//pretty much just returning what we really need as a state for this particular component
35+
appState =()=>{
36+
const {articles, searchTerm} = this.props.store.getState();
37+
return {articles, searchTerm};
38+
};
39+
40+
state = this.appState();
41+
//****
42+
43+
44+
onStoreChange = () => {
45+
//this.props.store.getState....we dont need to read directly from the props state but our own implementation
46+
this.setState(this.appState);
3247
}
3348

34-
componentDidMount(){
49+
componentDidMount() {
3550
//we need to update the component state so the app component state is in sync to the store state
36-
if(this.subsciptionId) {
37-
this.subsciptionId = this.props.store.subscribe(this.onStoreChange);
38-
}
51+
this.subsciptionId = this.props.store.subscribe(this.onStoreChange);
3952
this.props.store.startClock();
53+
54+
55+
//TODO: Leaving this purposely uncommented. Be aware that in production this has to be commented.
56+
//We need consistent measures that's why we need to add this lines so we messure the components at the same point
57+
//Those numbers need to be the same every time we refresh
58+
setImmediate(()=>{Perf.start();});
59+
setTimeout(()=>{ Perf.stop();
60+
Perf.printWasted();},5000);
4061
}
4162

42-
componentWillUnmount(){
63+
componentWillUnmount() {
4364
this.props.store.unsubscribe(this.subscriptionId);
44-
this.subscriptionId = null;
65+
}
66+
67+
componentWillUpdate(nextProps, nextState)
68+
{
69+
console.log('Updating TimeStamp');
4570
}
4671

4772
//In react we cannot return two elements unless
@@ -71,23 +96,32 @@ class App extends React.Component {
7196
// we can make a StateApi an event emmiter (Flux)
7297
// or manage subscriptions in the same object
7398

99+
100+
//To solve rendering the articles if the properties have not changed
101+
//Just re-render if you need to
102+
//Downside is that we will have to update this method if we have another property
103+
// shouldComponentUpdate(nextProps, nextState){
104+
// return (nextState.articles !== this.state.articles ||
105+
// nextState.searchTerm !== this.state.searchTerm);
106+
// }
107+
74108
render() {
75-
let { articles , searchTerm}=this.state;
109+
let {articles, searchTerm} = this.state;
76110
//making the search case insensitive
77111
const searchRegex = new RegExp(searchTerm, 'i');
78-
if(searchTerm){
79-
articles= pickBy(articles, (value) => {
112+
if (searchTerm) {
113+
articles = pickBy(articles, (value) => {
80114
return value.title.match(searchRegex) ||
81-
value.body.match(searchRegex);
115+
value.body.match(searchRegex);
82116
});
83117
}
84118
return (
85119
<div>
86-
<Timestamp />
87-
<SearchBar key="searchBar" />
120+
<Timestamp/>
121+
<SearchBar key="searchBar"/>
88122
<ArticleList key="articleList"
89123
articles={articles}
90-
/>
124+
/>
91125
</div>
92126
);
93127
}

lib/components/Article.js

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -31,25 +31,27 @@ const dateDisplay = (dateString) => new Date(dateString).toDateString();
3131

3232
//********** Presentational Component ****************
3333
//Reads the context from props so It is easy to shallow test it
34-
const Article = (props) => {
35-
const {article, author} = props;
36-
34+
class Article extends React.PureComponent {
3735

3836
//Ideally there should not be operations or functions here
39-
return (
40-
<div style={styles.article}>
41-
<div style={styles.title}>{article.title}</div>
42-
<div style={styles.date}>
43-
{dateDisplay((article.date))}
44-
{article.date}
37+
render() {
38+
const {article, author} = this.props;
39+
return (
40+
<div style={styles.article}>
41+
<div style={styles.title}>{article.title}</div>
42+
<div style={styles.date}>
43+
{dateDisplay((article.date))}
44+
{article.date}
45+
</div>
46+
<div style={styles.author}><a href={author.website}>
47+
{author.firstName} {author.lastName}
48+
</a></div>
49+
<div style={styles.body}>{article.body}</div>
4550
</div>
46-
<div style={styles.author}><a href={author.website}>
47-
{author.firstName} {author.lastName}
48-
</a></div>
49-
<div style={styles.body}>{article.body}</div>
50-
</div>
51-
);
52-
};
51+
);
52+
}
53+
}
54+
5355

5456
Article.propTypes = {
5557
article: PropTypes.shape({
@@ -59,12 +61,13 @@ Article.propTypes = {
5961
})
6062
};
6163

62-
function extraProps (store, originalProps){
64+
//We are using props that do not use the state
65+
function extraProps(store, originalProps) {
6366
return {
6467
author: store.lookupAuthor(originalProps.article.authorId)
6568
};
6669
}
6770

6871
//This is similar as connect in Redux
69-
const ArticleContainer = storeProvider (extraProps)(Article);
72+
const ArticleContainer = storeProvider(extraProps)(Article);
7073
export default ArticleContainer;

lib/components/ArticleList.js

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,19 @@
11
import React from 'react';
22
import Article from './Article';
33

4-
const ArticleList = (props) => {
5-
return (
6-
<div>
7-
{Object.values(props.articles).map( (article) =>
8-
<Article key={article.id}
9-
article={article}/>
10-
)}
11-
</div>
12-
);
13-
};
4+
class ArticleList extends React.PureComponent
5+
{
6+
render()
7+
{
8+
return (
9+
<div>
10+
{Object.values(this.props.articles).map((article) =>
11+
<Article key={article.id}
12+
article={article}/>
13+
)}
14+
</div>
15+
);
16+
}
17+
}
1418

1519
export default ArticleList;

lib/components/SearchBar.js

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import React from 'react';
22
import debounce from 'lodash.debounce';//NOTE: we are just importing the debounce function from lodash not the whole package
33
import storeProvider from './storeProvider';
44

5-
class SearchBar extends React.Component{
5+
class SearchBar extends React.PureComponent{
66

77
state={
88
searchTerm: ''
@@ -21,6 +21,19 @@ class SearchBar extends React.Component{
2121

2222
};
2323

24+
//In this case we can just convert this to a PureComponent which does the job described below for us.
25+
// shouldComponentUpdate(nextProps, nextState){
26+
// //if we just pass FALSE it will render but does not allow us to type anything as the component is not connected with teh tree
27+
// //return false;
28+
// //If we pass TRUE it will rerender everysecons as our state with the tick interval is updating the state and a normal component re-renders every time the state is updated.
29+
// //return true;
30+
// //To solve the issue we should compare current and next props as well as current and previosu state
31+
// }
32+
33+
componentWillUpdate(nextProps, nextState)
34+
{
35+
console.log('Updating SearchBar');
36+
}
2437

2538
//Option 1. We could read the value of the text typed by ref (getting the current DOM node)
2639
// ref={(input) => this.searchInput = input}

lib/components/Timestamp.js

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,41 @@ import React from 'react';
33
import storeProvider from './storeProvider';
44

55
class Timestamp extends React.Component{
6+
7+
static timeDisplay = (timestamp) => timestamp.toLocaleTimeString([],{hour: '2-digit', minute:'2-digit'});
8+
9+
10+
//This logic is causing the Timestamp component to render every minute, in order to avoid that we
11+
//will move the ShouldComponentUpdate one level up to the Container Component
12+
13+
14+
// //We will do a custom should component update so it just updates based on the time and minutes update
15+
// shouldComponentUpdate(nextProps, nextState){
16+
// //const currentTimeDisplay= this.props.timestamp.toLocaleTimeString([],{hour: '2-digit', minute:'2-digit'});
17+
// //const nextTimeDisplay= nextProps.timestamp.toLocaleTimeString([],{hour: '2-digit', minute:'2-digit'});
18+
// //So if it is different re render just every minute not every second
19+
// //return currentTimeDisplay !== nextTimeDisplay;
20+
//
21+
// //Simplified version
22+
// return (this.timeDisplay(this.props.timestamp) !=
23+
// this.timeDisplay(nextProps.timestamp));
24+
// }
25+
26+
27+
628
render(){
7-
return(<div>{this.props.timestamp.toString()}</div>);
29+
return(<div>{this.props.timestampDisplay}</div>);
830
}
931
}
1032

33+
//We are using props that use the state (Contrary to articles List )
1134
function extraProps(store)
1235
{
36+
//This is the only component that is actually making use of the state
37+
//timestamp: store.getState().timestamp
1338
return {
14-
timestamp: store.getState().timestamp
39+
40+
timestampDisplay: Timestamp.timeDisplay(store.getState().timestamp)
1541
};
1642
}
1743

lib/components/__tests__/ArticleList.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import React from 'react';
33
//import renderer from 'react-test-renderer';
44
import ArticleList from '../ArticleList';
55
import { configure, shallow } from 'enzyme';
6-
import Adapter from 'enzyme-adapter-react-16';
6+
import Adapter from 'enzyme-adapter-react-15';
77

88
configure({ adapter: new Adapter() });
99

lib/components/storeProvider.js

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,27 +16,38 @@ const storeProvider = (extraProps = () =>({})) => (Component) => {
1616
static contextTypes = {store: PropTypes.object};
1717

1818

19+
//only the properties of the state that have been used for the warp component
20+
usedState = () => {
21+
return extraProps(this.context.store, this.props);
22+
};
23+
24+
state = this.usedState();
25+
26+
1927
onStoreChange=()=>{
20-
//Give the component the signal that it needs to re-render
21-
this.forceUpdate();
28+
if(this.subsciptionId) {
29+
//Give the component the signal that it needs to re-render
30+
//this.forceUpdate();
31+
this.setState(this.usedState());
32+
}
2233
}
2334

2435
componentDidMount(){
2536
//we need to update the component state so the app component state is in sync to the store state
26-
if(this.subsciptionId) {
27-
this.subsciptionId = this.context.store.subscribe(this.onStoreChange);
28-
}
37+
this.subsciptionId = this.context.store.subscribe(this.onStoreChange);
2938
}
3039

3140
componentWillUnmount(){
3241
this.context.store.unsubscribe(this.subscriptionId);
3342
this.subscriptionId = null;
3443
}
3544

45+
46+
3647
render(){
3748
return <Component
3849
{...this.props}
39-
{...extraProps(this.context.store, this.props)}
50+
{...this.usedState()}
4051
store={this.context.store}/>;
4152
}
4253
};

lib/state-api/lib/index.js

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,33 @@ class StateApi{
77
authors: this.mapIntoObject(rawData.authors),
88
searchTerm :'',
99
timestamp: new Date()};
10-
1110
this.subscriptions ={};
12-
1311
this.lastSubscriptionId=0;
1412

15-
13+
//Simulating adding an article every minute
14+
setTimeout(() => {
15+
const fakeArticle = {
16+
...rawData.articles[0],
17+
id: 'fakeArticleId',
18+
};
19+
//This line mutates the state and that wont be recognized by the PureComponent
20+
// this.data.articles[fakeArticle.id] = fakeArticle;
21+
22+
//Hence instead of mutating the state we will copy
23+
//We are copying the current state and modifying that copy of the current state
24+
//So for the previous object and current object are different
25+
//Copying current data
26+
//Change the articles object
27+
//add new article
28+
this.data = {
29+
...this.data,
30+
articles: {
31+
...this.data.articles,
32+
[fakeArticle.id]: fakeArticle,
33+
},
34+
};
35+
this.notifySubscribers();
36+
}, 1000);
1637
}
1738

1839

0 commit comments

Comments
 (0)