From 4ba15b7a82832c391b793f746226da1d90536c47 Mon Sep 17 00:00:00 2001
From: Christine Chambers <christine.d.hang@gmail.com>
Date: Mon, 8 Apr 2019 11:44:49 -0700
Subject: [PATCH] feat: callback hooks for Reactified components (#131)

* feat: callback hooks for Reactified components

Add a second argument to the exported `reactify` method to allow wrapped component to pass in callback hooks for React lifecycle methods. This PR only exposes the willUnmount hook. Other hooks can be added in the future.

* fix: improve code coverage

* chore: rename willUnmount hook to componentWillUnmount
---
 .../src/components/reactify.tsx                 |  9 +++++++++
 .../test/components/reactify.test.tsx           | 17 +++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/components/reactify.tsx b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/components/reactify.tsx
index 9075260f3e56..96d7ae19f455 100644
--- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/components/reactify.tsx
+++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/src/components/reactify.tsx
@@ -17,6 +17,11 @@ export type ReactifyProps = {
   className?: string;
 };
 
+// TODO: add more React lifecycle callbacks as needed
+export type LifeCycleCallbacks = {
+  componentWillUnmount?: () => void;
+};
+
 export interface RenderFuncType<Props> {
   (container: HTMLDivElement, props: Readonly<Props & ReactifyProps>): void;
   displayName?: string;
@@ -26,6 +31,7 @@ export interface RenderFuncType<Props> {
 
 export default function reactify<Props extends object>(
   renderFn: RenderFuncType<Props>,
+  callbacks?: LifeCycleCallbacks,
 ): React.ComponentClass<Props & ReactifyProps> {
   class ReactifiedComponent extends React.Component<Props & ReactifyProps> {
     // eslint-disable-next-line react/sort-comp
@@ -46,6 +52,9 @@ export default function reactify<Props extends object>(
 
     componentWillUnmount() {
       this.container = undefined;
+      if (callbacks && callbacks.componentWillUnmount) {
+        callbacks.componentWillUnmount();
+      }
     }
 
     setContainerRef(ref: HTMLDivElement) {
diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/components/reactify.test.tsx b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/components/reactify.test.tsx
index 629779f7b86b..13534b8d7763 100644
--- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/components/reactify.test.tsx
+++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/components/reactify.test.tsx
@@ -22,7 +22,10 @@ describe('reactify(renderFn)', () => {
     content: 'ghi',
   };
 
+  const willUnmountCb = jest.fn();
+
   const TheChart = reactify(renderFn);
+  const TheChartWithWillUnmountHook = reactify(renderFn, { componentWillUnmount: willUnmountCb });
 
   class TestComponent extends React.PureComponent<{}, { content: string }, any> {
     constructor(props = {}) {
@@ -42,6 +45,12 @@ describe('reactify(renderFn)', () => {
       return <TheChart id="test" content={content} />;
     }
   }
+  /* eslint-disable-next-line react/no-multi-comp */
+  class AnotherTestComponent extends React.PureComponent<{}, {}, any> {
+    render() {
+      return <TheChartWithWillUnmountHook id="another_test" />;
+    }
+  }
 
   it('returns a React component class', done => {
     const wrapper = mount(<TestComponent />);
@@ -92,4 +101,12 @@ describe('reactify(renderFn)', () => {
     new AnotherChart({ id: 'test' }).execute();
     expect(anotherRenderFn).not.toHaveBeenCalled();
   });
+  it('calls willUnmount hook when it is provided', done => {
+    const wrapper = mount(<AnotherTestComponent />);
+    setTimeout(() => {
+      wrapper.unmount();
+      expect(willUnmountCb).toHaveBeenCalledTimes(1);
+      done();
+    }, 20);
+  });
 });