chore(react): Update comments and test for Profiler

AbhiPrasad · AbhiPrasad · commit 598c5bb603cb · 2020-06-10T08:10:34.000-04:00
diff --git a/packages/react/package.json b/packages/react/package.json
@@ -32,6 +32,7 @@
     "@types/hoist-non-react-statics": "^3.3.1",
     "@types/react": "^16.9.35",
     "jest": "^24.7.1",
+    "jsdom": "^16.2.2",
     "npm-run-all": "^4.1.2",
     "prettier": "^1.17.0",
     "prettier-check": "^2.0.0",
diff --git a/packages/react/src/profiler.tsx b/packages/react/src/profiler.tsx
@@ -39,6 +39,10 @@ function afterNextFrame(callback: Function): void {
   timeout = window.setTimeout(done, 100);
 }
 
+/**
+ * getInitActivity pushes activity based on React component mount
+ * @param name displayName of component that started activity
+ */
 const getInitActivity = (name: string): number | null => {
   const tracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER);
 
@@ -68,10 +72,13 @@ class Profiler extends React.Component<ProfilerProps> {
     this.activity = getInitActivity(this.props.name);
   }
 
+  // If a component mounted, we can finish the mount activity.
   public componentDidMount(): void {
     afterNextFrame(this.finishProfile);
   }
 
+  // Sometimes a component will unmount first, so we make
+  // sure to also finish the mount activity here.
   public componentWillUnmount(): void {
     afterNextFrame(this.finishProfile);
   }
@@ -94,8 +101,15 @@ class Profiler extends React.Component<ProfilerProps> {
   }
 }
 
-function withProfiler<P extends object>(WrappedComponent: React.ComponentType<P>): React.FC<P> {
-  const componentDisplayName = WrappedComponent.displayName || WrappedComponent.name || UNKNOWN_COMPONENT;
+/**
+ * withProfiler is a higher order component that wraps a
+ * component in a {@link Profiler} component.
+ *
+ * @param WrappedComponent component that is wrapped by Profiler
+ * @param name displayName of component being profiled
+ */
+function withProfiler<P extends object>(WrappedComponent: React.ComponentType<P>, name?: string): React.FC<P> {
+  const componentDisplayName = name || WrappedComponent.displayName || WrappedComponent.name || UNKNOWN_COMPONENT;
 
   const Wrapped: React.FC<P> = (props: P) => (
     <Profiler name={componentDisplayName}>
diff --git a/packages/react/test/profiler.test.tsx b/packages/react/test/profiler.test.tsx
@@ -6,6 +6,17 @@ import { UNKNOWN_COMPONENT, useProfiler, withProfiler } from '../src/profiler';
 
 const mockPushActivity = jest.fn().mockReturnValue(1);
 const mockPopActivity = jest.fn();
+const mockLoggerWarn = jest.fn();
+
+let integrationIsNull = false;
+
+jest.mock('@sentry/utils', () => ({
+  logger: {
+    warn: (message: string) => {
+      mockLoggerWarn(message);
+    },
+  },
+}));
 
 jest.mock('@sentry/browser', () => ({
   getCurrentHub: () => ({
@@ -20,7 +31,11 @@ jest.mock('@sentry/browser', () => ({
         public static popActivity: () => void = mockPopActivity;
       }
 
-      return new MockIntegration('test');
+      if (!integrationIsNull) {
+        return new MockIntegration('test');
+      }
+
+      return null;
     },
   }),
 }));
@@ -30,6 +45,8 @@ describe('withProfiler', () => {
     jest.useFakeTimers();
     mockPushActivity.mockClear();
     mockPopActivity.mockClear();
+    mockLoggerWarn.mockClear();
+    integrationIsNull = false;
   });
 
   it('sets displayName properly', () => {
@@ -39,6 +56,18 @@ describe('withProfiler', () => {
     expect(ProfiledComponent.displayName).toBe('profiler(TestComponent)');
   });
 
+  it('sets a custom displayName', () => {
+    const TestComponent = () => <h1>Hello World</h1>;
+
+    const ProfiledComponent = withProfiler(TestComponent, 'BestComponent');
+    expect(ProfiledComponent.displayName).toBe('profiler(BestComponent)');
+  });
+
+  it('defaults to an unknown displayName', () => {
+    const ProfiledComponent = withProfiler(() => <h1>Hello World</h1>);
+    expect(ProfiledComponent.displayName).toBe(`profiler(${UNKNOWN_COMPONENT})`);
+  });
+
   it('popActivity() is called when unmounted', () => {
     const ProfiledComponent = withProfiler(() => <h1>Hello World</h1>);
 
@@ -63,13 +92,32 @@ describe('withProfiler', () => {
       op: 'react',
     });
   });
+
+  it('does not start an activity when integration is disabled', () => {
+    integrationIsNull = true;
+    const ProfiledComponent = withProfiler(() => <h1>Hello World</h1>);
+
+    expect(mockPushActivity).toHaveBeenCalledTimes(0);
+    expect(mockLoggerWarn).toHaveBeenCalledTimes(0);
+
+    const profiler = render(<ProfiledComponent />);
+    expect(mockPopActivity).toHaveBeenCalledTimes(0);
+    expect(mockPushActivity).toHaveBeenCalledTimes(0);
+
+    expect(mockLoggerWarn).toHaveBeenCalledTimes(1);
+
+    profiler.unmount();
+    expect(mockPopActivity).toHaveBeenCalledTimes(0);
+  });
 });
 
 describe('useProfiler()', () => {
   beforeEach(() => {
     jest.useFakeTimers();
     mockPushActivity.mockClear();
     mockPopActivity.mockClear();
+    mockLoggerWarn.mockClear();
+    integrationIsNull = false;
   });
 
   it('popActivity() is called when unmounted', () => {
@@ -95,4 +143,20 @@ describe('useProfiler()', () => {
       op: 'react',
     });
   });
+
+  it('does not start an activity when integration is disabled', () => {
+    integrationIsNull = true;
+    expect(mockPushActivity).toHaveBeenCalledTimes(0);
+    expect(mockLoggerWarn).toHaveBeenCalledTimes(0);
+
+    // tslint:disable-next-line: no-void-expression
+    const profiler = renderHook(() => useProfiler('Example'));
+    expect(mockPopActivity).toHaveBeenCalledTimes(0);
+    expect(mockPushActivity).toHaveBeenCalledTimes(0);
+
+    expect(mockLoggerWarn).toHaveBeenCalledTimes(1);
+
+    profiler.unmount();
+    expect(mockPopActivity).toHaveBeenCalledTimes(0);
+  });
 });
diff --git a/yarn.lock b/yarn.lock
