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

Perf actual React components #5264

Closed
wants to merge 10 commits into from

Conversation

gfogle
Copy link

@gfogle gfogle commented Oct 24, 2015

Working through #5254

I'm hijacking the lifecycle methods of the react components and then enabling DefaultPerf to measure them and print high-level statistics on the measures.

I started at a basic implementation. Would like to submit subsequent PRs with more advanced functionality such as tracking # of times returned true vs false for shouldComponentUpdate, etc.

@facebook-github-bot
Copy link

@gfogle updated the pull request.

@gfogle gfogle force-pushed the perf-lifecycles-of-components branch from b7b0fe5 to eb23bf1 Compare October 25, 2015 16:55
@gfogle gfogle changed the title jsdoc of context on renderNewRoot Perf actual React components Oct 25, 2015
@facebook-github-bot
Copy link

@gfogle updated the pull request.

@gfogle
Copy link
Author

gfogle commented Oct 25, 2015

FYI - travis is complaining about dangling comma rule. I get that this simplifies the diff, but it really looks weird to see those. fixing now though.

@gfogle gfogle force-pushed the perf-lifecycles-of-components branch from eb23bf1 to 87eb4cf Compare October 25, 2015 17:19
@facebook-github-bot
Copy link

@gfogle updated the pull request.

1 similar comment
@facebook-github-bot
Copy link

@gfogle updated the pull request.

this._instance[method]
);
}
}.bind(this));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bind isn't necessary, forEach takes a second arg, being thisarg if desired - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/forEach

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point

@facebook-github-bot
Copy link

@gfogle updated the pull request.

@gfogle
Copy link
Author

gfogle commented Oct 27, 2015

@sebmarkbage - I think this is a much clearer picture of the prior PR i closed because the commit history was just not worth rebasing out to reflect final implementation.

@gfogle gfogle force-pushed the perf-lifecycles-of-components branch from 4b24149 to 34292a0 Compare December 30, 2015 03:22
@facebook-github-bot
Copy link

@gfogle updated the pull request.

@facebook-github-bot
Copy link

@gfogle updated the pull request.

@sebmarkbage
Copy link
Collaborator

We've just recently started work on a new perf tool. #6046

This will allow us to do refactoring to the code base without breaking the perf tools all the time. Could you maybe check if your issue can be solved in #6046? Sorry for the inconvenience. We didn't plan for this new tool to replace the old one at this point.

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

Successfully merging this pull request may close these issues.

5 participants