Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce wasted renders for column_loading.js #5021

Merged
merged 2 commits into from Sep 20, 2017

Conversation

@nolanlawson
Copy link
Collaborator

commented Sep 20, 2017

This reduces some wasted time for this component, as shown in this ReactPerf.printWasted() before and after:

out2

(Scenario: load the local timeline in mobile view.)

@nolanlawson nolanlawson requested review from unarist and sorin-davidoi Sep 20, 2017
@@ -3,17 +3,25 @@ import PropTypes from 'prop-types';

import Column from '../../../components/column';
import ColumnHeader from '../../../components/column_header';
import ImmutablePureComponent from 'react-immutable-pure-component';

This comment has been minimized.

Copy link
@nolanlawson

nolanlawson Sep 20, 2017

Author Collaborator

I tried React.PureComponent but for some reason that resulted in more wasted renders than ImmutablePureComponent.

render() {
let { title, icon } = this.props;
title = title || '';
icon = icon || '';

This comment has been minimized.

Copy link
@ykzts

ykzts Sep 20, 2017

Collaborator
const { title = '', icon = '' } = this.props;

Is it not acceptable in the above writing style?

This comment has been minimized.

Copy link
@ykzts

ykzts Sep 20, 2017

Collaborator

Ah...

static propTypes = {
  title: PropTypes.oneOfType(PropTypes.node, PropTypes.string),
  icon: PropTypes.string,
};

static defaultProps = {
  title: '',
  icon: '',
};

The above may be better.

This comment has been minimized.

Copy link
@nolanlawson

nolanlawson Sep 20, 2017

Author Collaborator

Done! :)

};

render() {
let { title, icon } = this.props;

This comment has been minimized.

Copy link
@ykzts

ykzts Sep 20, 2017

Collaborator

let -> const

@Gargron Gargron merged commit 798b0fc into tootsuite:master Sep 20, 2017
2 checks passed
2 checks passed
codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
rutan added a commit to rutan/mastodon that referenced this pull request Oct 11, 2017
* Reduce wasted renders for column_loading.js

* Use defaultProps
takayamaki added a commit to takayamaki/mastodon that referenced this pull request Oct 12, 2017
* Reduce wasted renders for column_loading.js

* Use defaultProps
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.