-
Notifications
You must be signed in to change notification settings - Fork 11
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
[Ready for review] V0 initial improvements to performance data for demo #10
[Ready for review] V0 initial improvements to performance data for demo #10
Conversation
@@ -13,5 +13,13 @@ module.exports = { | |||
inlineRequires: true, | |||
}, | |||
}), | |||
minifierConfig: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This exists so that we don't get profiled components showing up as unknown
in spans.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice fix!
1e6beca
to
4d70c72
Compare
@@ -171,6 +171,9 @@ const ProductItem = (props: { | |||
imgcropped: string; | |||
appDispatch: AppDispatch; | |||
}): React.ReactElement => { | |||
React.useEffect(() => { | |||
fetch(`${BACKEND_URL}/success`) // exists just to add span data to demo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice usage and comment
/* You could wrap this with the Sentry Profiler, | ||
* but then you'd have hundreds/thousands of spans because the tools response is not paginated. | ||
*/ | ||
const ProfiledImage = Sentry.withProfiler(Image); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This helps me remember the Profiler was used, as I go through the rest of this file. Sweet.
react.update
spans -- working on that separately)react.update
spans showing. The plan is to move forward with what we have now (screenshot below) and then addreact.update
spans once we figure that out.