Skip to content

chore(clerk-react): Send telemetry events for React hooks#3257

Closed
LauraBeatris wants to merge 1 commit intoclerk:mainfrom
LauraBeatris:send-telemetry-events-react-hooks
Closed

chore(clerk-react): Send telemetry events for React hooks#3257
LauraBeatris wants to merge 1 commit intoclerk:mainfrom
LauraBeatris:send-telemetry-events-react-hooks

Conversation

@LauraBeatris
Copy link
Copy Markdown
Member

@LauraBeatris LauraBeatris commented Apr 24, 2024

Description

Resolves SDK-1672

Sends telemetry events for React hooks. This will be useful to track regarding the creation of custom flows with hooks such as useSignUp.

Checklist

  • npm test runs as expected.
  • npm run build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

@LauraBeatris LauraBeatris self-assigned this Apr 24, 2024
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 24, 2024

🦋 Changeset detected

Latest commit: 6dbb515

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@clerk/clerk-react Patch
@clerk/chrome-extension Patch
@clerk/clerk-expo Patch
gatsby-plugin-clerk Patch
@clerk/nextjs Patch
@clerk/remix Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[orgId, orgRole, userId, orgPermissions],
);

useEffect(() => {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We need to be careful that those events will be emitted on the React tree render. Especially during local development, this could be dispatched in a huge volume due to hot reloads.

I'm considering passing a sampling rate to decrease the fraction of all events that should be recorded. We already do so for COMPONENT_MOUNTED.

@LauraBeatris
Copy link
Copy Markdown
Member Author

LauraBeatris commented Apr 24, 2024

We should not emit events for all hooks. We need to consider what particular insights each hook would give us, and whether it's useful.

I advocate for sending events from useSignIn and useSignUp, is cause those give us insights into custom flows, eg: How this would react when Clerk Elements get released.

Edit: @panteliselef also mentioned that tracking organizations might be useful to have a sense for custom organization flows.

@LauraBeatris
Copy link
Copy Markdown
Member Author

LauraBeatris commented Apr 24, 2024

Blocking this PR as I'm still deciding on how to move forward with recordings events in code hot-paths.

Edit: We're going to move forward with introducing client-side caching. I'll open another PR for this.

@LauraBeatris
Copy link
Copy Markdown
Member Author

Also closing this one in order to open later directly from the repo instead of a forked branch.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants