From 3070c36f5921880685764200f22cafe75ec606c6 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Fri, 18 Jul 2025 16:55:34 +0300 Subject: [PATCH 01/19] fix(react-native): Enhance fragment detection for indirect references --- .../src/index.ts | 158 ++++- .../__snapshots__/test-plugin.test.ts.snap | 111 ++++ .../test/test-plugin.test.ts | 598 +++++++++++++++++- 3 files changed, 837 insertions(+), 30 deletions(-) diff --git a/packages/babel-plugin-component-annotate/src/index.ts b/packages/babel-plugin-component-annotate/src/index.ts index 19f0a519..32d6fbef 100644 --- a/packages/babel-plugin-component-annotate/src/index.ts +++ b/packages/babel-plugin-component-annotate/src/index.ts @@ -61,6 +61,12 @@ type AnnotationPlugin = PluginObj; 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,6 +75,8 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): return; } + const fragmentContext = state['sentryFragmentContext'] as FragmentContext | undefined; + functionBodyPushAttributes( state.opts["annotate-fragments"] === true, t, @@ -76,7 +84,8 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): path.node.id.name, sourceFileNameFromState(state), attributeNamesFromState(state), - state.opts.ignoredComponents ?? [] + state.opts.ignoredComponents ?? [], + fragmentContext ); }, ArrowFunctionExpression(path, state) { @@ -97,6 +106,8 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): return; } + const fragmentContext = state['sentryFragmentContext'] as FragmentContext | undefined; + functionBodyPushAttributes( state.opts["annotate-fragments"] === true, t, @@ -104,7 +115,8 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): parent.id.name, sourceFileNameFromState(state), attributeNamesFromState(state), - state.opts.ignoredComponents ?? [] + state.opts.ignoredComponents ?? [], + fragmentContext ); }, ClassDeclaration(path, state) { @@ -120,6 +132,8 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): const ignoredComponents = state.opts.ignoredComponents ?? []; + const fragmentContext = state['sentryFragmentContext'] as FragmentContext | undefined; + render.traverse({ ReturnStatement(returnStatement) { const arg = returnStatement.get("argument"); @@ -135,7 +149,8 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): name.node && name.node.name, sourceFileNameFromState(state), attributeNamesFromState(state), - ignoredComponents + ignoredComponents, + fragmentContext ); }, }); @@ -151,7 +166,8 @@ function functionBodyPushAttributes( componentName: string, sourceFileName: string | undefined, attributeNames: string[], - ignoredComponents: string[] + ignoredComponents: string[], + fragmentContext?: FragmentContext ): void { let jsxNode: Babel.NodePath; @@ -200,7 +216,8 @@ function functionBodyPushAttributes( componentName, sourceFileName, attributeNames, - ignoredComponents + ignoredComponents, + fragmentContext ); } const alternate = arg.get("alternate"); @@ -212,7 +229,8 @@ function functionBodyPushAttributes( componentName, sourceFileName, attributeNames, - ignoredComponents + ignoredComponents, + fragmentContext ); } return; @@ -236,7 +254,8 @@ function functionBodyPushAttributes( componentName, sourceFileName, attributeNames, - ignoredComponents + ignoredComponents, + fragmentContext ); } @@ -247,7 +266,8 @@ function processJSX( componentName: string | null, sourceFileName: string | undefined, attributeNames: string[], - ignoredComponents: string[] + ignoredComponents: string[], + fragmentContext?: FragmentContext ): void { if (!jsxNode) { return; @@ -264,7 +284,8 @@ function processJSX( componentName, sourceFileName, attributeNames, - ignoredComponents + ignoredComponents, + fragmentContext ); }); @@ -300,7 +321,8 @@ function processJSX( componentName, sourceFileName, attributeNames, - ignoredComponents + ignoredComponents, + fragmentContext ); } else { processJSX( @@ -310,7 +332,8 @@ function processJSX( null, sourceFileName, attributeNames, - ignoredComponents + ignoredComponents, + fragmentContext ); } }); @@ -322,11 +345,12 @@ function applyAttributes( componentName: string | null, sourceFileName: string | undefined, attributeNames: string[], - ignoredComponents: string[] + ignoredComponents: string[], + fragmentContext?: FragmentContext ): void { const [componentAttributeName, elementAttributeName, sourceFileAttributeName] = attributeNames; - if (isReactFragment(t, openingElement)) { + if (isReactFragment(t, openingElement, fragmentContext)) { return; } // e.g., Raw JSX text like the `A` in `

a

` @@ -443,18 +467,106 @@ function attributeNamesFromState(state: AnnotationPluginPass): [string, string, return [webComponentName, webElementName, webSourceFileName]; } -function isReactFragment(t: typeof Babel.types, openingElement: Babel.NodePath): boolean { +interface FragmentContext { + fragmentAliases: Set; + reactNamespaceAliases: Set; +} + +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') { + // import { Fragment } from 'react' -> Fragment + // import { Fragment as F } from 'react' -> F + if (spec.imported.name === 'Fragment') { + fragmentAliases.add(spec.local.name); + } + } else if (spec.type === 'ImportDefaultSpecifier') { + // import React from 'react' -> React + reactNamespaceAliases.add(spec.local.name); + } else if (spec.type === 'ImportNamespaceSpecifier') { + // 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' && + reactNamespaceAliases.has(init.object.name) && + init.property.name === 'Fragment') { + 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)) { + (varPath.node.id as any).properties.forEach((prop: any) => { + if (prop.type === 'ObjectProperty' && + prop.key?.type === 'Identifier' && + 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 +575,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 +595,23 @@ 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..97d17eb3 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,116 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +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 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 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 = () => { 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..3b38fbfc 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,601 @@ 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], + } + ); + + // 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(); + }); + + 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], + } + ); + + // JSX fragments should be ignored (including children) + 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], + } + ); + + // Aliased Fragment should be ignored (including children) + 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], + } + ); + + // Variable-assigned Fragment should be ignored (including children) + 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], + } + ); + + // Namespaced Fragment should be ignored (including children) + 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], + } + ); + + // Default import Fragment should be ignored (including children) + 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'; // ← Import Fragment directly, not as alias + + const MyFragment = Fragment; // ← Now this works + + 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], + } + ); + + // All fragments should be ignored + 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"'); + + // But the outer div should be annotated + expect(result?.code).toContain('"data-sentry-element": "div"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("still annotates regular components that look like fragments", () => { + const result = transform( + `// No React import - so Fragment is just a regular component + export default function TestComponent() { + return ( + +
This Fragment is not from React
+
+ ); + }`, + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + } + ); + + // Non-React Fragment should be annotated since it's not imported from React + expect(result?.code).toContain('"data-sentry-element": "Fragment"'); + expect(result?.code).toContain('"data-sentry-element": "div"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("handles complex variable assignment chains", () => { + const result = transform( + `import { Fragment } from 'react'; + + const MyFragment = Fragment; + const AnotherFragment = MyFragment; // ← This should NOT be detected as fragment + + export default function TestComponent() { + return ( + +
Content in chained fragment
+
+ ); + }`, + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + } + ); + + // First-level variable assignment should be detected and ignored + expect(result?.code).not.toContain('"data-sentry-element": "MyFragment"'); + + // Complex chains are not detected (by design for simplicity), so should be annotated + expect(result?.code).toContain('"data-sentry-element": "AnotherFragment"'); + + // And the inner div should be annotated since AnotherFragment is not detected as fragment + expect(result?.code).toContain('"data-sentry-element": "div"'); + 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, { "annotate-fragments": false }]], + } + ); + + // With annotate-fragments disabled, fragments should still be ignored + 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 }]], + } + ); + + // With annotate-fragments enabled, fragments should still be ignored + // (fragment detection overrides the annotate-fragments option) + 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], + } + ); + + // Destructured Fragment should be ignored (including children) + 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], + } + ); + + // Aliased destructured Fragment should be ignored (including children) + 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], + } + ); + + // Fragment should be ignored even with other destructured items (including children) + 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], + } + ); + + // Fragment from aliased React should be ignored (including children) + 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], + } + ); + + // Fragment from namespace destructuring should be ignored (including children) + 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], + } + ); + + // Both destructured fragments should be ignored + expect(result?.code).not.toContain('"data-sentry-element": "Fragment"'); + expect(result?.code).not.toContain('"data-sentry-element": "AliasedFrag"'); + + // But the outer div should be annotated + expect(result?.code).toContain('"data-sentry-element": "div"'); + expect(result?.code).toMatchSnapshot(); + }); + + it("does not confuse non-React destructuring", () => { + const result = transform( + `import { SomeOtherLib } from 'some-lib'; + +const { Fragment } = SomeOtherLib; + +export default function TestComponent() { + return ( + +
This Fragment is not from React
+
+ ); +}`, + { + filename: "/filename-test.js", + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + } + ); + + // Non-React Fragment should be annotated since it's not from React + expect(result?.code).toContain('"data-sentry-element": "Fragment"'); + expect(result?.code).toContain('"data-sentry-element": "div"'); + 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], + } + ); + + // All fragments should be ignored + 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"'); + + // But the outer div should be annotated + expect(result?.code).toContain('"data-sentry-element": "div"'); + expect(result?.code).toMatchSnapshot(); + }); +}); + +it("debug: fragment context collection", () => { + const result = transform( + `import { Fragment as F } from 'react'; +const MyFragment = Fragment; + +console.log("Debug test"); + +export default function TestComponent() { + return ( +
+ content + content + content +
); }`, { - filename: "/filename-test.js", + filename: "/debug.js", configFile: false, presets: ["@babel/preset-react"], plugins: [plugin], } ); - - // 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(); -}); + + console.log("Generated code:", result?.code); + + // Let's see what's happening + expect(result?.code).toBeDefined(); +}); \ No newline at end of file From b44fcc81ca93ae4fb60d6c172b51a3b7c4b49f58 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Wed, 6 Aug 2025 13:23:27 +0300 Subject: [PATCH 02/19] Use strongly typed sentryFragmentContext --- .../src/index.ts | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/packages/babel-plugin-component-annotate/src/index.ts b/packages/babel-plugin-component-annotate/src/index.ts index 32d6fbef..942cc6f0 100644 --- a/packages/babel-plugin-component-annotate/src/index.ts +++ b/packages/babel-plugin-component-annotate/src/index.ts @@ -51,8 +51,14 @@ interface AnnotationOpts { ignoredComponents?: string[]; } +interface FragmentContext { + fragmentAliases: Set; + reactNamespaceAliases: Set; +} + interface AnnotationPluginPass extends PluginPass { opts: AnnotationOpts; + sentryFragmentContext?: FragmentContext; } type AnnotationPlugin = PluginObj; @@ -64,7 +70,7 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): Program: { enter(path, state) { const fragmentContext = collectFragmentContext(path); - state['sentryFragmentContext'] = fragmentContext; + state.sentryFragmentContext = fragmentContext; } }, FunctionDeclaration(path, state) { @@ -75,8 +81,6 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): return; } - const fragmentContext = state['sentryFragmentContext'] as FragmentContext | undefined; - functionBodyPushAttributes( state.opts["annotate-fragments"] === true, t, @@ -85,7 +89,7 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): sourceFileNameFromState(state), attributeNamesFromState(state), state.opts.ignoredComponents ?? [], - fragmentContext + state.sentryFragmentContext ); }, ArrowFunctionExpression(path, state) { @@ -106,8 +110,6 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): return; } - const fragmentContext = state['sentryFragmentContext'] as FragmentContext | undefined; - functionBodyPushAttributes( state.opts["annotate-fragments"] === true, t, @@ -116,7 +118,7 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): sourceFileNameFromState(state), attributeNamesFromState(state), state.opts.ignoredComponents ?? [], - fragmentContext + state.sentryFragmentContext ); }, ClassDeclaration(path, state) { @@ -132,8 +134,6 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): const ignoredComponents = state.opts.ignoredComponents ?? []; - const fragmentContext = state['sentryFragmentContext'] as FragmentContext | undefined; - render.traverse({ ReturnStatement(returnStatement) { const arg = returnStatement.get("argument"); @@ -150,7 +150,7 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): sourceFileNameFromState(state), attributeNamesFromState(state), ignoredComponents, - fragmentContext + state.sentryFragmentContext ); }, }); @@ -467,11 +467,6 @@ function attributeNamesFromState(state: AnnotationPluginPass): [string, string, return [webComponentName, webElementName, webSourceFileName]; } -interface FragmentContext { - fragmentAliases: Set; - reactNamespaceAliases: Set; -} - function collectFragmentContext(programPath: Babel.NodePath): FragmentContext { const fragmentAliases = new Set(); const reactNamespaceAliases = new Set(['React']); // Default React namespace From 034a8183553305b849b626e161b445d72a19a37b Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Wed, 6 Aug 2025 13:44:16 +0300 Subject: [PATCH 03/19] Fixes lint issues --- .../.eslintrc.js | 3 +++ .../src/index.ts | 18 ++++++++++++------ .../test/test-plugin.test.ts | 4 +--- 3 files changed, 16 insertions(+), 9 deletions(-) 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 942cc6f0..5520321c 100644 --- a/packages/babel-plugin-component-annotate/src/index.ts +++ b/packages/babel-plugin-component-annotate/src/index.ts @@ -520,14 +520,20 @@ function collectFragmentContext(programPath: Babel.NodePath): FragmentContext { // Handle destructuring assignments: const { Fragment } = React if (varPath.node.id.type === 'ObjectPattern') { if (init.type === 'Identifier' && reactNamespaceAliases.has(init.name)) { - (varPath.node.id as any).properties.forEach((prop: any) => { - if (prop.type === 'ObjectProperty' && - prop.key?.type === 'Identifier' && - prop.value?.type === 'Identifier' && - prop.key.name === 'Fragment') { + 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); } - }); + } } } } 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 3b38fbfc..325baf3c 100644 --- a/packages/babel-plugin-component-annotate/test/test-plugin.test.ts +++ b/packages/babel-plugin-component-annotate/test/test-plugin.test.ts @@ -1848,9 +1848,7 @@ export default function TestComponent() { plugins: [plugin], } ); - - console.log("Generated code:", result?.code); - + // Let's see what's happening expect(result?.code).toBeDefined(); }); \ No newline at end of file From d208ae1ba67938c14bdd05a9fa654a20df9a1581 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Wed, 6 Aug 2025 16:48:08 +0300 Subject: [PATCH 04/19] Fix tests --- .../src/index.ts | 93 +++++++------ .../__snapshots__/test-plugin.test.ts.snap | 57 ++++++++ .../test/test-plugin.test.ts | 127 ++---------------- 3 files changed, 114 insertions(+), 163 deletions(-) diff --git a/packages/babel-plugin-component-annotate/src/index.ts b/packages/babel-plugin-component-annotate/src/index.ts index 5520321c..7679291e 100644 --- a/packages/babel-plugin-component-annotate/src/index.ts +++ b/packages/babel-plugin-component-annotate/src/index.ts @@ -71,7 +71,7 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel): enter(path, state) { const fragmentContext = collectFragmentContext(path); state.sentryFragmentContext = fragmentContext; - } + }, }, FunctionDeclaration(path, state) { if (!path.node.id || !path.node.id.name) { @@ -350,14 +350,17 @@ function applyAttributes( ): void { const [componentAttributeName, elementAttributeName, sourceFileAttributeName] = attributeNames; - if (isReactFragment(t, openingElement, fragmentContext)) { - 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); @@ -367,15 +370,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)) @@ -390,7 +389,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)) @@ -398,14 +396,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)) @@ -469,25 +469,25 @@ function attributeNamesFromState(state: AnnotationPluginPass): [string, string, function collectFragmentContext(programPath: Babel.NodePath): FragmentContext { const fragmentAliases = new Set(); - const reactNamespaceAliases = new Set(['React']); // Default React namespace - + 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') { + if (source === "react" || source === "React") { + importPath.node.specifiers.forEach((spec) => { + if (spec.type === "ImportSpecifier" && spec.imported.type === "Identifier") { // import { Fragment } from 'react' -> Fragment // import { Fragment as F } from 'react' -> F - if (spec.imported.name === 'Fragment') { + if (spec.imported.name === "Fragment") { fragmentAliases.add(spec.local.name); } - } else if (spec.type === 'ImportDefaultSpecifier') { + } else if (spec.type === "ImportDefaultSpecifier") { // import React from 'react' -> React reactNamespaceAliases.add(spec.local.name); - } else if (spec.type === 'ImportNamespaceSpecifier') { + } else if (spec.type === "ImportNamespaceSpecifier") { // import * as React from 'react' -> React reactNamespaceAliases.add(spec.local.name); } @@ -499,37 +499,39 @@ function collectFragmentContext(programPath: Babel.NodePath): FragmentContext { VariableDeclarator(varPath) { if (varPath.node.init) { const init = varPath.node.init; - + // Handle identifier assignments: const MyFragment = Fragment - if (varPath.node.id.type === 'Identifier') { + 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)) { + 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' && - reactNamespaceAliases.has(init.object.name) && - init.property.name === 'Fragment') { + if ( + init.type === "MemberExpression" && + init.object.type === "Identifier" && + init.property.type === "Identifier" && + reactNamespaceAliases.has(init.object.name) && + init.property.name === "Fragment" + ) { 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)) { + 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.type === "ObjectProperty" && prop.key && - prop.key.type === 'Identifier' && + prop.key.type === "Identifier" && prop.value && - prop.value.type === 'Identifier' && - prop.key.name === 'Fragment' + prop.value.type === "Identifier" && + prop.key.name === "Fragment" ) { fragmentAliases.add(prop.value.name); } @@ -537,14 +539,14 @@ function collectFragmentContext(programPath: Babel.NodePath): FragmentContext { } } } - } + }, }); return { fragmentAliases, reactNamespaceAliases }; } function isReactFragment( - t: typeof Babel.types, + t: typeof Babel.types, openingElement: Babel.NodePath, context?: FragmentContext // Add this optional parameter ): boolean { @@ -561,7 +563,7 @@ function isReactFragment( } // 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; @@ -604,7 +606,10 @@ function isReactFragment( // Enhanced checks using context if (context) { // Check React.Fragment pattern with known React namespaces - if (context.reactNamespaceAliases.has(objectName as string) && propertyName === "Fragment") { + if ( + context.reactNamespaceAliases.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 97d17eb3..a9597144 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,34 @@ // 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 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 { @@ -20,6 +49,23 @@ export default function TestComponent() { }" `; +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; @@ -94,6 +140,17 @@ export default function TestComponent() { }" `; +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() { 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 325baf3c..b18ab282 100644 --- a/packages/babel-plugin-component-annotate/test/test-plugin.test.ts +++ b/packages/babel-plugin-component-annotate/test/test-plugin.test.ts @@ -1276,7 +1276,6 @@ describe("Fragment Detection", () => { } ); - // 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(); @@ -1300,7 +1299,6 @@ describe("Fragment Detection", () => { } ); - // JSX fragments should be ignored (including children) expect(result?.code).toContain("React.createElement(React.Fragment"); expect(result?.code).not.toContain('"data-sentry-element": "Fragment"'); expect(result?.code).toMatchSnapshot(); @@ -1325,8 +1323,7 @@ export default function TestComponent() { } ); - // Aliased Fragment should be ignored (including children) - expect(result?.code).toContain('React.createElement(F'); + expect(result?.code).toContain("React.createElement(F"); expect(result?.code).not.toContain('"data-sentry-element": "F"'); expect(result?.code).toMatchSnapshot(); }); @@ -1352,8 +1349,7 @@ export default function TestComponent() { } ); - // Variable-assigned Fragment should be ignored (including children) - expect(result?.code).toContain('React.createElement(MyFragment'); + expect(result?.code).toContain("React.createElement(MyFragment"); expect(result?.code).not.toContain('"data-sentry-element": "MyFragment"'); expect(result?.code).toMatchSnapshot(); }); @@ -1377,8 +1373,7 @@ export default function TestComponent() { } ); - // Namespaced Fragment should be ignored (including children) - expect(result?.code).toContain('React.createElement(MyReact.Fragment'); + expect(result?.code).toContain("React.createElement(MyReact.Fragment"); expect(result?.code).not.toContain('"data-sentry-element": "MyReact.Fragment"'); expect(result?.code).toMatchSnapshot(); }); @@ -1402,17 +1397,16 @@ export default function TestComponent() { } ); - // Default import Fragment should be ignored (including children) - expect(result?.code).toContain('React.createElement(MyReact.Fragment'); + 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'; // ← Import Fragment directly, not as alias + `import React, { Fragment } from 'react'; - const MyFragment = Fragment; // ← Now this works + const MyFragment = Fragment; export default function TestComponent() { return ( @@ -1443,37 +1437,9 @@ export default function TestComponent() { } ); - // All fragments should be ignored 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"'); - - // But the outer div should be annotated - expect(result?.code).toContain('"data-sentry-element": "div"'); - expect(result?.code).toMatchSnapshot(); - }); - - it("still annotates regular components that look like fragments", () => { - const result = transform( - `// No React import - so Fragment is just a regular component - export default function TestComponent() { - return ( - -
This Fragment is not from React
-
- ); - }`, - { - filename: "/filename-test.js", - configFile: false, - presets: ["@babel/preset-react"], - plugins: [plugin], - } - ); - - // Non-React Fragment should be annotated since it's not imported from React - expect(result?.code).toContain('"data-sentry-element": "Fragment"'); - expect(result?.code).toContain('"data-sentry-element": "div"'); expect(result?.code).toMatchSnapshot(); }); @@ -1482,7 +1448,7 @@ export default function TestComponent() { `import { Fragment } from 'react'; const MyFragment = Fragment; - const AnotherFragment = MyFragment; // ← This should NOT be detected as fragment + const AnotherFragment = MyFragment; export default function TestComponent() { return ( @@ -1499,14 +1465,8 @@ export default function TestComponent() { } ); - // First-level variable assignment should be detected and ignored expect(result?.code).not.toContain('"data-sentry-element": "MyFragment"'); - - // Complex chains are not detected (by design for simplicity), so should be annotated - expect(result?.code).toContain('"data-sentry-element": "AnotherFragment"'); - - // And the inner div should be annotated since AnotherFragment is not detected as fragment - expect(result?.code).toContain('"data-sentry-element": "div"'); + expect(result?.code).not.toContain('"data-sentry-element": "AnotherFragment"'); expect(result?.code).toMatchSnapshot(); }); @@ -1529,7 +1489,6 @@ export default function TestComponent() { } ); - // With annotate-fragments disabled, fragments should still be ignored expect(result?.code).not.toContain('"data-sentry-element": "F"'); expect(result?.code).toMatchSnapshot(); }); @@ -1553,8 +1512,6 @@ export default function TestComponent() { } ); - // With annotate-fragments enabled, fragments should still be ignored - // (fragment detection overrides the annotate-fragments option) expect(result?.code).not.toContain('"data-sentry-element": "F"'); expect(result?.code).toMatchSnapshot(); }); @@ -1580,7 +1537,6 @@ export default function TestComponent() { } ); - // Destructured Fragment should be ignored (including children) expect(result?.code).not.toContain('"data-sentry-element": "Fragment"'); expect(result?.code).toMatchSnapshot(); }); @@ -1606,7 +1562,6 @@ export default function TestComponent() { } ); - // Aliased destructured Fragment should be ignored (including children) expect(result?.code).not.toContain('"data-sentry-element": "MyFragment"'); expect(result?.code).toMatchSnapshot(); }); @@ -1632,7 +1587,6 @@ export default function TestComponent() { } ); - // Fragment should be ignored even with other destructured items (including children) expect(result?.code).not.toContain('"data-sentry-element": "Fragment"'); expect(result?.code).toMatchSnapshot(); }); @@ -1658,7 +1612,6 @@ export default function TestComponent() { } ); - // Fragment from aliased React should be ignored (including children) expect(result?.code).not.toContain('"data-sentry-element": "Fragment"'); expect(result?.code).toMatchSnapshot(); }); @@ -1684,7 +1637,6 @@ export default function TestComponent() { } ); - // Fragment from namespace destructuring should be ignored (including children) expect(result?.code).not.toContain('"data-sentry-element": "F"'); expect(result?.code).toMatchSnapshot(); }); @@ -1718,39 +1670,8 @@ export default function TestComponent() { } ); - // Both destructured fragments should be ignored expect(result?.code).not.toContain('"data-sentry-element": "Fragment"'); expect(result?.code).not.toContain('"data-sentry-element": "AliasedFrag"'); - - // But the outer div should be annotated - expect(result?.code).toContain('"data-sentry-element": "div"'); - expect(result?.code).toMatchSnapshot(); - }); - - it("does not confuse non-React destructuring", () => { - const result = transform( - `import { SomeOtherLib } from 'some-lib'; - -const { Fragment } = SomeOtherLib; - -export default function TestComponent() { - return ( - -
This Fragment is not from React
-
- ); -}`, - { - filename: "/filename-test.js", - configFile: false, - presets: ["@babel/preset-react"], - plugins: [plugin], - } - ); - - // Non-React Fragment should be annotated since it's not from React - expect(result?.code).toContain('"data-sentry-element": "Fragment"'); - expect(result?.code).toContain('"data-sentry-element": "div"'); expect(result?.code).toMatchSnapshot(); }); @@ -1811,44 +1732,12 @@ export default function TestComponent() { } ); - // All fragments should be ignored 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"'); - - // But the outer div should be annotated - expect(result?.code).toContain('"data-sentry-element": "div"'); expect(result?.code).toMatchSnapshot(); }); }); - -it("debug: fragment context collection", () => { - const result = transform( - `import { Fragment as F } from 'react'; -const MyFragment = Fragment; - -console.log("Debug test"); - -export default function TestComponent() { - return ( -
- content - content - content -
- ); -}`, - { - filename: "/debug.js", - configFile: false, - presets: ["@babel/preset-react"], - plugins: [plugin], - } - ); - - // Let's see what's happening - expect(result?.code).toBeDefined(); -}); \ No newline at end of file From d728ba5e5e25bf6417eeff006568594542d436c0 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Wed, 6 Aug 2025 17:13:01 +0300 Subject: [PATCH 05/19] Refactor the long parameter lists to use a context object --- .../src/index.ts | 186 ++++++++---------- 1 file changed, 77 insertions(+), 109 deletions(-) diff --git a/packages/babel-plugin-component-annotate/src/index.ts b/packages/babel-plugin-component-annotate/src/index.ts index 7679291e..e3eccd6a 100644 --- a/packages/babel-plugin-component-annotate/src/index.ts +++ b/packages/babel-plugin-component-annotate/src/index.ts @@ -63,6 +63,24 @@ interface AnnotationPluginPass extends PluginPass { 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 { @@ -81,16 +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 ?? [], - state.sentryFragmentContext - ); + const context = createJSXProcessingContext(state, t, path.node.id.name); + functionBodyPushAttributes(context, path); }, ArrowFunctionExpression(path, state) { // We're expecting a `VariableDeclarator` like `const MyComponent =` @@ -110,16 +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 ?? [], - state.sentryFragmentContext - ); + const context = createJSXProcessingContext(state, t, parent.id.name); + functionBodyPushAttributes(context, path); }, ClassDeclaration(path, state) { const name = path.get("id"); @@ -132,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) { @@ -142,16 +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, - state.sentryFragmentContext - ); + processJSX(context, arg); }, }); }, @@ -159,15 +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[], - fragmentContext?: FragmentContext + 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; @@ -209,29 +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, - fragmentContext - ); + processJSX(context, consequent); } const alternate = arg.get("alternate"); if (alternate.isJSXFragment() || alternate.isJSXElement()) { - processJSX( - annotateFragments, - t, - alternate, - componentName, - sourceFileName, - attributeNames, - ignoredComponents, - fragmentContext - ); + processJSX(context, alternate); } return; } @@ -247,31 +240,26 @@ function functionBodyPushAttributes( return; } - processJSX( - annotateFragments, - t, - jsxNode, - componentName, - sourceFileName, - attributeNames, - ignoredComponents, - fragmentContext - ); + 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[], - fragmentContext?: FragmentContext + componentName?: string | null ): void { if (!jsxNode) { return; } + + // Use provided componentName or fall back to context componentName + const currentComponentName = componentName !== undefined ? 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"); @@ -279,13 +267,9 @@ function processJSX( openingElements.forEach((openingElement) => { applyAttributes( - t, + context, openingElement as Babel.NodePath, - componentName, - sourceFileName, - attributeNames, - ignoredComponents, - fragmentContext + currentComponentName ); }); @@ -296,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 @@ -314,40 +298,24 @@ function processJSX( if (shouldSetComponentName && openingElement && openingElement.node) { shouldSetComponentName = false; - processJSX( - annotateFragments, - t, - child, - componentName, - sourceFileName, - attributeNames, - ignoredComponents, - fragmentContext - ); + processJSX(context, child, currentComponentName); } else { - processJSX( - annotateFragments, - t, - child, - null, - sourceFileName, - attributeNames, - ignoredComponents, - fragmentContext - ); + processJSX(context, child, null); } }); } +/** + * 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[], - fragmentContext?: FragmentContext + componentName: string | null ): void { + const { t, attributeNames, ignoredComponents, fragmentContext, sourceFileName } = context; const [componentAttributeName, elementAttributeName, sourceFileAttributeName] = attributeNames; // e.g., Raw JSX text like the `A` in `

