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

Optionally adding original class name to ease debug and reporting on Sentry #1

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rodrigoalmeidaee
Copy link
Contributor

This pull request adds the possibility to include the original class name into the resulting class name (if you use a style named styles.foo, then foo is the original class name). We add it with an underscore prefix to avoid clashing with user-given class names. This is turned on by default on the runtime lib, and turned off by default on the build time lib.

The intended use cases of this feature are:

  • In development: ease finding which <div> corresponds to which component
  • In production: have more meaningful Sentry breadcrumbs. As shown below, the classname is automatically reported for every key user UI interaction. However, the mangled classnames contribute very little to understanding where did the user click, as shown below:

image

Both these problems could also be solved (in the build time lib at least) by adding the original classname as a data attribute (something on the lines of "data-original-class"), although that would require a much more complex processing since we would have to rewrite attributes that aren't even originally set on the components, and handle the merge of original class names separately (in situations in which cn(styles.foo, styles.bar) is used). So I went for the path of least resistance at first ;-)

@duailibe
Copy link
Contributor

duailibe commented Mar 7, 2019

I think this doesn't solve the problem, as you'll still have a bunch of .gk##_****** noise in the breadcrumbs.

Googling a bit, I've found that Sentry lets you customize the breadcrumb information: https://docs.sentry.io/error-reporting/configuration/filtering/?platform=javascript#before-breadcrumb

In development: ease finding which <div> corresponds to which component

I think we should invest in learning how to use proper tools, like the React DevTools which should help with that.

I'm just making the argument of not adding features unless they're needed, I'm not necessarily against this. If you still think this is necessary, lgtm, go ahead and merge.

@duailibe duailibe closed this Jun 21, 2019
@duailibe duailibe reopened this Jun 21, 2019
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