Skip to content

feat(react): Track duration of React component updates #5531

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

Merged
merged 7 commits into from
Aug 9, 2022

Conversation

0Calories
Copy link
Contributor

@0Calories 0Calories commented Aug 5, 2022

This PR gives the React SDK the ability to track the duration of ui.react.update spans by calling startChild within the shouldComponentUpdate lifecycle method, and then calling finish in componentDidUpdate

This will allow Performance users to see in their transactions how long it is taking their components to update

Before

image

After

image

@0Calories 0Calories added this to the Performance Issues milestone Aug 5, 2022
Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

@0Calories
Copy link
Contributor Author

Perfect! 😄

@0Calories 0Calories marked this pull request as ready for review August 5, 2022 19:57
@@ -34,6 +34,7 @@ class Profiler extends React.Component<ProfilerProps> {
* Made protected for the React Native SDK to access
*/
protected _mountSpan: Span | undefined = undefined;
protected _updateSpan: Span | undefined = undefined;
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a doc string for this?

// New props yet again
rerender(<ProfiledComponent num={2} />);
expect(mockStartChild).toHaveBeenCalledTimes(3);
expect(mockStartChild).toHaveBeenLastCalledWith({
data: { changedProps: ['num'] },
description: `<${UNKNOWN_COMPONENT}>`,
endTimestamp: expect.any(Number),
op: REACT_UPDATE_OP,
startTimestamp: expect.any(Number),
});
Copy link
Member

Choose a reason for hiding this comment

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

Can we assert on mockFinish after this block here?

@@ -76,8 +79,6 @@ class Profiler extends React.Component<ProfilerProps> {
// set as data on the span. We just store the prop keys as the values could be potenially very large.
const changedProps = Object.keys(updateProps).filter(k => updateProps[k] !== this.props.updateProps[k]);
if (changedProps.length > 0) {
// The update span is a point in time span with 0 duration, just signifying that the component
// has been updated.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this comment since it's no longer true

@0Calories 0Calories enabled auto-merge (squash) August 9, 2022 20:47
@0Calories 0Calories merged commit ee70717 into master Aug 9, 2022
@0Calories 0Calories deleted the feat/react-update-duration branch August 9, 2022 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants