Skip to content

Commit 5876b56

Browse files
committed
fix: instrumentation react not to exit Class visitor early if encountering a non-class-method path
1 parent 154e6ef commit 5876b56

File tree

4 files changed

+48
-18
lines changed

4 files changed

+48
-18
lines changed

packages/instrumentation-react/src/index.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ export default function ottrelitePlugin(): PluginObj {
129129

130130
if (LOG_DEBUG) {
131131
console.log(
132-
`[tracePlugin] Found directive "${traceDirective.value.value}" in ${locationDescription}, instrumenting component '${componentName}' lifecycle tracing.`
132+
`[tracePlugin] Found directive "${traceDirective.value.value}" in ${locationDescription}, instrumenting function component '${componentName}' lifecycle tracing.`
133133
);
134134
}
135135

@@ -240,14 +240,14 @@ export default function ottrelitePlugin(): PluginObj {
240240
Class(classPath, pluginPass) {
241241
for (const path of classPath.node.body.body) {
242242
if (!t.isClassMethod(path)) {
243-
return;
243+
continue;
244244
}
245245

246246
const traceDirective = path.body.directives.find(({ value }) =>
247247
value.value.startsWith('use trace')
248248
);
249249

250-
if (!traceDirective) return;
250+
if (!traceDirective) continue;
251251

252252
// remove that directive
253253
path.body.directives = path.body.directives.filter(
@@ -266,6 +266,12 @@ export default function ottrelitePlugin(): PluginObj {
266266
locationDescription
267267
);
268268

269+
if (LOG_DEBUG) {
270+
console.log(
271+
`[tracePlugin] Found directive "${traceDirective.value.value}" in ${locationDescription}, instrumenting class component '${componentName}' lifecycle tracing.`
272+
);
273+
}
274+
269275
(program as AugmentedProgramNode).__hasClassComponent = true;
270276

271277
// wrap the class component inside the HOC

packages/instrumentation-react/src/utils/getLocationDescription.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,15 @@ import { NodePath, PluginPass } from '@babel/core';
22
import * as t from '@babel/types';
33

44
export function getLocationDescription(
5-
path: NodePath | t.ClassMethod,
5+
path: NodePath<t.Function> | t.ClassMethod | t.ClassPrivateMethod,
66
pluginPass: PluginPass
77
) {
8-
const loc =
9-
path.type === 'ClassMethod' ? (path as t.ClassMethod).loc : path.node.loc;
8+
let loc: t.SourceLocation | undefined | null;
9+
if ('node' in path) {
10+
loc = path.node.loc;
11+
} else {
12+
loc = path.loc;
13+
}
14+
1015
return `${pluginPass.filename}:${loc?.start.line}:${loc?.start.column} through ${loc?.end.line}:${loc?.end.column}`;
1116
}

packages/instrumentation-react/src/utils/parseDirective.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,7 @@ export function parseDirective(
4545
let isFunctionComponent = true;
4646

4747
// II. Attempt to read displayName or name, if componentName is still undefined
48-
if (
49-
!componentName &&
50-
(path.node.type === 'FunctionDeclaration' ||
51-
path.node.type === 'FunctionExpression' ||
52-
path.node.type === 'ArrowFunctionExpression')
53-
) {
48+
if (!componentName && t.isFunction(path.node)) {
5449
// look for .displayName or .name assignments in parent scope
5550
const parent = path.findParent(
5651
(p) => p.isProgram() || p.isBlockStatement()
@@ -72,6 +67,14 @@ export function parseDirective(
7267
}
7368
}
7469

70+
const isClassMember =
71+
(path.parent.type === 'ClassMethod' &&
72+
path.parent.key.type === 'Identifier') ||
73+
(path.parent.type === 'ObjectProperty' && t.isFunction(path.node));
74+
if (isClassMember) {
75+
isFunctionComponent = false;
76+
}
77+
7578
// III. Infer the name from context, if componentName is still undefined
7679
if (!componentName) {
7780
if ('id' in path.node && path.node.id?.name) {
@@ -89,13 +92,10 @@ export function parseDirective(
8992
) {
9093
// object property assignment
9194
componentName ??= path.parent.key.name;
92-
} else if (
93-
path.parent.type === 'ClassMethod' &&
94-
path.parent.key.type === 'Identifier'
95-
) {
95+
} else if (isClassMember) {
9696
// class method
97-
isFunctionComponent = false;
98-
componentName ??= path.parent.key.name;
97+
componentName ??= ((path.parent as t.ClassMethod).key as t.Identifier)
98+
.name;
9999
} else if (path.parent.type === 'ExportDefaultDeclaration') {
100100
// anonymous export default
101101
throw new Error(

packages/instrumentation-react/tests/index.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,25 @@ describe('Ottrelite Babel Plugin', () => {
395395
expect(result).toContain(TRACE_COMPONENT_LIFECYCLE_HOC_NAME);
396396
});
397397

398+
test(`should add ${TRACE_COMPONENT_LIFECYCLE_HOOK_NAME} import when use trace directive is found in a class component that has other paths than the render method`, () => {
399+
const code = `
400+
class MyComponent {
401+
someMethod() {
402+
console.log('doing something');
403+
}
404+
405+
render() {
406+
'use trace';
407+
return <div>Hello</div>;
408+
}
409+
}
410+
`;
411+
412+
const result = transformCode(code);
413+
expect(result).toContain(CORE_PACKAGE_NAME);
414+
expect(result).toContain(TRACE_COMPONENT_LIFECYCLE_HOC_NAME);
415+
});
416+
398417
test(`should not add duplicate import if ${TRACE_COMPONENT_LIFECYCLE_HOOK_NAME} is already imported in a class component`, () => {
399418
const code = `
400419
import { ${TRACE_COMPONENT_LIFECYCLE_HOOK_NAME} } from '${CORE_PACKAGE_NAME}';

0 commit comments

Comments
 (0)