From 5657daf1a61453c770b3b3698f9edf86b06a75ce Mon Sep 17 00:00:00 2001 From: Marco Saia Date: Mon, 12 Oct 2026 10:27:54 +0200 Subject: [PATCH 1/5] Babel plugin: support expressions in action name attributes `dd-action-name`, `accessibilityLabel` and the custom `actionNameAttribute` were only collected when written as string literal attributes, so values like `testID={TestIDs.button}` or `accessibilityLabel={t('save')}` were silently dropped and the action name fell back to content. Expressions are now forwarded and evaluated at press time inside the generated wrapper, as long as all their bindings are visible from where the wrapper runs. Memoized handlers (`useCallback`/`useMemo`) get a separate args object that only keeps values visible from the component body, which also fixes a `ReferenceError` when content candidates or children referenced `.map()` params. At runtime, empty or unresolved values are skipped when resolving the target name instead of producing `"undefined"`. --- .../DdBabelInteractionTracking.ts | 26 +- .../DdBabelInteractionTracking.test.ts | 61 ++++ .../src/actions/rum/index.ts | 319 +++++++++++++----- .../src/actions/rum/tap.ts | 32 +- .../src/utils/nodeProcessing.ts | 53 +++ .../test/actionNameAttribute.test.ts | 269 +++++++++++++++ 6 files changed, 659 insertions(+), 101 deletions(-) create mode 100644 packages/react-native-babel-plugin/test/actionNameAttribute.test.ts diff --git a/packages/core/src/rum/instrumentation/interactionTracking/DdBabelInteractionTracking.ts b/packages/core/src/rum/instrumentation/interactionTracking/DdBabelInteractionTracking.ts index 4cb146783..da2de7b4c 100644 --- a/packages/core/src/rum/instrumentation/interactionTracking/DdBabelInteractionTracking.ts +++ b/packages/core/src/rum/instrumentation/interactionTracking/DdBabelInteractionTracking.ts @@ -85,10 +85,26 @@ class BabelInteractionTracking { return null; }; + // Values can come from runtime expressions, so everything that + // doesn't resolve to a usable name is ignored. + const getValidValues = (values: unknown): string[] | null => { + if (!Array.isArray(values)) { + return null; + } + + const validValues = values + .filter( + x => + (typeof x === 'string' && x.length > 0) || + (typeof x === 'number' && Number.isFinite(x)) + ) + .map(String); + + return validValues.length > 0 ? validValues : null; + }; + const getAccessibilityLabel = () => - useAccessibilityLabel && accessibilityLabel - ? accessibilityLabel - : null; + useAccessibilityLabel ? getValidValues(accessibilityLabel) : null; const index = handlerArgs ? handlerArgs.find(x => typeof x === 'number') || 0 @@ -96,8 +112,8 @@ class BabelInteractionTracking { // Order: actionName → actionNameAttribute → accessibilityLabel → content const selectedContent = - actionName || - Object.values(attrs)[0] || + getValidValues(actionName) || + getValidValues(Object.values(attrs)[0]) || getAccessibilityLabel() || tryContent(); diff --git a/packages/core/src/rum/instrumentation/interactionTracking/__tests__/DdBabelInteractionTracking.test.ts b/packages/core/src/rum/instrumentation/interactionTracking/__tests__/DdBabelInteractionTracking.test.ts index c74b86ba5..a35f56f1c 100644 --- a/packages/core/src/rum/instrumentation/interactionTracking/__tests__/DdBabelInteractionTracking.test.ts +++ b/packages/core/src/rum/instrumentation/interactionTracking/__tests__/DdBabelInteractionTracking.test.ts @@ -198,3 +198,64 @@ describe('DdBabelInteractionTracking.wrapRumAction', () => { ); }); }); + +describe('DdBabelInteractionTracking target name resolution', () => { + const mockAddAction = jest.fn().mockResolvedValue(undefined); + + beforeEach(() => { + mockAddAction.mockClear(); + DdBabelInteractionTracking.config = { + trackInteractions: true, + useAccessibilityLabel: true + }; + DdBabelInteractionTracking.attachRumInstance({ + addAction: mockAddAction + } as any); + }); + + const getTargetName = (targetObject: Record) => { + DdBabelInteractionTracking.wrapRumAction(jest.fn(), RumActionType.TAP, { + ...mockTargetObject, + options: { useContent: true, useNamePrefix: false }, + ...targetObject + } as any)(); + return mockAddAction.mock.calls[0][1]; + }; + + it('should use the actionNameAttribute value', () => { + expect(getTargetName({ testID: ['save_button'] })).toBe('save_button'); + }); + + it('should skip undefined and empty actionNameAttribute values', () => { + expect( + getTargetName({ + testID: [undefined, ''], + accessibilityLabel: ['Save'] + }) + ).toBe('Save'); + }); + + it('should skip empty dd-action-name and fall back to actionNameAttribute', () => { + expect( + getTargetName({ 'dd-action-name': [], testID: ['save_button'] }) + ).toBe('save_button'); + }); + + it('should fall back to content when no attribute resolves', () => { + expect( + getTargetName({ + testID: [null], + accessibilityLabel: [undefined], + getContent: () => ['Save content'] + }) + ).toBe('Save content'); + }); + + it('should fall back to the component name when nothing resolves', () => { + expect(getTargetName({ testID: [undefined] })).toBe('Button'); + }); + + it('should stringify numeric values', () => { + expect(getTargetName({ testID: [42] })).toBe('42'); + }); +}); diff --git a/packages/react-native-babel-plugin/src/actions/rum/index.ts b/packages/react-native-babel-plugin/src/actions/rum/index.ts index 5e9bb9e66..113e32231 100644 --- a/packages/react-native-babel-plugin/src/actions/rum/index.ts +++ b/packages/react-native-babel-plugin/src/actions/rum/index.ts @@ -28,11 +28,19 @@ import { getImportDeclaration, getNodeName, insertAtProgramTop, + isExpressionInScope, toExpression } from '../../utils'; import { handleTapAction } from './tap'; +type DdValues = Record< + string, + | Babel.types.ArrayExpression + | Babel.types.ArrowFunctionExpression + | Babel.types.ObjectExpression +>; + /** * Inserts RUM Action Tracking import at the top of the Program. * @@ -85,7 +93,8 @@ export function handleJSXElementActionPaths( const { actionPathList, actionPathNames, - ddValues + ddValues, + hoistedDdValues } = getJSXElementActionPaths(componentName, t, path, state, options); // Create known custom components list tracked by the plugin @@ -116,13 +125,22 @@ export function handleJSXElementActionPaths( const programPath = path.findParent(p => p.isProgram() ) as Babel.NodePath; - setContentAttribute(componentName, t, path, state, ddValues, programPath); + setContentAttribute( + componentName, + t, + path, + state, + ddValues, + hoistedDdValues, + programPath + ); // Wrap every actionable handler attribute with RUM for (const attrPath of actionPathList) { attrPath.node.extra = { ...attrPath.node.extra, - ddValues + ddValues, + hoistedDdValues }; handleRumActions(t, attrPath, state, componentNameList); } @@ -390,24 +408,24 @@ function jsxChildToRuntimeCall( * - the element's children (rendered via a React.createElement(Fragment) call) * - common label-like props (`trackingLabel`, `title`, `label`, `text`, plus an optional custom prop) * - * @param componentName Host component name (controls whether content is used). - * @param t Babel types helper. - * @param path JSXElement path. - * @param state Plugin state with trackedComponents metadata. - * @param ddValues Mutable map of computed values attached to attributes via `node.extra.ddValues`. - * @param programPath Program path for inserting jsx-runtime imports. + * A second resolver is attached into `hoistedDdValues`, leaving out any candidate or + * children that reference bindings not visible from the component body (e.g., `.map()` params). + * + * @param componentName Host component name (controls whether content is used). + * @param t Babel types helper. + * @param path JSXElement path. + * @param state Plugin state with trackedComponents metadata. + * @param ddValues Mutable map of computed values attached to attributes via `node.extra.ddValues`. + * @param hoistedDdValues Mutable map of values used when the args object is hoisted into a memoization hook. + * @param programPath Program path for inserting jsx-runtime imports. */ export function setContentAttribute( componentName: string, t: typeof Babel.types, path: Babel.NodePath, state: PluginPassState, - ddValues: Record< - string, - | Babel.types.ArrayExpression - | Babel.types.ArrowFunctionExpression - | Babel.types.ObjectExpression - >, + ddValues: DdValues, + hoistedDdValues: DdValues, programPath: Babel.NodePath ) { const componentData = state.trackedComponents?.[componentName]; @@ -419,17 +437,27 @@ export function setContentAttribute( LABEL_PROPS.push(componentData.contentProp); } + const attrPaths = path.get( + 'openingElement.attributes' + ) as Babel.NodePath< + Babel.types.JSXAttribute | Babel.types.JSXSpreadAttribute + >[]; + + // Where the args object ends up for memoized handlers + const hoistPath = getHoistTargetPath(path); + // Retrieve literal/expr values from matching attributes const candidates: Babel.types.Expression[] = []; + const hoistedCandidates: Babel.types.Expression[] = []; for (const name of LABEL_PROPS) { - const attr = (path.node.openingElement.attributes as ( - | Babel.types.JSXAttribute - | Babel.types.JSXSpreadAttribute - )[]).find( - a => t.isJSXAttribute(a) && t.isJSXIdentifier(a.name, { name }) - ) as Babel.types.JSXAttribute | undefined; - - if (!attr) { + const attrPath = attrPaths.find( + a => + t.isJSXAttribute(a.node) && + t.isJSXIdentifier(a.node.name, { name }) + ) as Babel.NodePath | undefined; + + const attr = attrPath?.node; + if (!attrPath || !attr) { continue; } @@ -439,10 +467,19 @@ export function setContentAttribute( if (t.isStringLiteral(attr.value)) { candidates.push(attr.value); + hoistedCandidates.push(attr.value); } else if (t.isJSXExpressionContainer(attr.value)) { - candidates.push( - attr.value.expression as Babel.types.Expression - ); + const expression = attr.value + .expression as Babel.types.Expression; + candidates.push(expression); + + const exprPath = attrPath.get( + 'value.expression' + ) as Babel.NodePath; + + if (isExpressionInScope(exprPath, hoistPath)) { + hoistedCandidates.push(expression); + } } } @@ -453,58 +490,96 @@ export function setContentAttribute( .map(child => jsxChildToRuntimeCall(t, child, programPath)) .filter((x): x is Babel.types.Expression => x !== null); - const fragmentId = addNamed( - programPath, - 'Fragment', - 'react/jsx-runtime' - ); - - const childrenProp = - convertedChildren.length === 1 - ? t.objectProperty( - t.identifier('children'), - convertedChildren[0] - ) - : convertedChildren.length > 1 - ? t.objectProperty( - t.identifier('children'), - t.arrayExpression(convertedChildren) - ) - : null; - - const fragmentProps = childrenProp - ? t.objectExpression([childrenProp]) - : t.objectExpression([]); - - const runtimeFn = - convertedChildren.length > 1 - ? addNamed(programPath, 'jsxs', 'react/jsx-runtime') - : addNamed(programPath, 'jsx', 'react/jsx-runtime'); - - const fragment = t.callExpression(t.cloneNode(runtimeFn), [ - t.cloneNode(fragmentId), - fragmentProps - ]); + // Share jsx-runtime imports between both resolvers + const runtimeIds: Record = {}; + const getRuntimeId = (name: string) => { + if (!runtimeIds[name]) { + runtimeIds[name] = addNamed( + programPath, + name, + 'react/jsx-runtime' + ) as Babel.types.Identifier; + } + return runtimeIds[name]; + }; - // () => __ddExtractText(jsxs(Fragment, { children: [...] }), [candidates...]) - const getContentNode = t.arrowFunctionExpression( - [], - t.blockStatement([ - t.returnStatement( - t.callExpression(t.identifier('__ddExtractText'), [ - fragment, - t.arrayExpression( - candidates.map(e => t.cloneNode(e, true)) - ) - ]) - ) - ]) + ddValues.getContent = getContentNode( + t, + convertedChildren, + candidates, + getRuntimeId ); - ddValues.getContent = getContentNode; + const childrenInScope = path + .get('children') + .every(child => isExpressionInScope(child, hoistPath)); + + hoistedDdValues.getContent = + childrenInScope && hoistedCandidates.length === candidates.length + ? ddValues.getContent + : getContentNode( + t, + childrenInScope ? convertedChildren : [], + hoistedCandidates, + getRuntimeId + ); } } +/** + * Builds the `getContent` resolver: + * `() => __ddExtractText(jsxs(Fragment, { children: [...] }), [candidates...])` + * + * @param t Babel types helper. + * @param convertedChildren Children already converted to jsx()/jsxs() runtime calls. + * @param candidates Label-like prop values. + * @param getRuntimeId Returns the local identifier for a `react/jsx-runtime` import. + */ +function getContentNode( + t: typeof Babel.types, + convertedChildren: Babel.types.Expression[], + candidates: Babel.types.Expression[], + getRuntimeId: (name: string) => Babel.types.Identifier +) { + const fragmentId = getRuntimeId('Fragment'); + + const childrenProp = + convertedChildren.length === 1 + ? t.objectProperty(t.identifier('children'), convertedChildren[0]) + : convertedChildren.length > 1 + ? t.objectProperty( + t.identifier('children'), + t.arrayExpression(convertedChildren) + ) + : null; + + const fragmentProps = childrenProp + ? t.objectExpression([childrenProp]) + : t.objectExpression([]); + + const runtimeFn = + convertedChildren.length > 1 + ? getRuntimeId('jsxs') + : getRuntimeId('jsx'); + + const fragment = t.callExpression(t.cloneNode(runtimeFn), [ + t.cloneNode(fragmentId), + fragmentProps + ]); + + return t.arrowFunctionExpression( + [], + t.blockStatement([ + t.returnStatement( + t.callExpression(t.identifier('__ddExtractText'), [ + fragment, + t.arrayExpression(candidates.map(e => t.cloneNode(e, true))) + ]) + ) + ]) + ); +} + /** * Scans a JSXElement and derives: * - `actionPathList`: attribute paths to wrap (based on configured handler names) @@ -532,12 +607,11 @@ export function getJSXElementActionPaths( ].filter(Boolean); // Map for dd attributes and misc options - const ddValues: Record< - string, - | Babel.types.ArrayExpression - | Babel.types.ArrowFunctionExpression - | Babel.types.ObjectExpression - > = {}; + const ddValues: DdValues = {}; + + // Same as `ddValues`, restricted to values that stay valid when the args object + // is hoisted into a `useCallback`/`useMemo` call in the component body + const hoistedDdValues: DdValues = {}; // Handler names to consider actionable (e.g., onPress, onLongPress) const actionMapList = @@ -568,6 +642,10 @@ export function getJSXElementActionPaths( ]); } + if (ddValues['options']) { + hoistedDdValues['options'] = ddValues['options']; + } + // Traverse attributes to collect DD props and actionable handlers path.traverse({ JSXAttribute(subpath) { @@ -580,20 +658,46 @@ export function getJSXElementActionPaths( return; } - // Collect literal DD attributes into arrays inside ddValues + // Collect DD attributes into arrays inside ddValues // Required for handling `CompoundComponents` const isValidAttr = ddAttrs.includes(attrName); if (isValidAttr) { const data = subpath.node.value; if (t.isStringLiteral(data)) { - if (!ddValues[attrName]) { - ddValues[attrName] = t.arrayExpression([]); + pushDdValue(t, ddValues, attrName, data); + pushDdValue(t, hoistedDdValues, attrName, data); + } else if ( + t.isJSXExpressionContainer(data) && + !t.isJSXEmptyExpression(data.expression) + ) { + // Expressions are evaluated at runtime inside the generated wrapper, + // so they are only forwarded when their bindings are visible there + const exprPath = subpath.get( + 'value.expression' + ) as Babel.NodePath; + + if (!isExpressionInScope(exprPath, path)) { + return; } - const valuesArray = ddValues[attrName]; - if (t.isArrayExpression(valuesArray)) { - valuesArray.elements.push(data); + pushDdValue( + t, + ddValues, + attrName, + t.cloneNode(data.expression, true) + ); + + // Memoized handlers hoist the args object to the component body + if ( + isExpressionInScope(exprPath, getHoistTargetPath(path)) + ) { + pushDdValue( + t, + hoistedDdValues, + attrName, + t.cloneNode(data.expression, true) + ); } } @@ -616,7 +720,52 @@ export function getJSXElementActionPaths( } }); - return { actionPathList, actionPathNames, ddValues }; + return { actionPathList, actionPathNames, ddValues, hoistedDdValues }; +} + +/** + * Returns the path whose scope matches where memoized handlers get their args object + * hoisted to: the nearest enclosing component (same lookup as `handleMemoization`), + * or the Program when there is none. + * + * @param path JSXElement path. + */ +function getHoistTargetPath( + path: Babel.NodePath +): Babel.NodePath { + return ( + path.findParent( + p => + p.isFunctionDeclaration() || + p.isFunctionExpression() || + p.isClassDeclaration() || + p.isProgram() + ) || path + ); +} + +/** + * Appends a value to the array stored under `attrName`, creating it if needed. + * + * @param t Babel types helper. + * @param values Map of computed values (`ddValues` or `hoistedDdValues`). + * @param attrName Attribute name used as key. + * @param value Expression to append. + */ +function pushDdValue( + t: typeof Babel.types, + values: DdValues, + attrName: string, + value: Babel.types.Expression +) { + if (!values[attrName]) { + values[attrName] = t.arrayExpression([]); + } + + const valuesArray = values[attrName]; + if (t.isArrayExpression(valuesArray)) { + valuesArray.elements.push(value); + } } /** diff --git a/packages/react-native-babel-plugin/src/actions/rum/tap.ts b/packages/react-native-babel-plugin/src/actions/rum/tap.ts index 1aa7e5665..853e5cefc 100644 --- a/packages/react-native-babel-plugin/src/actions/rum/tap.ts +++ b/packages/react-native-babel-plugin/src/actions/rum/tap.ts @@ -70,20 +70,30 @@ export function handleTapAction( (path.node.extra.ddValues as any)['handlerArgs'] = handlerArgs; } - const argsObject = t.objectExpression([ - ...Object.entries(path.node?.extra?.ddValues || {}).map( - ([key, value]) => { + if (path.node?.extra?.hoistedDdValues) { + (path.node.extra.hoistedDdValues as any)['handlerArgs'] = handlerArgs; + } + + const getArgsObject = (ddValues: unknown) => + t.objectExpression([ + ...Object.entries(ddValues || {}).map(([key, value]) => { return t.objectProperty( t.stringLiteral(key), toExpression(t, value) ); - } - ), - t.objectProperty( - t.stringLiteral('componentName'), - t.stringLiteral(parentName) - ) - ]); + }), + t.objectProperty( + t.stringLiteral('componentName'), + t.stringLiteral(parentName) + ) + ]); + + const argsObject = getArgsObject(path.node?.extra?.ddValues); + + // Memoized handlers are wrapped in the component body, outside the JSX scope + const hoistedArgsObject = path.node?.extra?.hoistedDdValues + ? getArgsObject(path.node.extra.hoistedDdValues) + : argsObject; // Used only for function references // Wraps our custom logic at the level of the memoization call insead of on the property level (default behaviour) @@ -99,7 +109,7 @@ export function handleTapAction( expression, fName, fNode, - argsObject, + hoistedArgsObject, handler?.mode ); diff --git a/packages/react-native-babel-plugin/src/utils/nodeProcessing.ts b/packages/react-native-babel-plugin/src/utils/nodeProcessing.ts index 4ff102214..bd69fc69f 100644 --- a/packages/react-native-babel-plugin/src/utils/nodeProcessing.ts +++ b/packages/react-native-babel-plugin/src/utils/nodeProcessing.ts @@ -603,3 +603,56 @@ export function findIdentifierInScope( return foundValue; } + +/** + * Checks whether an expression can be safely evaluated at a different location + * than where it was written, i.e. whether every binding it references resolves + * to the same binding from `targetPath`'s scope. Globals are always visible. + * + * @param exprPath Path of the expression to check. + * @param targetPath Path where the expression would be evaluated. + * @returns `true` if all referenced bindings are visible from the target location. + */ +export function isExpressionInScope( + exprPath: Babel.NodePath, + targetPath: Babel.NodePath +): boolean { + let inScope = true; + + const check = ( + path: Babel.NodePath + ) => { + if (!inScope || !path.isReferencedIdentifier()) { + return; + } + + const name = path.node.name; + const binding = path.scope.getBinding(name); + + if (binding && targetPath.scope.getBinding(name) !== binding) { + inScope = false; + } + }; + + if (exprPath.isIdentifier() || exprPath.isJSXIdentifier()) { + check(exprPath); + return inScope; + } + + exprPath.traverse({ + Identifier(path) { + check(path); + if (!inScope) { + path.stop(); + } + }, + JSXIdentifier(path) { + check(path); + if (!inScope) { + path.stop(); + } + } + }); + + return inScope; +} diff --git a/packages/react-native-babel-plugin/test/actionNameAttribute.test.ts b/packages/react-native-babel-plugin/test/actionNameAttribute.test.ts new file mode 100644 index 000000000..4ff4c2290 --- /dev/null +++ b/packages/react-native-babel-plugin/test/actionNameAttribute.test.ts @@ -0,0 +1,269 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2016-Present Datadog, Inc. + */ + +import { transform } from '@babel/core'; + +import plugin from '../src/index'; +import type { PluginOptions } from '../src/types'; + +// Covers how DD attribute values (`dd-action-name`, `accessibilityLabel` and the custom +// `actionNameAttribute`) reach the runtime depending on the JSX attribute value shape. + +function transformCode(code: string) { + const options: PluginOptions = { + actionNameAttribute: 'testID', + components: { + useContent: true, + useNamePrefix: false, + tracked: [ + { + name: 'CustomButton', + handlers: [{ event: 'onPress', action: 'TAP' }] + } + ] + }, + sessionReplay: { svgTracking: false } + } as PluginOptions; + + return transform(code, { + filename: 'file.tsx', + presets: [ + ['@babel/preset-react', { runtime: 'classic' }], + '@babel/preset-typescript' + ], + plugins: [ + [plugin, options], + '@babel/plugin-transform-modules-commonjs' + ], + configFile: false + })?.code as string; +} + +/** + * Transforms `code`, executes it with stubbed modules, presses every rendered + * element and returns the `targetObject` each press handed to `wrapRumAction`. + */ +function pressAll(code: string, modules: Record = {}) { + const output = transformCode(code); + const captured: Record[] = []; + const rendered: Record[] = []; + + const DdBabelInteractionTracking = { + getInstance: () => DdBabelInteractionTracking, + wrapRumAction: ( + fn: (...a: any[]) => any, + _type: string, + target: Record + ) => (...args: any[]) => { + captured.push(target); + return fn?.(...args); + } + }; + + const React = { + createElement: (_type: any, props: any) => { + rendered.push(props || {}); + return { props }; + }, + Fragment: 'Fragment', + useCallback: (fn: any) => fn + }; + + const allModules: Record = { + react: React, + 'react/jsx-runtime': { + jsx: React.createElement, + jsxs: React.createElement, + Fragment: 'Fragment' + }, + 'react-native': { + Button: 'Button', + Pressable: 'Pressable', + Text: 'Text' + }, + '@datadog/mobile-react-native': { + DdBabelInteractionTracking, + __ddExtractText: () => [] + }, + './CustomButton': { CustomButton: 'CustomButton' }, + ...modules + }; + + const exportsObj: Record = {}; + // eslint-disable-next-line no-new-func + new Function('require', 'exports', 'React', output)( + (name: string) => allModules[name], + exportsObj, + React + ); + if (typeof exportsObj.render === 'function') { + exportsObj.render(); + } + + for (const props of rendered) { + props.onPress?.(); + } + + return { output, captured }; +} + +describe('Babel plugin: DD attribute values', () => { + const testIDs = { './testIDs': { TestIDs: { someTestId: 'imported_id' } } }; + + it('forwards string literal attributes', () => { + const { captured } = pressAll(` + import { Button } from 'react-native'; +