ref(react): Split up HOC and Profiler

Author: AbhiPrasad

packages/react/src/index.ts

@@ -1,3 +1,5 @@
-import * as Sentry from '@sentry/browser';
+export * from '@sentry/browser';
 
-export { Sentry };
+import { Profiler, withProfiler } from './profiler';
+
+export { Profiler, withProfiler };

packages/react/src/profiler.tsx

@@ -3,19 +3,14 @@ import { Integration, IntegrationClass } from '@sentry/types';
 import { logger } from '@sentry/utils';
 import * as React from 'react';
 
-/** The Props Injected by the HOC */
-interface InjectedProps {
-  /** Called when a transaction is finished */
-  finishProfile(): void;
-}
-
-const DEFAULT_DURATION = 30000;
+export const DEFAULT_DURATION = 30000;
+export const UNKNOWN_COMPONENT = 'unknown';
 
 const TRACING_GETTER = ({
   id: 'Tracing',
 } as any) as IntegrationClass<Integration>;
 
-const getInitActivity = (componentDisplayName: string, timeout = DEFAULT_DURATION): number | null => {
+const getInitActivity = (componentDisplayName: string, timeout: number): number | null => {
   const tracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER);
 
   if (tracingIntegration !== null) {
@@ -37,45 +32,63 @@ const getInitActivity = (componentDisplayName: string, timeout = DEFAULT_DURATIO
   return null;
 };
 
-/**
- * withProfiler() is a HOC that leverages the Sentry AM tracing integration to
- * send transactions about a React component.
- * @param WrappedComponent The component profiled
- * @param timeout A maximum timeout for the component render
- */
-export const withProfiler = <P extends object>(WrappedComponent: React.ComponentType<P>, timeout?: number) => {
-  const componentDisplayName = WrappedComponent.displayName || WrappedComponent.name || 'Component';
+interface ProfilerProps {
+  componentDisplayName?: string;
+  timeout?: number;
+}
 
-  return class extends React.Component<Omit<P, keyof InjectedProps>, { activity: number | null }> {
-    public static displayName: string = `profiler(${componentDisplayName})`;
+interface ProfilerState {
+  activity: number | null;
+}
 
-    public constructor(props: P) {
-      super(props);
+class Profiler extends React.Component<ProfilerProps, ProfilerState> {
+  public constructor(props: ProfilerProps) {
+    super(props);
 
-      this.state = {
-        activity: getInitActivity(componentDisplayName, timeout),
-      };
-    }
+    const { componentDisplayName = UNKNOWN_COMPONENT, timeout = DEFAULT_DURATION } = this.props;
 
-    public componentWillUnmount(): void {
-      this.finishProfile();
-    }
-
-    public finishProfile = () => {
-      if (!this.state.activity) {
-        return;
-      }
-
-      const tracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER);
-      if (tracingIntegration !== null) {
-        // tslint:disable-next-line:no-unsafe-any
-        (tracingIntegration as any).constructor.popActivity(this.state.activity);
-        this.setState({ activity: null });
-      }
+    this.state = {
+      activity: getInitActivity(componentDisplayName, timeout),
     };
+  }
+
+  public componentWillUnmount(): void {
+    this.finishProfile();
+  }
+
+  public finishProfile = () => {
+    if (!this.state.activity) {
+      return;
+    }
 
-    public render(): React.ReactNode {
-      return <WrappedComponent {...this.props as P} />;
+    const tracingIntegration = getCurrentHub().getIntegration(TRACING_GETTER);
+    if (tracingIntegration !== null) {
+      // tslint:disable-next-line:no-unsafe-any
+      (tracingIntegration as any).constructor.popActivity(this.state.activity);
+      this.setState({ activity: null });
     }
   };
-};
+
+  public render(): React.ReactNode {
+    return this.props.children;
+  }
+}
+
+function withProfiler<P extends object>(
+  WrappedComponent: React.ComponentType<P>,
+  profilerProps?: ProfilerProps,
+): React.FC<P> {
+  const componentDisplayName = WrappedComponent.displayName || WrappedComponent.name || UNKNOWN_COMPONENT;
+
+  const Wrapped: React.FC<P> = (props: P) => (
+    <Profiler componentDisplayName={componentDisplayName} {...profilerProps}>
+      <WrappedComponent {...props} />
+    </Profiler>
+  );
+
+  Wrapped.displayName = `profiler(${componentDisplayName})`;
+
+  return Wrapped;
+}
+
+export { withProfiler, Profiler };

packages/react/test/profiler.test.tsx

@@ -1,7 +1,7 @@
 import * as React from 'react';
-import { create, ReactTestInstance } from 'react-test-renderer';
+import { create } from 'react-test-renderer';
 
-import { withProfiler } from '../src/profiler';
+import { DEFAULT_DURATION, UNKNOWN_COMPONENT, withProfiler } from '../src/profiler';
 
 const mockPushActivity = jest.fn().mockReturnValue(1);
 const mockPopActivity = jest.fn();
@@ -38,14 +38,6 @@ describe('withProfiler', () => {
       mockPopActivity.mockClear();
     });
 
-    it('is called with pushActivity() when mounted', () => {
-      const ProfiledComponent = withProfiler(() => <h1>Hello World</h1>);
-
-      expect(mockPushActivity).toHaveBeenCalledTimes(0);
-      create(<ProfiledComponent />);
-      expect(mockPushActivity).toHaveBeenCalledTimes(1);
-    });
-
     it('is called with popActivity() when unmounted', () => {
       const ProfiledComponent = withProfiler(() => <h1>Hello World</h1>);
 
@@ -55,20 +47,46 @@ describe('withProfiler', () => {
       profiler.unmount();
 
       expect(mockPopActivity).toHaveBeenCalledTimes(1);
+      expect(mockPopActivity).toHaveBeenLastCalledWith(1);
     });
 
-    it('calls finishProfile() when unmounting', () => {
-      const ProfiledComponent = withProfiler(() => <h1>Hello World</h1>);
-
-      const mockFinishProfile = jest.fn();
-      const profiler = create(<ProfiledComponent />);
-
-      const instance = profiler.getInstance() as ReactTestInstance & { finishProfile(): void };
-      instance.finishProfile = mockFinishProfile;
-
-      expect(mockFinishProfile).toHaveBeenCalledTimes(0);
-      profiler.unmount();
-      expect(mockFinishProfile).toHaveBeenCalledTimes(1);
+    describe('pushActivity()', () => {
+      it('is called when mounted', () => {
+        const ProfiledComponent = withProfiler(() => <h1>Testing</h1>);
+
+        expect(mockPushActivity).toHaveBeenCalledTimes(0);
+        create(<ProfiledComponent />);
+        expect(mockPushActivity).toHaveBeenCalledTimes(1);
+        expect(mockPushActivity).toHaveBeenLastCalledWith(
+          UNKNOWN_COMPONENT,
+          {
+            data: {},
+            description: `<${UNKNOWN_COMPONENT}>`,
+            op: 'react',
+          },
+          { autoPopAfter: DEFAULT_DURATION },
+        );
+      });
+
+      it('is called with a custom timeout', () => {
+        const ProfiledComponent = withProfiler(() => <h1>Hello World</h1>, { timeout: 32 });
+
+        create(<ProfiledComponent />);
+        expect(mockPushActivity).toHaveBeenLastCalledWith(expect.any(String), expect.any(Object), {
+          autoPopAfter: 32,
+        });
+      });
+
+      it('is called with a custom displayName', () => {
+        const ProfiledComponent = withProfiler(() => <h1>Hello World</h1>, { componentDisplayName: 'Test' });
+
+        create(<ProfiledComponent />);
+        expect(mockPushActivity).toHaveBeenLastCalledWith(
+          'Test',
+          expect.objectContaining({ description: '<Test>' }),
+          expect.any(Object),
+        );
+      });
     });
   });
 });