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

chore(react): cleanup outstanding TODOs on @sentry/react #2661

Merged
merged 4 commits into from Jun 11, 2020

Conversation

AbhiPrasad
Copy link
Member

@AbhiPrasad AbhiPrasad commented Jun 9, 2020

Following up #2659

Motivation

As I'm getting ready to test with getsentry, I went ahead and made a PR to cleanup and review all the react integration components.

Didn't think these small changes required a CHANGELOG entry, but please let me know if they do.

The commits

feat(react): Add Event Processor for React 0e2bf38

  • This adds a addGlobalEventProcessor that changes the sdk name to sentry.javascript.react

chore(react): Update comments and test for Profiler aeb266f

  • This updates the comments and tests for the Profiler components

fix(react): Correctly give eventID to dialog 3fca5b0

  • This commit makes it so that in the error boundary, the eventID produced by sentry.captureException is given to the report dialog
  • Also fixes a bug in the tests relating to onReset

fix(react): Prevent state updates from creating activities 23a96b5

  • We have to initialize the activity using a useState as otherwise we create new transactions every render. Previously if we updated a child component (like clicking a button to increment a counter), we get this:

image

Future

Test with getsentry. Also need to add a proper README, but will do that after getsentry testing.

@getsentry-bot
Copy link
Contributor

getsentry-bot commented Jun 9, 2020

Warnings
⚠️ Please add a changelog entry for your changes.
Messages
📖

@sentry/browser bundle gzip'ed minified size: (ES5: 17.083 kB) (ES6: 16.1523 kB)

📖 ✅ TSLint passed

Generated by 🚫 dangerJS against 1db3474

@@ -1,4 +1,27 @@
import { addGlobalEventProcessor, SDK_VERSION } from '@sentry/browser';
Copy link
Member

Choose a reason for hiding this comment

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

That we import SDK_VERSION here is not optimal.
The versions should never go out of sync but maybe it's better to also create a version.ts file like in the other packages and put it there. (Name & Version).

@AbhiPrasad AbhiPrasad merged commit 092367d into master Jun 11, 2020
@AbhiPrasad AbhiPrasad deleted the abhi/react/cleanup branch June 11, 2020 18:28
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.

None yet

3 participants