Browse Source

feat(js): Add useApi hook and implement in withApi (#28257)

Evan Purkhiser 3 years ago
parent
commit
0750603acd

+ 48 - 0
static/app/utils/useApi.tsx

@@ -0,0 +1,48 @@
+import {useEffect, useRef} from 'react';
+
+import {Client} from 'app/api';
+
+type Options = {
+  /**
+   * Enabling this option will disable clearing in-flight requests when the
+   * component is unmounted.
+   *
+   * This may be useful in situations where your component needs to finish up
+   * somewhere the client was passed into some type of action creator and the
+   * component is unmounted.
+   */
+  persistInFlight?: boolean;
+  /**
+   * An existing API client may be provided.
+   *
+   * This is a continent way to re-use clients and still inherit the
+   * persistInFlight configuration.
+   */
+  api?: Client;
+};
+
+/**
+ * Returns an API client that will have it's requests canceled when the owning
+ * React component is unmounted (may be disabled via options).
+ */
+function useApi({persistInFlight, api: providedApi}: Options = {}) {
+  const localApi = useRef<Client>();
+
+  // Lazily construct the client if we weren't provided with one
+  if (localApi.current === undefined && providedApi === undefined) {
+    localApi.current = new Client();
+  }
+
+  // Use the provided client if available
+  const api = providedApi ?? localApi.current!;
+
+  function handleCleanup() {
+    !persistInFlight && api.clear();
+  }
+
+  useEffect(() => handleCleanup, []);
+
+  return api;
+}
+
+export default useApi;

+ 13 - 34
static/app/utils/withApi.tsx

@@ -1,7 +1,6 @@
-import * as React from 'react';
-
 import {Client} from 'app/api';
 import getDisplayName from 'app/utils/getDisplayName';
+import useApi from 'app/utils/useApi';
 
 type InjectedApiProps = {
   api: Client;
@@ -9,46 +8,26 @@ type InjectedApiProps = {
 
 type WrappedProps<P> = Omit<P, keyof InjectedApiProps> & Partial<InjectedApiProps>;
 
-type OptionProps = {
-  /**
-   * Enabling this option will disable clearing in-flight requests when the
-   * component is unmounted.
-   *
-   * This may be useful in situations where your component needs to finish up
-   * some where the client was passed into some type of action creator and the
-   * component is unmounted.
-   */
-  persistInFlight?: boolean;
-};
-
 /**
  * React Higher-Order Component (HoC) that provides "api" client when mounted,
  * and clears API requests when component is unmounted.
+ *
+ * If an `api` prop is provided when the component is invoked it will be passed
+ * through.
  */
 const withApi = <P extends InjectedApiProps>(
   WrappedComponent: React.ComponentType<P>,
-  {persistInFlight}: OptionProps = {}
-) =>
-  class extends React.Component<WrappedProps<P>> {
-    static displayName = `withApi(${getDisplayName(WrappedComponent)})`;
-
-    constructor(props: WrappedProps<P>) {
-      super(props);
-      this.api = new Client();
-    }
+  options: Parameters<typeof useApi>[0] = {}
+) => {
+  const WithApi: React.FC<WrappedProps<P>> = ({api: propsApi, ...props}) => {
+    const api = useApi({api: propsApi, ...options});
 
-    componentWillUnmount() {
-      if (!persistInFlight) {
-        this.api.clear();
-      }
-    }
+    return <WrappedComponent {...(props as P)} api={api} />;
+  };
 
-    private api: Client;
+  WithApi.displayName = `withApi(${getDisplayName(WrappedComponent)})`;
 
-    render() {
-      const {api, ...props} = this.props;
-      return <WrappedComponent {...({api: api ?? this.api, ...props} as P)} />;
-    }
-  };
+  return WithApi;
+};
 
 export default withApi;

+ 58 - 0
tests/js/spec/utils/useApi.spec.tsx

@@ -0,0 +1,58 @@
+import {mountWithTheme} from 'sentry-test/reactTestingLibrary';
+
+import {Client} from 'app/api';
+import useApi from 'app/utils/useApi';
+
+describe('useApi', function () {
+  let apiInstance: Client;
+
+  type Props = {
+    /**
+     * Test passthrough API clients
+     */
+    api?: Client;
+    /**
+     * Test persistInFlight
+     */
+    persistInFlight?: boolean;
+  };
+
+  const MyComponent: React.FC<Props> = ({api, persistInFlight}) => {
+    apiInstance = useApi({api, persistInFlight});
+    return <div />;
+  };
+
+  it('renders MyComponent with an api prop', function () {
+    mountWithTheme(<MyComponent />);
+
+    expect(apiInstance).toBeInstanceOf(Client);
+  });
+
+  it('cancels pending API requests when component is unmounted', function () {
+    const {unmount} = mountWithTheme(<MyComponent />);
+
+    jest.spyOn(apiInstance, 'clear');
+    unmount();
+
+    expect(apiInstance.clear).toHaveBeenCalled();
+  });
+
+  it('does not cancel inflights when persistInFlight is true', function () {
+    const {unmount} = mountWithTheme(<MyComponent persistInFlight />);
+
+    jest.spyOn(apiInstance, 'clear');
+    unmount();
+
+    expect(apiInstance.clear).not.toHaveBeenCalled();
+  });
+
+  it('uses pass through API when provided', function () {
+    const myClient = new Client();
+    const {unmount} = mountWithTheme(<MyComponent api={myClient} />);
+
+    jest.spyOn(myClient, 'clear');
+    unmount();
+
+    expect(myClient.clear).toHaveBeenCalled();
+  });
+});

+ 0 - 43
tests/js/spec/utils/withApi.spec.jsx

@@ -1,43 +0,0 @@
-import {mountWithTheme} from 'sentry-test/enzyme';
-
-import withApi from 'app/utils/withApi';
-
-describe('withApi', function () {
-  let apiInstance;
-  const MyComponent = jest.fn(props => {
-    apiInstance = props.api;
-    return <div />;
-  });
-
-  it('renders MyComponent with an api prop', function () {
-    const MyComponentWithApi = withApi(MyComponent);
-    mountWithTheme(<MyComponentWithApi />);
-    expect(MyComponent).toHaveBeenCalledWith(
-      expect.objectContaining({
-        api: expect.anything(),
-      }),
-      expect.anything()
-    );
-  });
-
-  it('cancels pending API requests when component is unmounted', function () {
-    const MyComponentWithApi = withApi(MyComponent);
-    const wrapper = mountWithTheme(<MyComponentWithApi />);
-    jest.spyOn(apiInstance, 'clear');
-    expect(apiInstance.clear).not.toHaveBeenCalled();
-    wrapper.unmount();
-    expect(apiInstance.clear).toHaveBeenCalled();
-
-    apiInstance.clear.mockRestore();
-  });
-
-  it('does not cancels pending API requests if persistInFlight is enabled', function () {
-    const MyComponentWithApi = withApi(MyComponent, {persistInFlight: true});
-    const wrapper = mountWithTheme(<MyComponentWithApi />);
-    jest.spyOn(apiInstance, 'clear');
-    wrapper.unmount();
-    expect(apiInstance.clear).not.toHaveBeenCalled();
-
-    apiInstance.clear.mockRestore();
-  });
-});

+ 46 - 0
tests/js/spec/utils/withApi.spec.tsx

@@ -0,0 +1,46 @@
+import {mountWithTheme} from 'sentry-test/reactTestingLibrary';
+
+import {Client} from 'app/api';
+import withApi from 'app/utils/withApi';
+
+describe('withApi', function () {
+  let apiInstance: Client | undefined;
+
+  type Props = {
+    /**
+     * Test passthrough API clients
+     */
+    api?: Client;
+  };
+
+  const MyComponent = jest.fn((props: Props) => {
+    apiInstance = props.api;
+    return <div />;
+  });
+
+  it('renders MyComponent with an api prop', function () {
+    const MyComponentWithApi = withApi(MyComponent);
+    mountWithTheme(<MyComponentWithApi />);
+
+    expect(MyComponent).toHaveBeenCalledWith(
+      expect.objectContaining({api: apiInstance}),
+      expect.anything()
+    );
+  });
+
+  it('cancels pending API requests when component is unmounted', async function () {
+    const MyComponentWithApi = withApi(MyComponent);
+    const wrapper = mountWithTheme(<MyComponentWithApi />);
+
+    if (apiInstance === undefined) {
+      throw new Error("apiInstance wasn't defined");
+    }
+
+    jest.spyOn(apiInstance, 'clear');
+
+    expect(apiInstance?.clear).not.toHaveBeenCalled();
+    wrapper.unmount();
+
+    expect(apiInstance?.clear).toHaveBeenCalled();
+  });
+});