Wrap lifecycle events with ReactPerf.measure if __DEV__

[gfogle]

In order to do more advanced metrics with ReactPerf, ReactDefaultPerf, etc. we really need to have those being run through the measure cycle as well as renders.

This will allow for more advanced metrics like # of times should component update called per component, etc.

Ultimately, would like to see React pump out warnings for things like component has no state or props and no shouldComponentUpdate or shouldComponentUpdate always returns true/false.

Seems like this would be much more impactful if baked into core rather than having to write additional 3rd-party tools to hijack ReactComponents to measure them.

[gfogle]

this is in relation to #4521 (updated based on @jimfb )

[jimfb]

You posted a self-referential link back to this pull request; I assume you intended to link to: #4521

The unit tests aren't passing, as per Travis.

Conceptually this feature seems reasonable to me but we should confirm with @spicyj and @sebmarkbage

[gfogle]

@jimfb - looking into the tests now. I'm hijacking the lifecycles so where they're re-attached might be the issue.

[gfogle]

@jimfb - sorry for the delay in getting to look at this. Travis is kosher now. Would love for some feedback on this. If my description of the PR isn't clear just let me know.

[sebmarkbage]

This is only wrapping it in ReactClass which doesn't work with ES6 classes etc. It should be done in CompositeComponent if anything. However, I'm not sure if these necessarily are makes sense as part of ReactPerf or some more complete profiling solution.

[gfogle]

Yea I originally was going to break it up into smaller PRs. I'll just submit all of it at once.

George

On Oct 6, 2015, at 12:47 AM, Sebastian Markbåge notifications@github.com wrote:

This is only wrapping it in ReactClass which doesn't work with ES6 classes etc. It should be done in CompositeComponent if anything. However, I'm not sure if these necessarily are makes sense as part of ReactPerf or some more complete profiling solution.

—
Reply to this email directly or view it on GitHub.

[gfogle]

@sebmarkbage - I'm looking through implementing these features onto ReactCompositeComponent instead. Trouble I'm seeing is that only the methods defined on RCC are running through ReactDefaultPerf i.e. _renderValidatedComponent, updateComponent, mountComponent, etc. So trying to hijack the inst var in mountComponent works and lifecycles can be re-routed through ReactPerf... but what actually routes through the ReactPerf.measure is the same functions above: mountComponent, updateComponent, etc.

will look into this more. most likely will close this PR and issue and take different approach