Browse Source

ref(ui): Convert hookOrDefault to functional component (#56582)

Scott Cooper 1 year ago
parent
commit
1e63e40b8f
2 changed files with 90 additions and 39 deletions
  1. 47 0
      static/app/components/hookOrDefault.spec.tsx
  2. 43 39
      static/app/components/hookOrDefault.tsx

+ 47 - 0
static/app/components/hookOrDefault.spec.tsx

@@ -0,0 +1,47 @@
+import {render, screen} from 'sentry-test/reactTestingLibrary';
+
+import HookStore from 'sentry/stores/hookStore';
+
+import HookOrDefault from './hookOrDefault';
+
+describe('HookOrDefault', () => {
+  beforeEach(() => {
+    HookStore.init();
+  });
+
+  it('should render default', () => {
+    const Component = HookOrDefault({
+      hookName: 'component:replay-onboarding-cta',
+      defaultComponent: () => (
+        <div data-test-id="default-component">Default Component</div>
+      ),
+    });
+
+    render(<Component organization={TestStubs.Organization()}>Test</Component>);
+    expect(screen.getByTestId('default-component')).toBeInTheDocument();
+    expect(screen.getByText('Default Component')).toBeInTheDocument();
+  });
+
+  it('should render from HookStore', () => {
+    HookStore.add(
+      'component:replay-onboarding-cta',
+      () =>
+        function ({organization}) {
+          return <div data-test-id="hook-component">{organization.slug}</div>;
+        }
+    );
+
+    const Component = HookOrDefault({
+      hookName: 'component:replay-onboarding-cta',
+      defaultComponent: () => (
+        <div data-test-id="default-component">Default Component</div>
+      ),
+    });
+
+    render(<Component organization={TestStubs.Organization()}>Test</Component>);
+
+    expect(screen.getByTestId('hook-component')).toBeInTheDocument();
+    expect(screen.queryByTestId('default-component')).not.toBeInTheDocument();
+    expect(screen.getByText('org-slug')).toBeInTheDocument();
+  });
+});

+ 43 - 39
static/app/components/hookOrDefault.tsx

@@ -1,9 +1,9 @@
-import {Component, lazy, Suspense} from 'react';
+import {ComponentProps, lazy, Suspense, useEffect, useState} from 'react';
 
 import HookStore from 'sentry/stores/hookStore';
 import {HookName, Hooks} from 'sentry/types/hooks';
 
-type Params<H extends HookName> = {
+interface Params<H extends HookName> {
   /**
    * The name of the hook as listed in hookstore.add(hookName, callback)
    */
@@ -18,7 +18,7 @@ type Params<H extends HookName> = {
    * use React.Suspense and React.lazy to render the component.
    */
   defaultComponentPromise?: () => Promise<ReturnType<Hooks[H]>>;
-};
+}
 
 /**
  * Use this instead of the usual ternery operator when using getsentry hooks.
@@ -46,54 +46,58 @@ function HookOrDefault<H extends HookName>({
   hookName,
   defaultComponent,
   defaultComponentPromise,
-}: Params<H>) {
-  type Props = React.ComponentProps<ReturnType<Hooks[H]>>;
-  type State = {hooks: Hooks[H][]};
-
-  class HookOrDefaultComponent extends Component<Props, State> {
-    static displayName = `HookOrDefaultComponent(${hookName})`;
+}: Params<H>): React.FunctionComponent<ComponentProps<ReturnType<Hooks[H]>>> {
+  type Props = ComponentProps<ReturnType<Hooks[H]>>;
 
-    state: State = {
-      hooks: HookStore.get(hookName),
-    };
+  // Defining the props here is unnecessary and slow for typescript
+  function getDefaultComponent(): React.ComponentType<any> | undefined {
+    // If `defaultComponentPromise` is passed, then return a Suspended component
+    if (defaultComponentPromise) {
+      // Lazy adds a complicated type that is not important
+      const DefaultComponent: React.ComponentType<any> = lazy(defaultComponentPromise);
 
-    componentWillUnmount() {
-      this.unlistener?.();
+      return function (props: Props) {
+        return (
+          <Suspense fallback={null}>
+            <DefaultComponent {...props} />
+          </Suspense>
+        );
+      };
     }
 
-    unlistener = HookStore.listen(
-      (name: string, hooks: Hooks[HookName][]) =>
-        name === hookName && this.setState({hooks}),
-      undefined
-    );
+    return defaultComponent;
+  }
 
-    get defaultComponent() {
-      // If `defaultComponentPromise` is passed, then return a Suspended component
-      if (defaultComponentPromise) {
-        const DefaultComponent = lazy(defaultComponentPromise);
+  function HookOrDefaultComponent(props: Props) {
+    const [hooks, setHooks] = useState<Hooks[H][]>(HookStore.get(hookName));
 
-        return function (props: Props) {
-          return (
-            <Suspense fallback={null}>
-              <DefaultComponent {...props} />
-            </Suspense>
-          );
-        };
-      }
+    useEffect(() => {
+      const unsubscribe = HookStore.listen((name: string, newHooks: Hooks[H][]) => {
+        if (name === hookName) {
+          setHooks(newHooks);
+        }
+      }, undefined);
 
-      return defaultComponent;
-    }
+      return () => {
+        unsubscribe();
+      };
+    }, []);
 
-    render() {
-      const hookExists = this.state.hooks && this.state.hooks.length;
-      const componentFromHook = this.state.hooks[0]?.();
-      const HookComponent =
-        hookExists && componentFromHook ? componentFromHook : this.defaultComponent;
+    const hookExists = hooks && hooks.length;
+    const componentFromHook = hooks[0]?.();
+    // Defining the props here is unnecessary and slow for typescript
+    const HookComponent: React.ComponentType<any> =
+      hookExists && componentFromHook ? componentFromHook : getDefaultComponent();
 
-      return HookComponent ? <HookComponent {...this.props} /> : null;
+    if (!HookComponent) {
+      return null;
     }
+
+    return <HookComponent {...props} />;
   }
 
+  HookOrDefaultComponent.displayName = `HookOrDefaultComponent(${hookName})`;
+
   return HookOrDefaultComponent;
 }