diff --git a/CHANGELOG.md b/CHANGELOG.md index c2e7f8902a..959f05bec7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Features + +- Improve touch event component info if annotated with [`@sentry/babel-plugin-component-annotate`](https://www.npmjs.com/package/@sentry/babel-plugin-component-annotate) ([#3899](https://github.com/getsentry/sentry-react-native/pull/3899)) + ## 5.24.1 ### Fixes @@ -1033,6 +1039,7 @@ This has been fixed in [version `5.9.1`](https://github.com/getsentry/sentry-rea ## 5.4.0 ### Features + - Add TS 4.1 typings ([#2995](https://github.com/getsentry/sentry-react-native/pull/2995)) - TS 3.8 are present and work automatically with older projects - Add CPU Info to Device Context ([#2984](https://github.com/getsentry/sentry-react-native/pull/2984)) diff --git a/samples/expo/babel.config.js b/samples/expo/babel.config.js index 0f75e07239..7a13872315 100644 --- a/samples/expo/babel.config.js +++ b/samples/expo/babel.config.js @@ -1,4 +1,6 @@ -module.exports = function(api) { +const componentAnnotatePlugin = require('@sentry/babel-plugin-component-annotate'); + +module.exports = function (api) { api.cache(false); return { presets: ['babel-preset-expo'], @@ -11,6 +13,7 @@ module.exports = function(api) { }, }, ], + componentAnnotatePlugin, ], }; }; diff --git a/samples/expo/package.json b/samples/expo/package.json index c5a3506800..9809687549 100644 --- a/samples/expo/package.json +++ b/samples/expo/package.json @@ -33,6 +33,7 @@ "devDependencies": { "@babel/core": "^7.20.0", "@babel/preset-env": "7.1.6", + "@sentry/babel-plugin-component-annotate": "^2.18.0", "@types/node": "20.10.4" }, "overrides": { diff --git a/samples/expo/yarn.lock b/samples/expo/yarn.lock index 923a71ec6e..c0cc28dfff 100644 --- a/samples/expo/yarn.lock +++ b/samples/expo/yarn.lock @@ -2413,6 +2413,11 @@ component-type "^1.2.1" join-component "^1.1.0" +"@sentry/babel-plugin-component-annotate@^2.18.0": + version "2.18.0" + resolved "https://registry.yarnpkg.com/@sentry/babel-plugin-component-annotate/-/babel-plugin-component-annotate-2.18.0.tgz#3bee98f94945643b0762ceed1f6cca60db52bdbd" + integrity sha512-9L4RbhS3WNtc/SokIhc0dwgcvs78YSQPakZejsrIgnzLzCi8mS6PeT+BY0+QCtsXxjd1egM8hqcJeB0lukBkXA== + "@sideway/address@^4.1.3": version "4.1.4" resolved "https://registry.yarnpkg.com/@sideway/address/-/address-4.1.4.tgz#03dccebc6ea47fdc226f7d3d1ad512955d4783f0" diff --git a/samples/react-native/babel.config.js b/samples/react-native/babel.config.js index 66309ee5ed..8ad75b81a3 100644 --- a/samples/react-native/babel.config.js +++ b/samples/react-native/babel.config.js @@ -1,3 +1,5 @@ +const componentAnnotatePlugin = require('@sentry/babel-plugin-component-annotate'); + module.exports = { presets: ['module:@react-native/babel-preset'], plugins: [ @@ -9,5 +11,6 @@ module.exports = { }, }, ], + componentAnnotatePlugin, ], }; diff --git a/samples/react-native/package.json b/samples/react-native/package.json index 1f58d11bc0..5bdcc35af5 100644 --- a/samples/react-native/package.json +++ b/samples/react-native/package.json @@ -43,6 +43,7 @@ "@react-native/eslint-config": "^0.73.1", "@react-native/metro-config": "^0.73.1", "@react-native/typescript-config": "^0.73.1", + "@sentry/babel-plugin-component-annotate": "^2.18.0", "@types/react": "^18.2.65", "@types/react-native-vector-icons": "^6.4.18", "@types/react-test-renderer": "^18.0.0", diff --git a/samples/react-native/yarn.lock b/samples/react-native/yarn.lock index 07789f3bd5..efba588fb4 100644 --- a/samples/react-native/yarn.lock +++ b/samples/react-native/yarn.lock @@ -3127,6 +3127,11 @@ color "^4.2.3" warn-once "^0.1.0" +"@sentry/babel-plugin-component-annotate@^2.18.0": + version "2.18.0" + resolved "https://registry.yarnpkg.com/@sentry/babel-plugin-component-annotate/-/babel-plugin-component-annotate-2.18.0.tgz#3bee98f94945643b0762ceed1f6cca60db52bdbd" + integrity sha512-9L4RbhS3WNtc/SokIhc0dwgcvs78YSQPakZejsrIgnzLzCi8mS6PeT+BY0+QCtsXxjd1egM8hqcJeB0lukBkXA== + "@sideway/address@^4.1.3": version "4.1.4" resolved "https://registry.yarnpkg.com/@sideway/address/-/address-4.1.4.tgz#03dccebc6ea47fdc226f7d3d1ad512955d4783f0" diff --git a/src/js/touchevents.tsx b/src/js/touchevents.tsx index fa58273baf..88ba178864 100644 --- a/src/js/touchevents.tsx +++ b/src/js/touchevents.tsx @@ -2,7 +2,7 @@ import { addBreadcrumb, getCurrentHub } from '@sentry/core'; import type { SeverityLevel } from '@sentry/types'; import { logger } from '@sentry/utils'; import * as React from 'react'; -import type { GestureResponderEvent} from 'react-native'; +import type { GestureResponderEvent } from 'react-native'; import { StyleSheet, View } from 'react-native'; import { createIntegration } from './integrations/factory'; @@ -53,6 +53,9 @@ const DEFAULT_BREADCRUMB_TYPE = 'user'; const DEFAULT_MAX_COMPONENT_TREE_SIZE = 20; const SENTRY_LABEL_PROP_KEY = 'sentry-label'; +const SENTRY_COMPONENT_PROP_KEY = 'data-sentry-component'; +const SENTRY_ELEMENT_PROP_KEY = 'data-sentry-element'; +const SENTRY_FILE_PROP_KEY = 'data-sentry-source-file'; interface ElementInstance { elementType?: { @@ -63,6 +66,13 @@ interface ElementInstance { return?: ElementInstance; } +interface TouchedComponentInfo { + name?: string; + label?: string; + element?: string; + file?: string; +} + interface PrivateGestureResponderEvent extends GestureResponderEvent { _targetInst?: ElementInstance; } @@ -71,7 +81,6 @@ interface PrivateGestureResponderEvent extends GestureResponderEvent { * Boundary to log breadcrumbs for interaction events. */ class TouchEventBoundary extends React.Component { - public static displayName: string = '__Sentry.TouchEventBoundary'; public static defaultProps: Partial = { breadcrumbCategory: DEFAULT_BREADCRUMB_CATEGORY, @@ -113,18 +122,17 @@ class TouchEventBoundary extends React.Component { /** * Logs the touch event given the component tree names and a label. */ - private _logTouchEvent( - componentTreeNames: string[], - activeLabel?: string - ): void { + private _logTouchEvent(touchPath: TouchedComponentInfo[], label?: string): void { const level = 'info' as SeverityLevel; + + const root = touchPath[0]; + const detail = label ? label : `${root.name}${root.file ? ` (${root.file})` : ''}`; + const crumb = { category: this.props.breadcrumbCategory, - data: { componentTree: componentTreeNames }, + data: { path: touchPath }, level: level, - message: activeLabel - ? `Touch event within element: ${activeLabel}` - : 'Touch event within component tree', + message: `Touch event within element: ${detail}`, type: this.props.breadcrumbType, }; addBreadcrumb(crumb); @@ -147,7 +155,7 @@ class TouchEventBoundary extends React.Component { return ignoreNames.some( (ignoreName: string | RegExp) => (typeof ignoreName === 'string' && name === ignoreName) || - (ignoreName instanceof RegExp && name.match(ignoreName)) + (ignoreName instanceof RegExp && name.match(ignoreName)), ); } @@ -166,80 +174,91 @@ class TouchEventBoundary extends React.Component { } let currentInst: ElementInstance | undefined = e._targetInst; - - let activeLabel: string | undefined; - let activeDisplayName: string | undefined; - const componentTreeNames: string[] = []; + const touchPath: TouchedComponentInfo[] = []; while ( currentInst && // maxComponentTreeSize will always be defined as we have a defaultProps. But ts needs a check so this is here. this.props.maxComponentTreeSize && - componentTreeNames.length < this.props.maxComponentTreeSize + touchPath.length < this.props.maxComponentTreeSize ) { if ( // If the loop gets to the boundary itself, break. - currentInst.elementType?.displayName === - TouchEventBoundary.displayName + currentInst.elementType?.displayName === TouchEventBoundary.displayName ) { break; } - const props = currentInst.memoizedProps; - const sentryLabel = - typeof props?.[SENTRY_LABEL_PROP_KEY] !== 'undefined' - ? `${props[SENTRY_LABEL_PROP_KEY]}` + const props = currentInst.memoizedProps ?? {}; + const info: TouchedComponentInfo = {}; + + // provided by @sentry/babel-plugin-component-annotate + if (typeof props[SENTRY_COMPONENT_PROP_KEY] === 'string' && props[SENTRY_COMPONENT_PROP_KEY].length > 0 && props[SENTRY_COMPONENT_PROP_KEY] !== 'unknown') { + info.name = props[SENTRY_COMPONENT_PROP_KEY]; + } + if (typeof props[SENTRY_ELEMENT_PROP_KEY] === 'string' && props[SENTRY_ELEMENT_PROP_KEY].length > 0 && props[SENTRY_ELEMENT_PROP_KEY] !== 'unknown') { + info.element = props[SENTRY_ELEMENT_PROP_KEY]; + } + if (typeof props[SENTRY_FILE_PROP_KEY] === 'string' && props[SENTRY_FILE_PROP_KEY].length > 0 && props[SENTRY_FILE_PROP_KEY] !== 'unknown') { + info.file = props[SENTRY_FILE_PROP_KEY]; + } + + // use custom label if provided by the user, or displayName if available + const labelValue = + typeof props[SENTRY_LABEL_PROP_KEY] === 'string' + ? props[SENTRY_LABEL_PROP_KEY] + : // For some reason type narrowing doesn't work as expected with indexing when checking it all in one go in + // the "check-label" if sentence, so we have to assign it to a variable here first + typeof this.props.labelName === 'string' + ? props[this.props.labelName] : undefined; - // For some reason type narrowing doesn't work as expected with indexing when checking it all in one go in - // the "check-label" if sentence, so we have to assign it to a variable here first - let labelValue; - if (typeof this.props.labelName === 'string') - labelValue = props?.[this.props.labelName]; - - // Check the label first - if (sentryLabel && !this._isNameIgnored(sentryLabel)) { - if (!activeLabel) { - activeLabel = sentryLabel; - } - componentTreeNames.push(sentryLabel); - } else if ( - typeof labelValue === 'string' && - !this._isNameIgnored(labelValue) - ) { - if (!activeLabel) { - activeLabel = labelValue; - } - componentTreeNames.push(labelValue); - } else if (currentInst.elementType) { - const { elementType } = currentInst; - - if ( - elementType.displayName && - !this._isNameIgnored(elementType.displayName) - ) { - // Check display name - if (!activeDisplayName) { - activeDisplayName = elementType.displayName; - } - componentTreeNames.push(elementType.displayName); - } + if (typeof labelValue === 'string' && labelValue.length > 0) { + info.label = labelValue; + } + + if (!info.name && currentInst.elementType?.displayName) { + info.name = currentInst.elementType?.displayName; } + this._pushIfNotIgnored(touchPath, info); + currentInst = currentInst.return; } - const finalLabel = activeLabel ?? activeDisplayName; - - if (componentTreeNames.length > 0 || finalLabel) { - this._logTouchEvent(componentTreeNames, finalLabel); + const label = touchPath.find(info => info.label)?.label; + if (touchPath.length > 0) { + this._logTouchEvent(touchPath, label); } this._tracingIntegration?.startUserInteractionTransaction({ - elementId: activeLabel, + elementId: label, op: UI_ACTION_TOUCH, }); } + + /** + * Pushes the name to the componentTreeNames array if it is not ignored. + */ + private _pushIfNotIgnored(touchPath: TouchedComponentInfo[], value: TouchedComponentInfo): boolean { + if (!value.name && !value.label) { + return false; + } + if (value.name && this._isNameIgnored(value.name)) { + return false; + } + if (value.label && this._isNameIgnored(value.label)) { + return false; + } + + // Deduplicate same subsequent items. + if (touchPath.length > 0 && JSON.stringify(touchPath[touchPath.length - 1]) === JSON.stringify(value)) { + return false; + } + + touchPath.push(value); + return true; + } } /** @@ -250,9 +269,9 @@ class TouchEventBoundary extends React.Component { const withTouchEventBoundary = ( // eslint-disable-next-line @typescript-eslint/no-explicit-any InnerComponent: React.ComponentType, - boundaryProps?: TouchEventBoundaryProps + boundaryProps?: TouchEventBoundaryProps, ): React.FunctionComponent => { - const WrappedComponent: React.FunctionComponent = (props) => ( + const WrappedComponent: React.FunctionComponent = props => ( diff --git a/test/touchevents.test.tsx b/test/touchevents.test.tsx index c3a18b246e..4d5dd1f3cd 100644 --- a/test/touchevents.test.tsx +++ b/test/touchevents.test.tsx @@ -5,7 +5,7 @@ import * as core from '@sentry/core'; import type { SeverityLevel } from '@sentry/types'; import { TouchEventBoundary } from '../src/js/touchevents'; -import { getDefaultTestClientOptions,TestClient } from './mocks/client'; +import { getDefaultTestClientOptions, TestClient } from './mocks/client'; describe('TouchEventBoundary._onTouchStart', () => { let addBreadcrumb: jest.SpyInstance; @@ -101,7 +101,7 @@ describe('TouchEventBoundary._onTouchStart', () => { expect(addBreadcrumb).toBeCalledWith({ category: defaultProps.breadcrumbCategory, data: { - componentTree: ['View', 'Connect(View)', 'LABEL!'], + path: [{ name: 'View' }, { name: 'Connect(View)' }, { label: 'LABEL!' }], }, level: 'info' as SeverityLevel, message: 'Touch event within element: LABEL!', @@ -160,15 +160,15 @@ describe('TouchEventBoundary._onTouchStart', () => { expect(addBreadcrumb).toBeCalledWith({ category: defaultProps.breadcrumbCategory, data: { - componentTree: ['Styled(View2)', 'Styled(View)'], + path: [{ name: 'Styled(View)' }], }, level: 'info' as SeverityLevel, - message: 'Touch event within element: Styled(View2)', + message: 'Touch event within element: Styled(View)', type: defaultProps.breadcrumbType, }); }); - it('maxComponentTreeSize', () => { + it('maxpathSize', () => { const { defaultProps } = TouchEventBoundary; const boundary = new TouchEventBoundary({ ...defaultProps, @@ -210,11 +210,105 @@ describe('TouchEventBoundary._onTouchStart', () => { expect(addBreadcrumb).toBeCalledWith({ category: defaultProps.breadcrumbCategory, data: { - componentTree: ['Connect(View)', 'Styled(View)'], + path: [{ label: 'Connect(View)' }, { name: 'Styled(View)' }], }, level: 'info' as SeverityLevel, message: 'Touch event within element: Connect(View)', type: defaultProps.breadcrumbType, }); }); + + // see https://docs.sentry.io/platforms/javascript/guides/react/features/component-names/ + it('uses custom names provided by babel plugin', () => { + const { defaultProps } = TouchEventBoundary; + const boundary = new TouchEventBoundary(defaultProps); + + const event = { + _targetInst: { + elementType: { + displayName: 'View', + }, + memoizedProps: { + 'data-sentry-component': 'Screen', + 'data-sentry-element': 'AnimatedNativeScreen', + 'data-sentry-source-file': 'screen.tsx', + }, + return: { + elementType: { + displayName: 'Text', + }, + return: { + memoizedProps: { + 'custom-sentry-label-name': 'Connect(View)', + 'data-sentry-component': 'MyView', + 'data-sentry-element': 'unknown', // should be ignored + 'data-sentry-source-file': 'myview.tsx', + }, + return: { + elementType: { + displayName: 'Styled(View)', + }, + return: { + memoizedProps: { + 'data-sentry-component': 'Happy', + 'data-sentry-element': 'View', + 'data-sentry-source-file': 'happyview.js', + }, + }, + }, + }, + }, + }, + }; + + // @ts-expect-error Calling private member + boundary._onTouchStart(event); + + expect(addBreadcrumb).toBeCalledWith({ + category: defaultProps.breadcrumbCategory, + data: { + path: [ + { element: 'AnimatedNativeScreen', file: 'screen.tsx', name: 'Screen' }, + { name: 'Text' }, + { file: 'myview.tsx', name: 'MyView' }, + { name: 'Styled(View)' }, + { element: 'View', file: 'happyview.js', name: 'Happy' }, + ], + }, + level: 'info' as SeverityLevel, + message: 'Touch event within element: Screen (screen.tsx)', + type: defaultProps.breadcrumbType, + }); + }); + + it('deduplicates', () => { + const { defaultProps } = TouchEventBoundary; + const boundary = new TouchEventBoundary(defaultProps); + + const event = { + _targetInst: { + elementType: { + displayName: 'Text', + }, + return: { + elementType: { + displayName: 'Text', + }, + }, + }, + }; + + // @ts-expect-error Calling private member + boundary._onTouchStart(event); + + expect(addBreadcrumb).toBeCalledWith({ + category: defaultProps.breadcrumbCategory, + data: { + path: [{ name: 'Text' }], + }, + level: 'info' as SeverityLevel, + message: 'Touch event within element: Text', + type: defaultProps.breadcrumbType, + }); + }); });