Skip to content

Commit d208ae1

Browse files
antonisantonis
committed
Fix tests
1 parent 1ee4667 commit d208ae1

File tree

3 files changed

+114
-163
lines changed

3 files changed

+114
-163
lines changed

‎packages/babel-plugin-component-annotate/src/index.ts

Lines changed: 49 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ export default function componentNameAnnotatePlugin({ types: t }: typeof Babel):
7171
enter(path, state) {
7272
const fragmentContext = collectFragmentContext(path);
7373
state.sentryFragmentContext = fragmentContext;
74-
}
74+
},
7575
},
7676
FunctionDeclaration(path, state) {
7777
if (!path.node.id || !path.node.id.name) {
@@ -350,14 +350,17 @@ function applyAttributes(
350350
): void {
351351
const [componentAttributeName, elementAttributeName, sourceFileAttributeName] = attributeNames;
352352

353-
if (isReactFragment(t, openingElement, fragmentContext)) {
354-
return;
355-
}
356353
// e.g., Raw JSX text like the `A` in `<h1>a</h1>`
357354
if (!openingElement.node) {
358355
return;
359356
}
360357

358+
// Check if this is a React fragment - if so, skip attribute addition entirely
359+
const isFragment = isReactFragment(t, openingElement, fragmentContext);
360+
if (isFragment) {
361+
return;
362+
}
363+
361364
if (!openingElement.node.attributes) openingElement.node.attributes = [];
362365
const elementName = getPathName(t, openingElement);
363366

@@ -367,15 +370,11 @@ function applyAttributes(
367370

368371
// Add a stable attribute for the element name but only for non-DOM names
369372
let isAnIgnoredElement = false;
370-
if (
371-
!isAnIgnoredComponent &&
372-
!hasAttributeWithName(openingElement, componentAttributeName) &&
373-
(componentAttributeName !== elementAttributeName || !componentName)
374-
) {
373+
if (!isAnIgnoredComponent && !hasAttributeWithName(openingElement, elementAttributeName)) {
375374
if (DEFAULT_IGNORED_ELEMENTS.includes(elementName)) {
376375
isAnIgnoredElement = true;
377376
} else {
378-
// TODO: Is it possible to avoid this null check?
377+
// Always add element attribute for non-ignored elements
379378
if (elementAttributeName) {
380379
openingElement.node.attributes.push(
381380
t.jSXAttribute(t.jSXIdentifier(elementAttributeName), t.stringLiteral(elementName))
@@ -390,22 +389,23 @@ function applyAttributes(
390389
!isAnIgnoredComponent &&
391390
!hasAttributeWithName(openingElement, componentAttributeName)
392391
) {
393-
// TODO: Is it possible to avoid this null check?
394392
if (componentAttributeName) {
395393
openingElement.node.attributes.push(
396394
t.jSXAttribute(t.jSXIdentifier(componentAttributeName), t.stringLiteral(componentName))
397395
);
398396
}
399397
}
400398

401-
// Add a stable attribute for the source file name (absent for non-root elements)
399+
// Add a stable attribute for the source file name
400+
// Updated condition: add source file for elements that have either:
401+
// 1. A component name (root elements), OR
402+
// 2. An element name that's not ignored (child elements)
402403
if (
403404
sourceFileName &&
404405
!isAnIgnoredComponent &&
405-
(componentName || isAnIgnoredElement === false) &&
406+
(componentName || !isAnIgnoredElement) &&
406407
!hasAttributeWithName(openingElement, sourceFileAttributeName)
407408
) {
408-
// TODO: Is it possible to avoid this null check?
409409
if (sourceFileAttributeName) {
410410
openingElement.node.attributes.push(
411411
t.jSXAttribute(t.jSXIdentifier(sourceFileAttributeName), t.stringLiteral(sourceFileName))
@@ -469,25 +469,25 @@ function attributeNamesFromState(state: AnnotationPluginPass): [string, string,
469469

470470
function collectFragmentContext(programPath: Babel.NodePath): FragmentContext {
471471
const fragmentAliases = new Set<string>();
472-
const reactNamespaceAliases = new Set<string>(['React']); // Default React namespace
473-
472+
const reactNamespaceAliases = new Set<string>(["React"]); // Default React namespace
473+
474474
programPath.traverse({
475475
ImportDeclaration(importPath) {
476476
const source = importPath.node.source.value;
477-
477+
478478
// Handle React imports
479-
if (source === 'react' || source === 'React') {
480-
importPath.node.specifiers.forEach(spec => {
481-
if (spec.type === 'ImportSpecifier' && spec.imported.type === 'Identifier') {
479+
if (source === "react" || source === "React") {
480+
importPath.node.specifiers.forEach((spec) => {
481+
if (spec.type === "ImportSpecifier" && spec.imported.type === "Identifier") {
482482
// import { Fragment } from 'react' -> Fragment
483483
// import { Fragment as F } from 'react' -> F
484-
if (spec.imported.name === 'Fragment') {
484+
if (spec.imported.name === "Fragment") {
485485
fragmentAliases.add(spec.local.name);
486486
}
487-
} else if (spec.type === 'ImportDefaultSpecifier') {
487+
} else if (spec.type === "ImportDefaultSpecifier") {
488488
// import React from 'react' -> React
489489
reactNamespaceAliases.add(spec.local.name);
490-
} else if (spec.type === 'ImportNamespaceSpecifier') {
490+
} else if (spec.type === "ImportNamespaceSpecifier") {
491491
// import * as React from 'react' -> React
492492
reactNamespaceAliases.add(spec.local.name);
493493
}
@@ -499,52 +499,54 @@ function collectFragmentContext(programPath: Babel.NodePath): FragmentContext {
499499
VariableDeclarator(varPath) {
500500
if (varPath.node.init) {
501501
const init = varPath.node.init;
502-
502+
503503
// Handle identifier assignments: const MyFragment = Fragment
504-
if (varPath.node.id.type === 'Identifier') {
504+
if (varPath.node.id.type === "Identifier") {
505505
// Handle: const MyFragment = Fragment (only if Fragment is a known alias)
506-
if (init.type === 'Identifier' && fragmentAliases.has(init.name)) {
506+
if (init.type === "Identifier" && fragmentAliases.has(init.name)) {
507507
fragmentAliases.add(varPath.node.id.name);
508508
}
509-
509+
510510
// Handle: const MyFragment = React.Fragment (only for known React namespaces)
511-
if (init.type === 'MemberExpression' &&
512-
init.object.type === 'Identifier' &&
513-
init.property.type === 'Identifier' &&
514-
reactNamespaceAliases.has(init.object.name) &&
515-
init.property.name === 'Fragment') {
511+
if (
512+
init.type === "MemberExpression" &&
513+
init.object.type === "Identifier" &&
514+
init.property.type === "Identifier" &&
515+
reactNamespaceAliases.has(init.object.name) &&
516+
init.property.name === "Fragment"
517+
) {
516518
fragmentAliases.add(varPath.node.id.name);
517519
}
518520
}
519-
521+
520522
// Handle destructuring assignments: const { Fragment } = React
521-
if (varPath.node.id.type === 'ObjectPattern') {
522-
if (init.type === 'Identifier' && reactNamespaceAliases.has(init.name)) {
523+
if (varPath.node.id.type === "ObjectPattern") {
524+
if (init.type === "Identifier" && reactNamespaceAliases.has(init.name)) {
523525
const properties = varPath.node.id.properties;
524-
526+
525527
for (const prop of properties) {
526528
if (
527-
prop.type === 'ObjectProperty' &&
529+
prop.type === "ObjectProperty" &&
528530
prop.key &&
529-
prop.key.type === 'Identifier' &&
531+
prop.key.type === "Identifier" &&
530532
prop.value &&
531-
prop.value.type === 'Identifier' &&
532-
prop.key.name === 'Fragment'
533+
prop.value.type === "Identifier" &&
534+
prop.key.name === "Fragment"
533535
) {
534536
fragmentAliases.add(prop.value.name);
535537
}
536538
}
537539
}
538540
}
539541
}
540-
}
542+
},
541543
});
542544

543545
return { fragmentAliases, reactNamespaceAliases };
544546
}
545547

546548
function isReactFragment(
547-
t: typeof Babel.types,
549+
t: typeof Babel.types,
548550
openingElement: Babel.NodePath,
549551
context?: FragmentContext // Add this optional parameter
550552
): boolean {
@@ -561,7 +563,7 @@ function isReactFragment(
561563
}
562564

563565
// TODO: All these objects are typed as unknown, maybe an oversight in Babel types?
564-
566+
565567
// Check if the element name is a known fragment alias
566568
if (context && elementName && context.fragmentAliases.has(elementName)) {
567569
return true;
@@ -604,7 +606,10 @@ function isReactFragment(
604606
// Enhanced checks using context
605607
if (context) {
606608
// Check React.Fragment pattern with known React namespaces
607-
if (context.reactNamespaceAliases.has(objectName as string) && propertyName === "Fragment") {
609+
if (
610+
context.reactNamespaceAliases.has(objectName as string) &&
611+
propertyName === "Fragment"
612+
) {
608613
return true;
609614
}
610615

‎packages/babel-plugin-component-annotate/test/__snapshots__/test-plugin.test.ts.snap

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,34 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3+
exports[`Fragment Detection combines all fragment patterns correctly 1`] = `
4+
"import React, { Fragment as ImportedF } from 'react';
5+
import * as MyReact from 'react';
6+
const {
7+
Fragment: DestructuredF
8+
} = React;
9+
const {
10+
Fragment
11+
} = MyReact;
12+
const AssignedF = Fragment; // ← This uses the destructured Fragment from MyReact
13+
14+
export default function TestComponent() {
15+
return /*#__PURE__*/React.createElement(\\"div\\", {
16+
className: \\"container\\",
17+
\\"data-sentry-component\\": \\"TestComponent\\",
18+
\\"data-sentry-source-file\\": \\"filename-test.js\\"
19+
}, /*#__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\\")));
20+
}"
21+
`;
22+
23+
exports[`Fragment Detection handles complex variable assignment chains 1`] = `
24+
"import { Fragment } from 'react';
25+
const MyFragment = Fragment;
26+
const AnotherFragment = MyFragment;
27+
export default function TestComponent() {
28+
return /*#__PURE__*/React.createElement(AnotherFragment, null, /*#__PURE__*/React.createElement(\\"div\\", null, \\"Content in chained fragment\\"));
29+
}"
30+
`;
31+
332
exports[`Fragment Detection handles destructuring from aliased React imports 1`] = `
433
"import MyReact from 'react';
534
const {
@@ -20,6 +49,23 @@ export default function TestComponent() {
2049
}"
2150
`;
2251

52+
exports[`Fragment Detection handles multiple destructuring patterns in one file 1`] = `
53+
"import React from 'react';
54+
import * as MyReact from 'react';
55+
const {
56+
Fragment
57+
} = React;
58+
const {
59+
Fragment: AliasedFrag
60+
} = MyReact;
61+
export default function TestComponent() {
62+
return /*#__PURE__*/React.createElement(\\"div\\", {
63+
\\"data-sentry-component\\": \\"TestComponent\\",
64+
\\"data-sentry-source-file\\": \\"filename-test.js\\"
65+
}, /*#__PURE__*/React.createElement(Fragment, null, /*#__PURE__*/React.createElement(\\"span\\", null, \\"Regular destructured\\")), /*#__PURE__*/React.createElement(AliasedFrag, null, /*#__PURE__*/React.createElement(\\"p\\", null, \\"Aliased destructured\\")));
66+
}"
67+
`;
68+
2369
exports[`Fragment Detection ignores Fragment assigned to variable 1`] = `
2470
"import { Fragment } from 'react';
2571
const MyFragment = Fragment;
@@ -94,6 +140,17 @@ export default function TestComponent() {
94140
}"
95141
`;
96142

143+
exports[`Fragment Detection ignores multiple fragment patterns in same file 1`] = `
144+
"import React, { Fragment } from 'react';
145+
const MyFragment = Fragment;
146+
export default function TestComponent() {
147+
return /*#__PURE__*/React.createElement(\\"div\\", {
148+
\\"data-sentry-component\\": \\"TestComponent\\",
149+
\\"data-sentry-source-file\\": \\"filename-test.js\\"
150+
}, /*#__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\\")));
151+
}"
152+
`;
153+
97154
exports[`Fragment Detection works with annotate-fragments option disabled 1`] = `
98155
"import { Fragment as F } from 'react';
99156
export default function TestComponent() {

0 commit comments

Comments
 (0)