a

` From 00b983402c639ad650bfc47713b231c16c6c62f0 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Wed, 6 Aug 2025 18:35:38 +0300 Subject: [PATCH 06/19] Update snapshot --- .../test/__snapshots__/test-plugin.test.ts.snap | 7 ------- 1 file changed, 7 deletions(-) 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 a9597144..1b64bb4a 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 @@ -415,13 +415,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'; From 4063095b0a200c6ce81bc3fef47ecc8dce4279d2 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 7 Aug 2025 10:52:53 +0300 Subject: [PATCH 07/19] Adds performance benchmarks --- .../test/test-plugin.test.ts | 144 ++++++++++++++++++ 1 file changed, 144 insertions(+) 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 b18ab282..ca7f18a7 100644 --- a/packages/babel-plugin-component-annotate/test/test-plugin.test.ts +++ b/packages/babel-plugin-component-annotate/test/test-plugin.test.ts @@ -26,6 +26,13 @@ import { transform } from "@babel/core"; import plugin from "../src/index"; +const getTime = (): number => { + if (typeof performance !== "undefined" && performance.now) { + return performance.now(); + } + return Date.now(); +}; + const BananasPizzaAppStandardInput = `import React, { Component } from 'react'; import { StyleSheet, Text, TextInput, View, Image, UIManager } from 'react-native'; @@ -1741,3 +1748,140 @@ export default function TestComponent() { expect(result?.code).toMatchSnapshot(); }); }); + +describe("Fragment Performance Benchmarks", () => { + interface TestFile { + filename: string; + code: string; + } + + interface BenchmarkResult { + avg: number; + min: number; + max: number; + median: number; + } + + const createTestProject = (fileCount: number, fragmentRatio = 0.4): TestFile[] => { + const files: TestFile[] = []; + + for (let i = 0; i < fileCount; i++) { + const hasFragment = Math.random() < fragmentRatio; + const patterns = [ + `import React, { Fragment } from 'react';`, + `import { Fragment } from 'react';`, + `import * as React from 'react';`, + `import React from 'react';\nconst { Fragment } = React;`, + `import MyReact from 'react';\nconst { Fragment: F } = MyReact;`, + ]; + + const pattern = patterns[Math.floor(Math.random() * patterns.length)] as string; + const fragmentType = getFragmentType(pattern); + + // Add imports to other files for realistic import traversal + const imports: string[] = []; + const numImports = Math.min(3, i); + for (let j = 0; j < numImports; j++) { + imports.push(`import Component${j} from './component${j}';`); + } + + const code = `${pattern} +${imports.join("\n")} + +export default function Component${i}() { + return ${ + hasFragment + ? `<${fragmentType}>
Content ${i}
` + : `
Component ${i}
` + }; +}`; + + files.push({ filename: `component${i}.js`, code }); + } + + return files; + }; + + const getFragmentType = (pattern: string): string => { + if (pattern.includes("Fragment: F")) return "F"; + if (pattern.includes("Fragment }")) return "Fragment"; + if (pattern.includes("* as React")) return "React.Fragment"; + if (pattern.includes("const { Fragment }")) return "Fragment"; + return "Fragment"; + }; + + const runBenchmark = (files: TestFile[], iterations = 10): BenchmarkResult => { + const times: number[] = []; + + for (let i = 0; i < iterations; i++) { + const start = getTime(); + + // Transform all files (simulating your plugin processing) + files.forEach((file: TestFile) => { + const result = transform(file.code, { + filename: `/${file.filename}`, + configFile: false, + presets: ["@babel/preset-react"], + plugins: [plugin], + }); + if (!result?.code) throw new Error("Transform failed"); + }); + + const end = getTime(); + times.push(end - start); + } + + const sortedTimes = [...times].sort((a, b) => a - b); + return { + avg: times.reduce((a, b) => a + b) / times.length, + min: Math.min(...times), + max: Math.max(...times), + median: sortedTimes[Math.floor(sortedTimes.length / 2)], + }; + }; + + // Test different project sizes + const scenarios = [ + { name: "small" as const, files: 20, expectedMaxTime: 50 }, + { name: "medium" as const, files: 50, expectedMaxTime: 200 }, + { name: "large" as const, files: 200, expectedMaxTime: 500 }, + ]; + + scenarios.forEach((scenario) => { + it(`should perform well on ${scenario.name} projects (${scenario.files} files)`, () => { + const testFiles = createTestProject(scenario.files); + const results = runBenchmark(testFiles); + + expect(results.avg).toBeGreaterThan(0); + expect(results.avg).toBeLessThan(scenario.expectedMaxTime); + expect(results.median).toBeLessThan(scenario.expectedMaxTime * 0.8); + }); + }); + + it("should handle fragment-heavy projects efficiently", () => { + const fragmentHeavyFiles = createTestProject(25, 0.8); + const normalFiles = createTestProject(25, 0.2); + + const fragmentResults = runBenchmark(fragmentHeavyFiles, 5); + const normalResults = runBenchmark(normalFiles, 5); + + const overhead = (fragmentResults.avg - normalResults.avg) / normalResults.avg; + + expect(overhead).toBeLessThan(0.5); // Less than 50% overhead + expect(fragmentResults.avg).toBeLessThan(300); + }); + + it("should not consume excessive memory", () => { + const testFiles = createTestProject(50); + + if (global.gc) global.gc(); + + const memBefore = process.memoryUsage().heapUsed; + runBenchmark(testFiles, 1); + const memAfter = process.memoryUsage().heapUsed; + + const memoryDelta = (memAfter - memBefore) / 1024 / 1024; + + expect(memoryDelta).toBeLessThan(50); + }); +}); From 5389cb44dee805bf2dbdf763cd899ae02c0ec792 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 7 Aug 2025 14:11:16 +0300 Subject: [PATCH 08/19] Update comment for clarity Co-authored-by: LucasZF --- packages/babel-plugin-component-annotate/src/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/babel-plugin-component-annotate/src/index.ts b/packages/babel-plugin-component-annotate/src/index.ts index e3eccd6a..4c48566e 100644 --- a/packages/babel-plugin-component-annotate/src/index.ts +++ b/packages/babel-plugin-component-annotate/src/index.ts @@ -447,8 +447,8 @@ function collectFragmentContext(programPath: Babel.NodePath): FragmentContext { if (source === "react" || source === "React") { importPath.node.specifiers.forEach((spec) => { if (spec.type === "ImportSpecifier" && spec.imported.type === "Identifier") { - // import { Fragment } from 'react' -> Fragment - // import { Fragment as F } from 'react' -> F + // 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); } From d150d9345dfff1bf23d45c65e2a3bd6abfa6f93c Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 7 Aug 2025 14:12:09 +0300 Subject: [PATCH 09/19] Simplify conditional logic Co-authored-by: LucasZF --- packages/babel-plugin-component-annotate/src/index.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/babel-plugin-component-annotate/src/index.ts b/packages/babel-plugin-component-annotate/src/index.ts index 4c48566e..07aae185 100644 --- a/packages/babel-plugin-component-annotate/src/index.ts +++ b/packages/babel-plugin-component-annotate/src/index.ts @@ -452,12 +452,9 @@ function collectFragmentContext(programPath: Babel.NodePath): FragmentContext { if (spec.imported.name === "Fragment") { fragmentAliases.add(spec.local.name); } - } else if (spec.type === "ImportDefaultSpecifier") { - // import React from 'react' -> React - reactNamespaceAliases.add(spec.local.name); - } else if (spec.type === "ImportNamespaceSpecifier") { - // import * as React from 'react' -> React - reactNamespaceAliases.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); } }); } From 016ec8728f26ec5c900110683590bd3ff77c4417 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 7 Aug 2025 14:14:26 +0300 Subject: [PATCH 10/19] Fix formatting --- packages/babel-plugin-component-annotate/src/index.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/babel-plugin-component-annotate/src/index.ts b/packages/babel-plugin-component-annotate/src/index.ts index 07aae185..9708b439 100644 --- a/packages/babel-plugin-component-annotate/src/index.ts +++ b/packages/babel-plugin-component-annotate/src/index.ts @@ -452,9 +452,13 @@ function collectFragmentContext(programPath: Babel.NodePath): FragmentContext { if (spec.imported.name === "Fragment") { fragmentAliases.add(spec.local.name); } - } else if (spec.type === "ImportDefaultSpecifier" || spec.type === "ImportNamespaceSpecifier" ) { + } 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); + // import * as React from 'react' -> React + reactNamespaceAliases.add(spec.local.name); } }); } From 0fa60c5b0b641aae777a2fd8ffff8c3e409ab226 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 7 Aug 2025 14:16:12 +0300 Subject: [PATCH 11/19] Adjust benchmark baseline after CI run --- .../test/test-plugin.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 ca7f18a7..98ba5c80 100644 --- a/packages/babel-plugin-component-annotate/test/test-plugin.test.ts +++ b/packages/babel-plugin-component-annotate/test/test-plugin.test.ts @@ -1842,9 +1842,9 @@ export default function Component${i}() { // Test different project sizes const scenarios = [ - { name: "small" as const, files: 20, expectedMaxTime: 50 }, - { name: "medium" as const, files: 50, expectedMaxTime: 200 }, - { name: "large" as const, files: 200, expectedMaxTime: 500 }, + { name: "small" as const, files: 20, expectedMaxTime: 55 }, + { name: "medium" as const, files: 50, expectedMaxTime: 250 }, + { name: "large" as const, files: 200, expectedMaxTime: 550 }, ]; scenarios.forEach((scenario) => { From 039e4149fd413bd13e84d93c2723384f6ef87370 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 7 Aug 2025 14:22:05 +0300 Subject: [PATCH 12/19] fix typecheck issue --- .../babel-plugin-component-annotate/test/test-plugin.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 98ba5c80..6fcfe428 100644 --- a/packages/babel-plugin-component-annotate/test/test-plugin.test.ts +++ b/packages/babel-plugin-component-annotate/test/test-plugin.test.ts @@ -1836,7 +1836,7 @@ export default function Component${i}() { avg: times.reduce((a, b) => a + b) / times.length, min: Math.min(...times), max: Math.max(...times), - median: sortedTimes[Math.floor(sortedTimes.length / 2)], + median: sortedTimes[Math.floor(sortedTimes.length / 2)] ?? -1, }; }; From 5961ffea8a68dc91afebb03f96586b2b371476b6 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 7 Aug 2025 14:22:22 +0300 Subject: [PATCH 13/19] Make benchmark less flaky --- .../test/test-plugin.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 6fcfe428..1974362b 100644 --- a/packages/babel-plugin-component-annotate/test/test-plugin.test.ts +++ b/packages/babel-plugin-component-annotate/test/test-plugin.test.ts @@ -1842,9 +1842,9 @@ export default function Component${i}() { // Test different project sizes const scenarios = [ - { name: "small" as const, files: 20, expectedMaxTime: 55 }, - { name: "medium" as const, files: 50, expectedMaxTime: 250 }, - { name: "large" as const, files: 200, expectedMaxTime: 550 }, + { name: "small" as const, files: 20, expectedMaxTime: 60 }, + { name: "medium" as const, files: 50, expectedMaxTime: 300 }, + { name: "large" as const, files: 200, expectedMaxTime: 600 }, ]; scenarios.forEach((scenario) => { From 314f7bcef5a195530bb6ed7308faeb05d45a09dc Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 7 Aug 2025 14:31:46 +0300 Subject: [PATCH 14/19] Check fragment sooner --- packages/babel-plugin-component-annotate/src/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/babel-plugin-component-annotate/src/index.ts b/packages/babel-plugin-component-annotate/src/index.ts index 9708b439..2ca9a380 100644 --- a/packages/babel-plugin-component-annotate/src/index.ts +++ b/packages/babel-plugin-component-annotate/src/index.ts @@ -481,8 +481,8 @@ function collectFragmentContext(programPath: Babel.NodePath): FragmentContext { init.type === "MemberExpression" && init.object.type === "Identifier" && init.property.type === "Identifier" && - reactNamespaceAliases.has(init.object.name) && - init.property.name === "Fragment" + init.property.name === "Fragment" && + reactNamespaceAliases.has(init.object.name) ) { fragmentAliases.add(varPath.node.id.name); } From ea68863b67b2bd6863728da7b4aa9f017a083297 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 7 Aug 2025 16:15:14 +0300 Subject: [PATCH 15/19] Simplify componentName null/undefined handling --- packages/babel-plugin-component-annotate/src/index.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/babel-plugin-component-annotate/src/index.ts b/packages/babel-plugin-component-annotate/src/index.ts index 2ca9a380..54163dfd 100644 --- a/packages/babel-plugin-component-annotate/src/index.ts +++ b/packages/babel-plugin-component-annotate/src/index.ts @@ -251,14 +251,14 @@ function functionBodyPushAttributes( function processJSX( context: JSXProcessingContext, jsxNode: Babel.NodePath, - componentName?: string | null + componentName?: string ): void { if (!jsxNode) { return; } // Use provided componentName or fall back to context componentName - const currentComponentName = componentName !== undefined ? componentName : 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 @@ -300,7 +300,7 @@ function processJSX( shouldSetComponentName = false; processJSX(context, child, currentComponentName); } else { - processJSX(context, child, null); + processJSX(context, child, ""); } }); } @@ -313,7 +313,7 @@ function processJSX( function applyAttributes( context: JSXProcessingContext, openingElement: Babel.NodePath, - componentName: string | null + componentName: string ): void { const { t, attributeNames, ignoredComponents, fragmentContext, sourceFileName } = context; const [componentAttributeName, elementAttributeName, sourceFileAttributeName] = attributeNames; From 4dcfbf51a9ba726ad4beb066f403545d427b3704 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 7 Aug 2025 18:11:04 +0300 Subject: [PATCH 16/19] Add same alias test --- .../__snapshots__/test-plugin.test.ts.snap | 16 ++++++++ .../test/test-plugin.test.ts | 39 +++++++++++++++++++ 2 files changed, 55 insertions(+) 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 1b64bb4a..174fe5d4 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 @@ -168,6 +168,22 @@ export default function TestComponent() { }" `; +exports[`Fragment Performance Benchmarks 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[`arrow snapshot matches 1`] = ` "import React, { Component } from 'react'; const componentName = () => { 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 1974362b..3afe6ab1 100644 --- a/packages/babel-plugin-component-annotate/test/test-plugin.test.ts +++ b/packages/babel-plugin-component-annotate/test/test-plugin.test.ts @@ -1884,4 +1884,43 @@ export default function Component${i}() { expect(memoryDelta).toBeLessThan(50); }); + + 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(); + }); }); From 29e996f9372b507c24980226248f1d608b7ea397 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 7 Aug 2025 18:13:06 +0300 Subject: [PATCH 17/19] Removes benchmarks from unit tests --- .../__snapshots__/test-plugin.test.ts.snap | 32 ++-- .../test/test-plugin.test.ts | 137 ------------------ 2 files changed, 16 insertions(+), 153 deletions(-) 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 174fe5d4..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 @@ -20,6 +20,22 @@ export default function TestComponent() { }" `; +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; @@ -168,22 +184,6 @@ export default function TestComponent() { }" `; -exports[`Fragment Performance Benchmarks 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[`arrow snapshot matches 1`] = ` "import React, { Component } from 'react'; const componentName = () => { 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 3afe6ab1..b91b1c69 100644 --- a/packages/babel-plugin-component-annotate/test/test-plugin.test.ts +++ b/packages/babel-plugin-component-annotate/test/test-plugin.test.ts @@ -1747,143 +1747,6 @@ export default function TestComponent() { expect(result?.code).not.toContain('"data-sentry-element": "MyReact.Fragment"'); expect(result?.code).toMatchSnapshot(); }); -}); - -describe("Fragment Performance Benchmarks", () => { - interface TestFile { - filename: string; - code: string; - } - - interface BenchmarkResult { - avg: number; - min: number; - max: number; - median: number; - } - - const createTestProject = (fileCount: number, fragmentRatio = 0.4): TestFile[] => { - const files: TestFile[] = []; - - for (let i = 0; i < fileCount; i++) { - const hasFragment = Math.random() < fragmentRatio; - const patterns = [ - `import React, { Fragment } from 'react';`, - `import { Fragment } from 'react';`, - `import * as React from 'react';`, - `import React from 'react';\nconst { Fragment } = React;`, - `import MyReact from 'react';\nconst { Fragment: F } = MyReact;`, - ]; - - const pattern = patterns[Math.floor(Math.random() * patterns.length)] as string; - const fragmentType = getFragmentType(pattern); - - // Add imports to other files for realistic import traversal - const imports: string[] = []; - const numImports = Math.min(3, i); - for (let j = 0; j < numImports; j++) { - imports.push(`import Component${j} from './component${j}';`); - } - - const code = `${pattern} -${imports.join("\n")} - -export default function Component${i}() { - return ${ - hasFragment - ? `<${fragmentType}>
Content ${i}
` - : `
Component ${i}
` - }; -}`; - - files.push({ filename: `component${i}.js`, code }); - } - - return files; - }; - - const getFragmentType = (pattern: string): string => { - if (pattern.includes("Fragment: F")) return "F"; - if (pattern.includes("Fragment }")) return "Fragment"; - if (pattern.includes("* as React")) return "React.Fragment"; - if (pattern.includes("const { Fragment }")) return "Fragment"; - return "Fragment"; - }; - - const runBenchmark = (files: TestFile[], iterations = 10): BenchmarkResult => { - const times: number[] = []; - - for (let i = 0; i < iterations; i++) { - const start = getTime(); - - // Transform all files (simulating your plugin processing) - files.forEach((file: TestFile) => { - const result = transform(file.code, { - filename: `/${file.filename}`, - configFile: false, - presets: ["@babel/preset-react"], - plugins: [plugin], - }); - if (!result?.code) throw new Error("Transform failed"); - }); - - const end = getTime(); - times.push(end - start); - } - - const sortedTimes = [...times].sort((a, b) => a - b); - return { - avg: times.reduce((a, b) => a + b) / times.length, - min: Math.min(...times), - max: Math.max(...times), - median: sortedTimes[Math.floor(sortedTimes.length / 2)] ?? -1, - }; - }; - - // Test different project sizes - const scenarios = [ - { name: "small" as const, files: 20, expectedMaxTime: 60 }, - { name: "medium" as const, files: 50, expectedMaxTime: 300 }, - { name: "large" as const, files: 200, expectedMaxTime: 600 }, - ]; - - scenarios.forEach((scenario) => { - it(`should perform well on ${scenario.name} projects (${scenario.files} files)`, () => { - const testFiles = createTestProject(scenario.files); - const results = runBenchmark(testFiles); - - expect(results.avg).toBeGreaterThan(0); - expect(results.avg).toBeLessThan(scenario.expectedMaxTime); - expect(results.median).toBeLessThan(scenario.expectedMaxTime * 0.8); - }); - }); - - it("should handle fragment-heavy projects efficiently", () => { - const fragmentHeavyFiles = createTestProject(25, 0.8); - const normalFiles = createTestProject(25, 0.2); - - const fragmentResults = runBenchmark(fragmentHeavyFiles, 5); - const normalResults = runBenchmark(normalFiles, 5); - - const overhead = (fragmentResults.avg - normalResults.avg) / normalResults.avg; - - expect(overhead).toBeLessThan(0.5); // Less than 50% overhead - expect(fragmentResults.avg).toBeLessThan(300); - }); - - it("should not consume excessive memory", () => { - const testFiles = createTestProject(50); - - if (global.gc) global.gc(); - - const memBefore = process.memoryUsage().heapUsed; - runBenchmark(testFiles, 1); - const memAfter = process.memoryUsage().heapUsed; - - const memoryDelta = (memAfter - memBefore) / 1024 / 1024; - - expect(memoryDelta).toBeLessThan(50); - }); it("handles Fragment aliased correctly when used by other non-Fragment components in a different scope", () => { const result = transform( From 5e866c3cee7a043feeaf4d734db4dfda06a007c8 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 7 Aug 2025 18:25:23 +0300 Subject: [PATCH 18/19] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e44bc30..a52b12fb 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.0.2 From f83899eed460fa2d85e58601c7c89df0628a447e Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 7 Aug 2025 18:32:19 +0300 Subject: [PATCH 19/19] Remove unused function after benchmarks removal --- .../test/test-plugin.test.ts | 7 ------- 1 file changed, 7 deletions(-) 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 b91b1c69..a453da77 100644 --- a/packages/babel-plugin-component-annotate/test/test-plugin.test.ts +++ b/packages/babel-plugin-component-annotate/test/test-plugin.test.ts @@ -26,13 +26,6 @@ import { transform } from "@babel/core"; import plugin from "../src/index"; -const getTime = (): number => { - if (typeof performance !== "undefined" && performance.now) { - return performance.now(); - } - return Date.now(); -}; - const BananasPizzaAppStandardInput = `import React, { Component } from 'react'; import { StyleSheet, Text, TextInput, View, Image, UIManager } from 'react-native';