diff --git a/CHANGELOG.md b/CHANGELOG.md index 62a776ed..90c016f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased - "You know what they say ‘Fool me once, strike one, but fool me twice… strike three.’" — Michael Scott +- fix(react-native): Enhance fragment detection for indirect references ([#767](https://github.com/getsentry/sentry-javascript-bundler-plugins/pull/767)) ## 4.1.0 diff --git a/packages/babel-plugin-component-annotate/.eslintrc.js b/packages/babel-plugin-component-annotate/.eslintrc.js index 42b35d84..01f80fed 100644 --- a/packages/babel-plugin-component-annotate/.eslintrc.js +++ b/packages/babel-plugin-component-annotate/.eslintrc.js @@ -9,6 +9,9 @@ module.exports = { tsconfigRootDir: __dirname, project: ["./src/tsconfig.json", "./test/tsconfig.json"], }, + globals: { + Set: "readonly", + }, env: { node: true, }, diff --git a/packages/babel-plugin-component-annotate/src/index.ts b/packages/babel-plugin-component-annotate/src/index.ts index 19f0a519..54163dfd 100644 --- a/packages/babel-plugin-component-annotate/src/index.ts +++ b/packages/babel-plugin-component-annotate/src/index.ts @@ -51,16 +51,46 @@ interface AnnotationOpts { ignoredComponents?: string[]; } +interface FragmentContext { + fragmentAliases: Set; + reactNamespaceAliases: Set; +} + interface AnnotationPluginPass extends PluginPass { opts: AnnotationOpts; + sentryFragmentContext?: FragmentContext; } type AnnotationPlugin = PluginObj; +// Shared context object for all JSX processing functions +interface JSXProcessingContext { + /** Whether to annotate React fragments */ + annotateFragments: boolean; + /** Babel types object */ + t: typeof Babel.types; + /** Name of the React component */ + componentName: string; + /** Source file name (optional) */ + sourceFileName?: string; + /** Array of attribute names [component, element, sourceFile] */ + attributeNames: string[]; + /** Array of component names to ignore */ + ignoredComponents: string[]; + /** Fragment context for identifying React fragments */ + fragmentContext?: FragmentContext; +} + // We must export the plugin as default, otherwise the Babel loader will not be able to resolve it when configured using its string identifier export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): AnnotationPlugin { return { visitor: { + Program: { + enter(path, state) { + const fragmentContext = collectFragmentContext(path); + state.sentryFragmentContext = fragmentContext; + }, + }, FunctionDeclaration(path, state) { if (!path.node.id || !path.node.id.name) { return; @@ -69,15 +99,8 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): return; } - functionBodyPushAttributes( - state.opts["annotate-fragments"] === true, - t, - path, - path.node.id.name, - sourceFileNameFromState(state), - attributeNamesFromState(state), - state.opts.ignoredComponents ?? [] - ); + const context = createJSXProcessingContext(state, t, path.node.id.name); + functionBodyPushAttributes(context, path); }, ArrowFunctionExpression(path, state) { // We're expecting a `VariableDeclarator` like `const MyComponent =` @@ -97,15 +120,8 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): return; } - functionBodyPushAttributes( - state.opts["annotate-fragments"] === true, - t, - path, - parent.id.name, - sourceFileNameFromState(state), - attributeNamesFromState(state), - state.opts.ignoredComponents ?? [] - ); + const context = createJSXProcessingContext(state, t, parent.id.name); + functionBodyPushAttributes(context, path); }, ClassDeclaration(path, state) { const name = path.get("id"); @@ -118,7 +134,7 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): return; } - const ignoredComponents = state.opts.ignoredComponents ?? []; + const context = createJSXProcessingContext(state, t, name.node?.name || ""); render.traverse({ ReturnStatement(returnStatement) { @@ -128,15 +144,7 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): return; } - processJSX( - state.opts["annotate-fragments"] === true, - t, - arg, - name.node && name.node.name, - sourceFileNameFromState(state), - attributeNamesFromState(state), - ignoredComponents - ); + processJSX(context, arg); }, }); }, @@ -144,14 +152,33 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): }; } -function functionBodyPushAttributes( - annotateFragments: boolean, +/** + * Creates a JSX processing context from the plugin state + */ +function createJSXProcessingContext( + state: AnnotationPluginPass, t: typeof Babel.types, - path: Babel.NodePath, - componentName: string, - sourceFileName: string | undefined, - attributeNames: string[], - ignoredComponents: string[] + componentName: string +): JSXProcessingContext { + return { + annotateFragments: state.opts["annotate-fragments"] === true, + t, + componentName, + sourceFileName: sourceFileNameFromState(state), + attributeNames: attributeNamesFromState(state), + ignoredComponents: state.opts.ignoredComponents ?? [], + fragmentContext: state.sentryFragmentContext, + }; +} + +/** + * Processes the body of a function to add Sentry tracking attributes to JSX elements. + * Handles various function body structures including direct JSX returns, conditional expressions, + * and nested JSX elements. + */ +function functionBodyPushAttributes( + context: JSXProcessingContext, + path: Babel.NodePath ): void { let jsxNode: Babel.NodePath; @@ -193,27 +220,11 @@ function functionBodyPushAttributes( if (arg.isConditionalExpression()) { const consequent = arg.get("consequent"); if (consequent.isJSXFragment() || consequent.isJSXElement()) { - processJSX( - annotateFragments, - t, - consequent, - componentName, - sourceFileName, - attributeNames, - ignoredComponents - ); + processJSX(context, consequent); } const alternate = arg.get("alternate"); if (alternate.isJSXFragment() || alternate.isJSXElement()) { - processJSX( - annotateFragments, - t, - alternate, - componentName, - sourceFileName, - attributeNames, - ignoredComponents - ); + processJSX(context, alternate); } return; } @@ -229,29 +240,26 @@ function functionBodyPushAttributes( return; } - processJSX( - annotateFragments, - t, - jsxNode, - componentName, - sourceFileName, - attributeNames, - ignoredComponents - ); + processJSX(context, jsxNode); } +/** + * Recursively processes JSX elements to add Sentry tracking attributes. + * Handles both JSX elements and fragments, applying appropriate attributes + * based on configuration and component context. + */ function processJSX( - annotateFragments: boolean, - t: typeof Babel.types, + context: JSXProcessingContext, jsxNode: Babel.NodePath, - componentName: string | null, - sourceFileName: string | undefined, - attributeNames: string[], - ignoredComponents: string[] + componentName?: string ): void { if (!jsxNode) { return; } + + // Use provided componentName or fall back to context componentName + const currentComponentName = componentName ?? context.componentName; + // NOTE: I don't know of a case where `openingElement` would have more than one item, // but it's safer to always iterate const paths = jsxNode.get("openingElement"); @@ -259,12 +267,9 @@ function processJSX( openingElements.forEach((openingElement) => { applyAttributes( - t, + context, openingElement as Babel.NodePath, - componentName, - sourceFileName, - attributeNames, - ignoredComponents + currentComponentName ); }); @@ -275,7 +280,7 @@ function processJSX( children = [children]; } - let shouldSetComponentName = annotateFragments; + let shouldSetComponentName = context.annotateFragments; children.forEach((child) => { // Happens for some node types like plain text @@ -293,47 +298,37 @@ function processJSX( if (shouldSetComponentName && openingElement && openingElement.node) { shouldSetComponentName = false; - processJSX( - annotateFragments, - t, - child, - componentName, - sourceFileName, - attributeNames, - ignoredComponents - ); + processJSX(context, child, currentComponentName); } else { - processJSX( - annotateFragments, - t, - child, - null, - sourceFileName, - attributeNames, - ignoredComponents - ); + processJSX(context, child, ""); } }); } +/** + * Applies Sentry tracking attributes to a JSX opening element. + * Adds component name, element name, and source file attributes while + * respecting ignore lists and fragment detection. + */ function applyAttributes( - t: typeof Babel.types, + context: JSXProcessingContext, openingElement: Babel.NodePath, - componentName: string | null, - sourceFileName: string | undefined, - attributeNames: string[], - ignoredComponents: string[] + componentName: string ): void { + const { t, attributeNames, ignoredComponents, fragmentContext, sourceFileName } = context; const [componentAttributeName, elementAttributeName, sourceFileAttributeName] = attributeNames; - if (isReactFragment(t, openingElement)) { - return; - } // e.g., Raw JSX text like the `A` in `

a

` if (!openingElement.node) { return; } + // Check if this is a React fragment - if so, skip attribute addition entirely + const isFragment = isReactFragment(t, openingElement, fragmentContext); + if (isFragment) { + return; + } + if (!openingElement.node.attributes) openingElement.node.attributes = []; const elementName = getPathName(t, openingElement); @@ -343,15 +338,11 @@ function applyAttributes( // Add a stable attribute for the element name but only for non-DOM names let isAnIgnoredElement = false; - if ( - !isAnIgnoredComponent && - !hasAttributeWithName(openingElement, componentAttributeName) && - (componentAttributeName !== elementAttributeName || !componentName) - ) { + if (!isAnIgnoredComponent && !hasAttributeWithName(openingElement, elementAttributeName)) { if (DEFAULT_IGNORED_ELEMENTS.includes(elementName)) { isAnIgnoredElement = true; } else { - // TODO: Is it possible to avoid this null check? + // Always add element attribute for non-ignored elements if (elementAttributeName) { openingElement.node.attributes.push( t.jSXAttribute(t.jSXIdentifier(elementAttributeName), t.stringLiteral(elementName)) @@ -366,7 +357,6 @@ function applyAttributes( !isAnIgnoredComponent && !hasAttributeWithName(openingElement, componentAttributeName) ) { - // TODO: Is it possible to avoid this null check? if (componentAttributeName) { openingElement.node.attributes.push( t.jSXAttribute(t.jSXIdentifier(componentAttributeName), t.stringLiteral(componentName)) @@ -374,14 +364,16 @@ function applyAttributes( } } - // Add a stable attribute for the source file name (absent for non-root elements) + // Add a stable attribute for the source file name + // Updated condition: add source file for elements that have either: + // 1. A component name (root elements), OR + // 2. An element name that's not ignored (child elements) if ( sourceFileName && !isAnIgnoredComponent && - (componentName || isAnIgnoredElement === false) && + (componentName || !isAnIgnoredElement) && !hasAttributeWithName(openingElement, sourceFileAttributeName) ) { - // TODO: Is it possible to avoid this null check? if (sourceFileAttributeName) { openingElement.node.attributes.push( t.jSXAttribute(t.jSXIdentifier(sourceFileAttributeName), t.stringLiteral(sourceFileName)) @@ -443,18 +435,110 @@ function attributeNamesFromState(state: AnnotationPluginPass): [string, string, return [webComponentName, webElementName, webSourceFileName]; } -function isReactFragment(t: typeof Babel.types, openingElement: Babel.NodePath): boolean { +function collectFragmentContext(programPath: Babel.NodePath): FragmentContext { + const fragmentAliases = new Set(); + const reactNamespaceAliases = new Set(["React"]); // Default React namespace + + programPath.traverse({ + ImportDeclaration(importPath) { + const source = importPath.node.source.value; + + // Handle React imports + if (source === "react" || source === "React") { + importPath.node.specifiers.forEach((spec) => { + if (spec.type === "ImportSpecifier" && spec.imported.type === "Identifier") { + // Detect aliased React.Fragment imports (e.g., `Fragment as F`) + // so we can later identify as a fragment in JSX. + if (spec.imported.name === "Fragment") { + fragmentAliases.add(spec.local.name); + } + } else if ( + spec.type === "ImportDefaultSpecifier" || + spec.type === "ImportNamespaceSpecifier" + ) { + // import React from 'react' -> React OR + // import * as React from 'react' -> React + reactNamespaceAliases.add(spec.local.name); + } + }); + } + }, + + // Handle simple variable assignments only (avoid complex cases) + VariableDeclarator(varPath) { + if (varPath.node.init) { + const init = varPath.node.init; + + // Handle identifier assignments: const MyFragment = Fragment + if (varPath.node.id.type === "Identifier") { + // Handle: const MyFragment = Fragment (only if Fragment is a known alias) + if (init.type === "Identifier" && fragmentAliases.has(init.name)) { + fragmentAliases.add(varPath.node.id.name); + } + + // Handle: const MyFragment = React.Fragment (only for known React namespaces) + if ( + init.type === "MemberExpression" && + init.object.type === "Identifier" && + init.property.type === "Identifier" && + init.property.name === "Fragment" && + reactNamespaceAliases.has(init.object.name) + ) { + fragmentAliases.add(varPath.node.id.name); + } + } + + // Handle destructuring assignments: const { Fragment } = React + if (varPath.node.id.type === "ObjectPattern") { + if (init.type === "Identifier" && reactNamespaceAliases.has(init.name)) { + const properties = varPath.node.id.properties; + + for (const prop of properties) { + if ( + prop.type === "ObjectProperty" && + prop.key && + prop.key.type === "Identifier" && + prop.value && + prop.value.type === "Identifier" && + prop.key.name === "Fragment" + ) { + fragmentAliases.add(prop.value.name); + } + } + } + } + } + }, + }); + + return { fragmentAliases, reactNamespaceAliases }; +} + +function isReactFragment( + t: typeof Babel.types, + openingElement: Babel.NodePath, + context?: FragmentContext // Add this optional parameter +): boolean { + // Handle JSX fragments (<>) if (openingElement.isJSXFragment()) { return true; } const elementName = getPathName(t, openingElement); + // Direct fragment references if (elementName === "Fragment" || elementName === "React.Fragment") { return true; } // TODO: All these objects are typed as unknown, maybe an oversight in Babel types? + + // Check if the element name is a known fragment alias + if (context && elementName && context.fragmentAliases.has(elementName)) { + return true; + } + + // Handle JSXMemberExpression if ( openingElement.node && "name" in openingElement.node && @@ -463,10 +547,6 @@ function isReactFragment(t: typeof Babel.types, openingElement: Babel.NodePath): "type" in openingElement.node.name && openingElement.node.name.type === "JSXMemberExpression" ) { - if (!("name" in openingElement.node)) { - return false; - } - const nodeName = openingElement.node.name; if (typeof nodeName !== "object" || !nodeName) { return false; @@ -487,9 +567,26 @@ function isReactFragment(t: typeof Babel.types, openingElement: Babel.NodePath): const objectName = "name" in nodeNameObject && nodeNameObject.name; const propertyName = "name" in nodeNameProperty && nodeNameProperty.name; + // React.Fragment check if (objectName === "React" && propertyName === "Fragment") { return true; } + + // Enhanced checks using context + if (context) { + // Check React.Fragment pattern with known React namespaces + if ( + context.reactNamespaceAliases.has(objectName as string) && + propertyName === "Fragment" + ) { + return true; + } + + // Check MyFragment.Fragment pattern + if (context.fragmentAliases.has(objectName as string) && propertyName === "Fragment") { + return true; + } + } } } diff --git a/packages/babel-plugin-component-annotate/test/__snapshots__/test-plugin.test.ts.snap b/packages/babel-plugin-component-annotate/test/__snapshots__/test-plugin.test.ts.snap index af43b6c0..8a2951dc 100644 --- a/packages/babel-plugin-component-annotate/test/__snapshots__/test-plugin.test.ts.snap +++ b/packages/babel-plugin-component-annotate/test/__snapshots__/test-plugin.test.ts.snap @@ -1,5 +1,189 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`Fragment Detection combines all fragment patterns correctly 1`] = ` +"import React, { Fragment as ImportedF } from 'react'; +import * as MyReact from 'react'; +const { + Fragment: DestructuredF +} = React; +const { + Fragment +} = MyReact; +const AssignedF = Fragment; // ← This uses the destructured Fragment from MyReact + +export default function TestComponent() { + return /*#__PURE__*/React.createElement(\\"div\\", { + className: \\"container\\", + \\"data-sentry-component\\": \\"TestComponent\\", + \\"data-sentry-source-file\\": \\"filename-test.js\\" + }, /*#__PURE__*/React.createElement(React.Fragment, null, /*#__PURE__*/React.createElement(\\"span\\", null, \\"JSX Fragment content\\")), /*#__PURE__*/React.createElement(ImportedF, null, /*#__PURE__*/React.createElement(\\"span\\", null, \\"Imported alias content\\")), /*#__PURE__*/React.createElement(DestructuredF, null, /*#__PURE__*/React.createElement(\\"span\\", null, \\"Destructured content\\")), /*#__PURE__*/React.createElement(Fragment, null, /*#__PURE__*/React.createElement(\\"span\\", null, \\"Namespace destructured content\\")), /*#__PURE__*/React.createElement(AssignedF, null, /*#__PURE__*/React.createElement(\\"span\\", null, \\"Variable assigned content\\")), /*#__PURE__*/React.createElement(React.Fragment, null, /*#__PURE__*/React.createElement(\\"span\\", null, \\"React.Fragment content\\")), /*#__PURE__*/React.createElement(MyReact.Fragment, null, /*#__PURE__*/React.createElement(\\"span\\", null, \\"Namespace Fragment content\\"))); +}" +`; + +exports[`Fragment Detection handles Fragment aliased correctly when used by other non-Fragment components in a different scope 1`] = ` +"import { Fragment as OriginalF } from 'react'; +import { OtherComponent } from 'some-library'; +function TestComponent() { + const F = OriginalF; + + // Use Fragment alias - should be ignored + return /*#__PURE__*/React.createElement(F, null, /*#__PURE__*/React.createElement(\\"div\\", null, \\"This should NOT have data-sentry-element (Fragment)\\")); +} +function AnotherComponent() { + // Different component with same alias name in different function scope + const F = OtherComponent; + return /*#__PURE__*/React.createElement(F, null, /*#__PURE__*/React.createElement(\\"div\\", null, \\"This SHOULD have data-sentry-element (not Fragment)\\")); +}" +`; + +exports[`Fragment Detection handles complex variable assignment chains 1`] = ` +"import { Fragment } from 'react'; +const MyFragment = Fragment; +const AnotherFragment = MyFragment; +export default function TestComponent() { + return /*#__PURE__*/React.createElement(AnotherFragment, null, /*#__PURE__*/React.createElement(\\"div\\", null, \\"Content in chained fragment\\")); +}" +`; + +exports[`Fragment Detection handles destructuring from aliased React imports 1`] = ` +"import MyReact from 'react'; +const { + Fragment +} = MyReact; +export default function TestComponent() { + return /*#__PURE__*/React.createElement(Fragment, null, /*#__PURE__*/React.createElement(\\"div\\", null, \\"Content from aliased React destructuring\\")); +}" +`; + +exports[`Fragment Detection handles destructuring from namespace imports 1`] = ` +"import * as ReactLib from 'react'; +const { + Fragment: F +} = ReactLib; +export default function TestComponent() { + return /*#__PURE__*/React.createElement(F, null, /*#__PURE__*/React.createElement(\\"div\\", null, \\"Content from namespace destructuring\\")); +}" +`; + +exports[`Fragment Detection handles multiple destructuring patterns in one file 1`] = ` +"import React from 'react'; +import * as MyReact from 'react'; +const { + Fragment +} = React; +const { + Fragment: AliasedFrag +} = MyReact; +export default function TestComponent() { + return /*#__PURE__*/React.createElement(\\"div\\", { + \\"data-sentry-component\\": \\"TestComponent\\", + \\"data-sentry-source-file\\": \\"filename-test.js\\" + }, /*#__PURE__*/React.createElement(Fragment, null, /*#__PURE__*/React.createElement(\\"span\\", null, \\"Regular destructured\\")), /*#__PURE__*/React.createElement(AliasedFrag, null, /*#__PURE__*/React.createElement(\\"p\\", null, \\"Aliased destructured\\"))); +}" +`; + +exports[`Fragment Detection ignores Fragment assigned to variable 1`] = ` +"import { Fragment } from 'react'; +const MyFragment = Fragment; +export default function TestComponent() { + return /*#__PURE__*/React.createElement(MyFragment, null, /*#__PURE__*/React.createElement(\\"div\\", null, \\"Content in variable fragment\\")); +}" +`; + +exports[`Fragment Detection ignores Fragment from React destructuring 1`] = ` +"import React from 'react'; +const { + Fragment +} = React; +export default function TestComponent() { + return /*#__PURE__*/React.createElement(Fragment, null, /*#__PURE__*/React.createElement(\\"div\\", null, \\"Content in destructured fragment\\")); +}" +`; + +exports[`Fragment Detection ignores Fragment from mixed destructuring 1`] = ` +"import React from 'react'; +const { + Fragment, + createElement, + useState +} = React; +export default function TestComponent() { + return /*#__PURE__*/React.createElement(Fragment, null, /*#__PURE__*/React.createElement(\\"div\\", null, \\"Content with other destructured items\\")); +}" +`; + +exports[`Fragment Detection ignores Fragment imported with alias 1`] = ` +"import { Fragment as F } from 'react'; +export default function TestComponent() { + return /*#__PURE__*/React.createElement(F, null, /*#__PURE__*/React.createElement(\\"div\\", null, \\"Content in aliased fragment\\")); +}" +`; + +exports[`Fragment Detection ignores Fragment with React namespace alias 1`] = ` +"import * as MyReact from 'react'; +export default function TestComponent() { + return /*#__PURE__*/React.createElement(MyReact.Fragment, null, /*#__PURE__*/React.createElement(\\"div\\", null, \\"Content in namespaced fragment\\")); +}" +`; + +exports[`Fragment Detection ignores Fragment with destructuring alias 1`] = ` +"import React from 'react'; +const { + Fragment: MyFragment +} = React; +export default function TestComponent() { + return /*#__PURE__*/React.createElement(MyFragment, null, /*#__PURE__*/React.createElement(\\"div\\", null, \\"Content in aliased destructured fragment\\")); +}" +`; + +exports[`Fragment Detection ignores JSX fragments (<>) 1`] = ` +"export default function TestComponent() { + return /*#__PURE__*/React.createElement(React.Fragment, null, /*#__PURE__*/React.createElement(\\"div\\", null, \\"Content in JSX fragment\\"), /*#__PURE__*/React.createElement(\\"span\\", null, \\"More content\\")); +}" +`; + +exports[`Fragment Detection ignores React default import with Fragment 1`] = ` +"import MyReact from 'react'; +export default function TestComponent() { + return /*#__PURE__*/React.createElement(MyReact.Fragment, null, /*#__PURE__*/React.createElement(\\"div\\", null, \\"Content in default import fragment\\")); +}" +`; + +exports[`Fragment Detection ignores React.Fragment with member expression handling 1`] = ` +"import React from 'react'; +export default function TestComponent() { + return /*#__PURE__*/React.createElement(React.Fragment, null, /*#__PURE__*/React.createElement(\\"div\\", null, \\"Content\\")); +}" +`; + +exports[`Fragment Detection ignores multiple fragment patterns in same file 1`] = ` +"import React, { Fragment } from 'react'; +const MyFragment = Fragment; +export default function TestComponent() { + return /*#__PURE__*/React.createElement(\\"div\\", { + \\"data-sentry-component\\": \\"TestComponent\\", + \\"data-sentry-source-file\\": \\"filename-test.js\\" + }, /*#__PURE__*/React.createElement(React.Fragment, null, /*#__PURE__*/React.createElement(\\"div\\", null, \\"JSX Fragment content\\")), /*#__PURE__*/React.createElement(Fragment, null, /*#__PURE__*/React.createElement(\\"span\\", null, \\"Direct Fragment content\\")), /*#__PURE__*/React.createElement(MyFragment, null, /*#__PURE__*/React.createElement(\\"p\\", null, \\"Variable Fragment content\\")), /*#__PURE__*/React.createElement(React.Fragment, null, /*#__PURE__*/React.createElement(\\"h1\\", null, \\"React.Fragment content\\"))); +}" +`; + +exports[`Fragment Detection works with annotate-fragments option disabled 1`] = ` +"import { Fragment as F } from 'react'; +export default function TestComponent() { + return /*#__PURE__*/React.createElement(F, null, /*#__PURE__*/React.createElement(\\"div\\", null, \\"Content\\")); +}" +`; + +exports[`Fragment Detection works with annotate-fragments option enabled 1`] = ` +"import { Fragment as F } from 'react'; +export default function TestComponent() { + return /*#__PURE__*/React.createElement(F, null, /*#__PURE__*/React.createElement(\\"div\\", { + \\"data-sentry-component\\": \\"TestComponent\\", + \\"data-sentry-source-file\\": \\"filename-test.js\\" + }, \\"Content\\")); +}" +`; + exports[`arrow snapshot matches 1`] = ` "import React, { Component } from 'react'; const componentName = () => { @@ -247,13 +431,6 @@ export default function componentName() { }" `; -exports[`ignores React.Fragment with member expression handling 1`] = ` -"import React from 'react'; -export default function TestComponent() { - return /*#__PURE__*/React.createElement(React.Fragment, null, /*#__PURE__*/React.createElement(\\"div\\", null, \\"Content\\")); -}" -`; - exports[`ignores components with member expressions when in ignoredComponents 1`] = ` "import React from 'react'; import { Tab } from '@headlessui/react'; diff --git a/packages/babel-plugin-component-annotate/test/test-plugin.test.ts b/packages/babel-plugin-component-annotate/test/test-plugin.test.ts index 7d6a73e8..a453da77 100644 --- a/packages/babel-plugin-component-annotate/test/test-plugin.test.ts +++ b/packages/babel-plugin-component-annotate/test/test-plugin.test.ts @@ -1256,27 +1256,527 @@ export default function TestComponent() { expect(result?.code).toMatchSnapshot(); }); -it("ignores React.Fragment with member expression handling", () => { - const result = transform( - `import React from 'react'; +describe("Fragment Detection", () => { + it("ignores React.Fragment with member expression handling", () => { + const result = transform( + `import React from 'react'; + + export default function TestComponent() { + return ( + +
Content
+
+ ); + }`, + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + } + ); + + expect(result?.code).toContain("React.createElement(React.Fragment"); + expect(result?.code).not.toContain('"data-sentry-element": "React.Fragment"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("ignores JSX fragments (<>)", () => { + const result = transform( + `export default function TestComponent() { + return ( + <> +
Content in JSX fragment
+ More content + + ); +}`, + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + } + ); + + expect(result?.code).toContain("React.createElement(React.Fragment"); + expect(result?.code).not.toContain('"data-sentry-element": "Fragment"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("ignores Fragment imported with alias", () => { + const result = transform( + `import { Fragment as F } from 'react'; + +export default function TestComponent() { + return ( + +
Content in aliased fragment
+
+ ); +}`, + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + } + ); + + expect(result?.code).toContain("React.createElement(F"); + expect(result?.code).not.toContain('"data-sentry-element": "F"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("ignores Fragment assigned to variable", () => { + const result = transform( + `import { Fragment } from 'react'; + +const MyFragment = Fragment; + +export default function TestComponent() { + return ( + +
Content in variable fragment
+
+ ); +}`, + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + } + ); + + expect(result?.code).toContain("React.createElement(MyFragment"); + expect(result?.code).not.toContain('"data-sentry-element": "MyFragment"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("ignores Fragment with React namespace alias", () => { + const result = transform( + `import * as MyReact from 'react'; + +export default function TestComponent() { + return ( + +
Content in namespaced fragment
+
+ ); +}`, + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + } + ); + + expect(result?.code).toContain("React.createElement(MyReact.Fragment"); + expect(result?.code).not.toContain('"data-sentry-element": "MyReact.Fragment"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("ignores React default import with Fragment", () => { + const result = transform( + `import MyReact from 'react'; + +export default function TestComponent() { + return ( + +
Content in default import fragment
+
+ ); +}`, + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + } + ); + + expect(result?.code).toContain("React.createElement(MyReact.Fragment"); + expect(result?.code).not.toContain('"data-sentry-element": "MyReact.Fragment"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("ignores multiple fragment patterns in same file", () => { + const result = transform( + `import React, { Fragment } from 'react'; + + const MyFragment = Fragment; + + export default function TestComponent() { + return ( +
+ <> +
JSX Fragment content
+ + + + Direct Fragment content + + + +

Variable Fragment content

+
+ + +

React.Fragment content

+
+
+ ); + }`, + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + } + ); + + expect(result?.code).not.toContain('"data-sentry-element": "Fragment"'); + expect(result?.code).not.toContain('"data-sentry-element": "MyFragment"'); + expect(result?.code).not.toContain('"data-sentry-element": "React.Fragment"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("handles complex variable assignment chains", () => { + const result = transform( + `import { Fragment } from 'react'; + + const MyFragment = Fragment; + const AnotherFragment = MyFragment; + + export default function TestComponent() { + return ( + +
Content in chained fragment
+
+ ); + }`, + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + } + ); + + expect(result?.code).not.toContain('"data-sentry-element": "MyFragment"'); + expect(result?.code).not.toContain('"data-sentry-element": "AnotherFragment"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("works with annotate-fragments option disabled", () => { + const result = transform( + `import { Fragment as F } from 'react'; export default function TestComponent() { return ( - +
Content
-
+
); }`, - { - filename: "/filename-test.js", - configFile: false, - presets: ["@babel/preset-react"], - plugins: [plugin], - } + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [[plugin, { "annotate-fragments": false }]], + } + ); + + expect(result?.code).not.toContain('"data-sentry-element": "F"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("works with annotate-fragments option enabled", () => { + const result = transform( + `import { Fragment as F } from 'react'; + +export default function TestComponent() { + return ( + +
Content
+
); +}`, + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [[plugin, { "annotate-fragments": true }]], + } + ); - // React.Fragment should be ignored by default - expect(result?.code).toContain("React.createElement(React.Fragment"); - expect(result?.code).not.toContain('"data-sentry-element": "React.Fragment"'); - expect(result?.code).toMatchSnapshot(); + expect(result?.code).not.toContain('"data-sentry-element": "F"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("ignores Fragment from React destructuring", () => { + const result = transform( + `import React from 'react'; + +const { Fragment } = React; + +export default function TestComponent() { + return ( + +
Content in destructured fragment
+
+ ); +}`, + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + } + ); + + expect(result?.code).not.toContain('"data-sentry-element": "Fragment"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("ignores Fragment with destructuring alias", () => { + const result = transform( + `import React from 'react'; + +const { Fragment: MyFragment } = React; + +export default function TestComponent() { + return ( + +
Content in aliased destructured fragment
+
+ ); +}`, + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + } + ); + + expect(result?.code).not.toContain('"data-sentry-element": "MyFragment"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("ignores Fragment from mixed destructuring", () => { + const result = transform( + `import React from 'react'; + +const { Fragment, createElement, useState } = React; + +export default function TestComponent() { + return ( + +
Content with other destructured items
+
+ ); +}`, + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + } + ); + + expect(result?.code).not.toContain('"data-sentry-element": "Fragment"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("handles destructuring from aliased React imports", () => { + const result = transform( + `import MyReact from 'react'; + +const { Fragment } = MyReact; + +export default function TestComponent() { + return ( + +
Content from aliased React destructuring
+
+ ); +}`, + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + } + ); + + expect(result?.code).not.toContain('"data-sentry-element": "Fragment"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("handles destructuring from namespace imports", () => { + const result = transform( + `import * as ReactLib from 'react'; + +const { Fragment: F } = ReactLib; + +export default function TestComponent() { + return ( + +
Content from namespace destructuring
+
+ ); +}`, + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + } + ); + + expect(result?.code).not.toContain('"data-sentry-element": "F"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("handles multiple destructuring patterns in one file", () => { + const result = transform( + `import React from 'react'; +import * as MyReact from 'react'; + +const { Fragment } = React; +const { Fragment: AliasedFrag } = MyReact; + +export default function TestComponent() { + return ( +
+ + Regular destructured + + + +

Aliased destructured

+
+
+ ); +}`, + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + } + ); + + expect(result?.code).not.toContain('"data-sentry-element": "Fragment"'); + expect(result?.code).not.toContain('"data-sentry-element": "AliasedFrag"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("combines all fragment patterns correctly", () => { + const result = transform( + `import React, { Fragment as ImportedF } from 'react'; + import * as MyReact from 'react'; + + const { Fragment: DestructuredF } = React; + const { Fragment } = MyReact; + const AssignedF = Fragment; // ← This uses the destructured Fragment from MyReact + + export default function TestComponent() { + return ( +
+ {/* JSX Fragment */} + <> + JSX Fragment content + + + {/* Imported alias */} + + Imported alias content + + + {/* Destructured */} + + Destructured content + + + {/* Destructured from namespace */} + + Namespace destructured content + + + {/* Variable assigned */} + + Variable assigned content + + + {/* React.Fragment */} + + React.Fragment content + + + {/* Namespace Fragment */} + + Namespace Fragment content + +
+ ); + }`, + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + } + ); + + expect(result?.code).not.toContain('"data-sentry-element": "ImportedF"'); + expect(result?.code).not.toContain('"data-sentry-element": "DestructuredF"'); + expect(result?.code).not.toContain('"data-sentry-element": "Fragment"'); + expect(result?.code).not.toContain('"data-sentry-element": "AssignedF"'); + expect(result?.code).not.toContain('"data-sentry-element": "React.Fragment"'); + expect(result?.code).not.toContain('"data-sentry-element": "MyReact.Fragment"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("handles Fragment aliased correctly when used by other non-Fragment components in a different scope", () => { + const result = transform( + `import { Fragment as OriginalF } from 'react'; +import { OtherComponent } from 'some-library'; + +function TestComponent() { + const F = OriginalF; + + // Use Fragment alias - should be ignored + return ( + +
This should NOT have data-sentry-element (Fragment)
+
+ ); +} + +function AnotherComponent() { + // Different component with same alias name in different function scope + const F = OtherComponent; + + return ( + +
This SHOULD have data-sentry-element (not Fragment)
+
+ ); +} +`, + { + filename: "/variable-assignment-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + } + ); + + expect(result?.code).not.toContain('"data-sentry-element": "F"'); + expect(result?.code).toMatchSnapshot(); + }); });