From b9acfe6214cd613d355700ee16da1dbd0a521084 Mon Sep 17 00:00:00 2001
From: Lukasz Ostrowski <lukasz.ostrowski@saleor.io>
Date: Mon, 27 Feb 2023 16:35:35 +0100
Subject: [PATCH] Improve AppPage (#3216)

* WIP - reduce rerenders and change how dashboard sends events to app

* Cleanup

* Refactor

* Add loader behind the iframe

* Fix linter
---
 src/apps/components/AppFrame/AppFrame.tsx     | 101 ++++++++----------
 .../components/AppFrame/appActionsHandler.ts  |  34 ++++--
 src/apps/components/AppFrame/styles.ts        |  10 ++
 src/apps/components/AppFrame/useAppActions.ts |  43 +++++---
 .../AppFrame/useAppDashboardUpdates.ts        |  58 ++++++++++
 .../components/AppFrame/usePostToExtension.ts |  22 ++++
 .../components/AppPage/AppPage.stories.tsx    |   2 +-
 src/apps/components/AppPage/AppPage.tsx       |   7 +-
 src/apps/components/AppPage/index.ts          |   1 -
 src/apps/components/AppPage/styles.ts         |   1 +
 src/apps/index.tsx                            |   2 +-
 src/apps/views/App/{App.tsx => AppView.tsx}   |  32 +++---
 src/apps/views/App/index.ts                   |   3 +-
 src/apps/views/AppSettings/AppSettings.tsx    |   2 +-
 src/new-apps/index.tsx                        |  10 +-
 15 files changed, 219 insertions(+), 109 deletions(-)
 create mode 100644 src/apps/components/AppFrame/useAppDashboardUpdates.ts
 create mode 100644 src/apps/components/AppFrame/usePostToExtension.ts
 rename src/apps/views/App/{App.tsx => AppView.tsx} (68%)

diff --git a/src/apps/components/AppFrame/AppFrame.tsx b/src/apps/components/AppFrame/AppFrame.tsx
index 58b1312da..9ff2a11b4 100644
--- a/src/apps/components/AppFrame/AppFrame.tsx
+++ b/src/apps/components/AppFrame/AppFrame.tsx
@@ -1,16 +1,14 @@
+import { useAppDashboardUpdates } from "@dashboard/apps/components/AppFrame/useAppDashboardUpdates";
 import {
   AppDetailsUrlQueryParams,
-  getAppDeepPathFromDashboardUrl,
   prepareFeatureFlagsList,
   resolveAppIframeUrl,
 } from "@dashboard/apps/urls";
 import { useAllFlags } from "@dashboard/hooks/useFlags";
-import useLocale from "@dashboard/hooks/useLocale";
-import useShop from "@dashboard/hooks/useShop";
+import { CircularProgress } from "@material-ui/core";
 import { useTheme } from "@saleor/macaw-ui";
 import clsx from "clsx";
-import React, { useEffect } from "react";
-import { useLocation } from "react-router";
+import React from "react";
 
 import { useStyles } from "./styles";
 import { useAppActions } from "./useAppActions";
@@ -39,46 +37,36 @@ export const AppFrame: React.FC<Props> = ({
   onError,
   refetch,
 }) => {
-  const shop = useShop();
-  const frameRef = React.useRef<HTMLIFrameElement>(null);
+  const frameRef = React.useRef<HTMLIFrameElement | null>(null);
   const { themeType } = useTheme();
   const classes = useStyles();
   const appOrigin = getOrigin(src);
   const flags = useAllFlags();
-  const { postToExtension } = useAppActions(frameRef, appOrigin, appId);
-  const location = useLocation();
-  const { locale } = useLocale();
 
-  useEffect(() => {
-    postToExtension({
-      type: "localeChanged",
-      payload: {
-        locale,
-      },
-    });
-  }, [locale, postToExtension]);
+  /**
+   * React on messages from App
+   */
+  const { postToExtension, handshakeDone, setHandshakeDone } = useAppActions(
+    frameRef.current,
+    appOrigin,
+    appId,
+    appToken,
+  );
 
-  useEffect(() => {
-    postToExtension({
-      type: "theme",
-      payload: {
-        theme: themeType,
-      },
-    });
-  }, [themeType, postToExtension]);
-
-  useEffect(() => {
-    postToExtension({
-      type: "redirect",
-      payload: {
-        path: getAppDeepPathFromDashboardUrl(location.pathname, appId),
-      },
-    });
-  }, [location.pathname]);
+  /**
+   * Listen to Dashboard context like theme or locale and inform app about it
+   */
+  useAppDashboardUpdates(frameRef.current, appOrigin, handshakeDone, appId);
 
   useTokenRefresh(appToken, refetch);
 
   const handleLoad = () => {
+    /**
+     * @deprecated
+     *
+     * Move handshake to notifyReady, so app is requesting token after it's ready to receive it
+     * Currently handshake it 2 times, for compatibility
+     */
     postToExtension({
       type: "handshake",
       payload: {
@@ -86,34 +74,33 @@ export const AppFrame: React.FC<Props> = ({
         version: 1,
       },
     });
-    postToExtension({
-      type: "theme",
-      payload: {
-        theme: themeType,
-      },
-    });
+
+    setHandshakeDone(true);
 
     if (onLoad) {
       onLoad();
     }
   };
 
-  if (!shop?.domain.host) {
-    return null;
-  }
-
   return (
-    <iframe
-      ref={frameRef}
-      src={resolveAppIframeUrl(appId, src, {
-        ...params,
-        featureFlags: prepareFeatureFlagsList(flags),
-        theme: themeType,
-      })}
-      onError={onError}
-      onLoad={handleLoad}
-      className={clsx(classes.iframe, className)}
-      sandbox="allow-same-origin allow-forms allow-scripts"
-    />
+    <>
+      <div className={classes.loader}>
+        <CircularProgress color="primary" />
+      </div>
+      <iframe
+        ref={frameRef}
+        src={resolveAppIframeUrl(appId, src, {
+          ...params,
+          featureFlags: prepareFeatureFlagsList(flags),
+          theme: themeType,
+        })}
+        onError={onError}
+        onLoad={handleLoad}
+        className={clsx(classes.iframe, className, {
+          [classes.iframeHidden]: !handshakeDone,
+        })}
+        sandbox="allow-same-origin allow-forms allow-scripts"
+      />
+    </>
   );
 };
diff --git a/src/apps/components/AppFrame/appActionsHandler.ts b/src/apps/components/AppFrame/appActionsHandler.ts
index 5d6506f23..00a763888 100644
--- a/src/apps/components/AppFrame/appActionsHandler.ts
+++ b/src/apps/components/AppFrame/appActionsHandler.ts
@@ -1,3 +1,4 @@
+import { usePostToExtension } from "@dashboard/apps/components/AppFrame/usePostToExtension";
 import { useExternalApp } from "@dashboard/apps/components/ExternalAppContext/ExternalAppContext";
 import { getAppMountUri } from "@dashboard/config";
 import useNavigator from "@dashboard/hooks/useNavigator";
@@ -169,19 +170,34 @@ const useHandleUpdateRoutingAction = (appId: string) => ({
   },
 });
 
-const useNotifyReadyAction = () => ({
-  handle(action: NotifyReady) {
-    console.debug(
-      `Handling NotifyReady action with ID: ${action.payload.actionId}`,
-    );
-    console.warn("Not implemented");
-    return createResponseStatus(action.payload.actionId, true);
-  },
-});
+/**
+ * TODO Remove prop drilling, consume context
+ */
+const useNotifyReadyAction = (
+  frameEl: HTMLIFrameElement | null,
+  appOrigin: string,
+  appToken: string,
+) => {
+  const postToExtension = usePostToExtension(frameEl, appOrigin);
+
+  return {
+    handle(action: NotifyReady) {
+      postToExtension({
+        type: "handshake",
+        payload: {
+          token: appToken,
+          version: 1,
+        },
+      });
+      return createResponseStatus(action.payload.actionId, true);
+    },
+  };
+};
 
 export const AppActionsHandler = {
   useHandleNotificationAction,
   useHandleUpdateRoutingAction,
   useHandleRedirectAction,
   useNotifyReadyAction,
+  createResponseStatus,
 };
diff --git a/src/apps/components/AppFrame/styles.ts b/src/apps/components/AppFrame/styles.ts
index 74445c16e..0ab9389ac 100644
--- a/src/apps/components/AppFrame/styles.ts
+++ b/src/apps/components/AppFrame/styles.ts
@@ -7,6 +7,16 @@ export const useStyles = makeStyles(
       height: "100%",
       border: "none",
     },
+    iframeHidden: {
+      visibility: "hidden",
+    },
+    loader: {
+      position: "absolute",
+      top: "50%",
+      left: "50%",
+      transform: "translate(-50%, -50%)",
+      zIndex: -1,
+    },
   }),
   { name: "AppFrame" },
 );
diff --git a/src/apps/components/AppFrame/useAppActions.ts b/src/apps/components/AppFrame/useAppActions.ts
index eef9588b2..bcecefb5f 100644
--- a/src/apps/components/AppFrame/useAppActions.ts
+++ b/src/apps/components/AppFrame/useAppActions.ts
@@ -1,22 +1,32 @@
 import { AppActionsHandler } from "@dashboard/apps/components/AppFrame/appActionsHandler";
-import {
-  Actions,
-  DispatchResponseEvent,
-  Events,
-} from "@saleor/app-sdk/app-bridge";
-import React from "react";
+import { usePostToExtension } from "@dashboard/apps/components/AppFrame/usePostToExtension";
+import { Actions, DispatchResponseEvent } from "@saleor/app-sdk/app-bridge";
+import React, { useState } from "react";
 
 export const useAppActions = (
-  frameEl: React.MutableRefObject<HTMLIFrameElement | null>,
+  frameEl: HTMLIFrameElement | null,
   appOrigin: string,
   appId: string,
+  appToken: string,
 ) => {
+  const postToExtension = usePostToExtension(frameEl, appOrigin);
+
   const { handle: handleNotification } =
     AppActionsHandler.useHandleNotificationAction();
   const { handle: handleUpdateRouting } =
     AppActionsHandler.useHandleUpdateRoutingAction(appId);
   const { handle: handleRedirect } =
     AppActionsHandler.useHandleRedirectAction(appId);
+  const { handle: handleNotifyReady } = AppActionsHandler.useNotifyReadyAction(
+    frameEl,
+    appOrigin,
+    appToken,
+  );
+
+  /**
+   * Store if app has performed a handshake with Dashboard, to avoid sending events before that
+   */
+  const [handshakeDone, setHandshakeDone] = useState(false);
 
   const handleAction = (action: Actions | undefined): DispatchResponseEvent => {
     switch (action?.type) {
@@ -29,8 +39,15 @@ export const useAppActions = (
       case "updateRouting": {
         return handleUpdateRouting(action);
       }
+      /**
+       * Send handshake after app informs its ready and mounted
+       */
       case "notifyReady": {
-        console.warn("Not implemented");
+        const response = handleNotifyReady(action);
+
+        setHandshakeDone(true);
+
+        return response;
       }
       default: {
         throw new Error("Unknown action type");
@@ -38,12 +55,6 @@ export const useAppActions = (
     }
   };
 
-  const postToExtension = (event: Events) => {
-    if (frameEl?.current?.contentWindow) {
-      frameEl.current.contentWindow.postMessage(event, appOrigin);
-    }
-  };
-
   React.useEffect(() => {
     const handler = (event: MessageEvent<Actions>) => {
       if (event.origin === appOrigin) {
@@ -58,9 +69,11 @@ export const useAppActions = (
     return () => {
       window.removeEventListener("message", handler);
     };
-  }, []);
+  }, [appOrigin, handleAction, postToExtension]);
 
   return {
+    handshakeDone,
     postToExtension,
+    setHandshakeDone,
   };
 };
diff --git a/src/apps/components/AppFrame/useAppDashboardUpdates.ts b/src/apps/components/AppFrame/useAppDashboardUpdates.ts
new file mode 100644
index 000000000..43a7d2dac
--- /dev/null
+++ b/src/apps/components/AppFrame/useAppDashboardUpdates.ts
@@ -0,0 +1,58 @@
+import { usePostToExtension } from "@dashboard/apps/components/AppFrame/usePostToExtension";
+import { getAppDeepPathFromDashboardUrl } from "@dashboard/apps/urls";
+import useLocale from "@dashboard/hooks/useLocale";
+import { useTheme } from "@saleor/macaw-ui";
+import { useEffect } from "react";
+
+/**
+ * TODO: Refactor prop-drilling, use context or some atomic state
+ */
+export const useAppDashboardUpdates = (
+  frameEl: HTMLIFrameElement | null,
+  appOrigin: string,
+  enabled: boolean,
+  appId: string,
+) => {
+  const postToExtension = usePostToExtension(frameEl, appOrigin);
+
+  const { locale } = useLocale();
+  const { themeType } = useTheme();
+
+  useEffect(() => {
+    if (!enabled) {
+      return;
+    }
+
+    postToExtension({
+      type: "localeChanged",
+      payload: {
+        locale,
+      },
+    });
+  }, [enabled, locale, postToExtension]);
+
+  useEffect(() => {
+    if (!enabled) {
+      return;
+    }
+
+    postToExtension({
+      type: "theme",
+      payload: {
+        theme: themeType,
+      },
+    });
+  }, [themeType, postToExtension, enabled]);
+
+  useEffect(() => {
+    if (!enabled) {
+      return;
+    }
+    postToExtension({
+      type: "redirect",
+      payload: {
+        path: getAppDeepPathFromDashboardUrl(location.pathname, appId),
+      },
+    });
+  }, [appId, enabled, postToExtension]);
+};
diff --git a/src/apps/components/AppFrame/usePostToExtension.ts b/src/apps/components/AppFrame/usePostToExtension.ts
new file mode 100644
index 000000000..6e103ad81
--- /dev/null
+++ b/src/apps/components/AppFrame/usePostToExtension.ts
@@ -0,0 +1,22 @@
+import { Events } from "@saleor/app-sdk/app-bridge";
+import { useCallback } from "react";
+
+export const usePostToExtension = (
+  iframeElement: HTMLIFrameElement | null,
+  appOrigin: string,
+) => {
+  const postToExtension = useCallback(
+    (event: Events) => {
+      if (iframeElement?.contentWindow) {
+        try {
+          iframeElement.contentWindow.postMessage(event, appOrigin);
+        } catch (e) {
+          console.error(e);
+        }
+      }
+    },
+    [appOrigin, iframeElement],
+  );
+
+  return postToExtension;
+};
diff --git a/src/apps/components/AppPage/AppPage.stories.tsx b/src/apps/components/AppPage/AppPage.stories.tsx
index b2ed77233..422933af7 100644
--- a/src/apps/components/AppPage/AppPage.stories.tsx
+++ b/src/apps/components/AppPage/AppPage.stories.tsx
@@ -3,7 +3,7 @@ import { storiesOf } from "@storybook/react";
 import React from "react";
 
 import { appDetails } from "../../fixtures";
-import AppPage, { AppPageProps } from "./AppPage";
+import { AppPage, AppPageProps } from "./AppPage";
 
 const props: AppPageProps = {
   data: appDetails,
diff --git a/src/apps/components/AppPage/AppPage.tsx b/src/apps/components/AppPage/AppPage.tsx
index 1c07b8386..dc734d149 100644
--- a/src/apps/components/AppPage/AppPage.tsx
+++ b/src/apps/components/AppPage/AppPage.tsx
@@ -21,12 +21,12 @@ export const AppPage: React.FC<AppPageProps> = ({
 
   return (
     <div className={classes.iframeContainer}>
-      {url && (
+      {url && data.id && data.accessToken && (
         <AppFrame
           src={url}
-          appToken={data?.accessToken ?? ""}
+          appToken={data?.accessToken}
           onError={onError}
-          appId={data?.id ?? ""}
+          appId={data?.id}
           refetch={refetch}
         />
       )}
@@ -35,4 +35,3 @@ export const AppPage: React.FC<AppPageProps> = ({
 };
 
 AppPage.displayName = "AppPage";
-export default AppPage;
diff --git a/src/apps/components/AppPage/index.ts b/src/apps/components/AppPage/index.ts
index f05fbdcaf..351021ad6 100644
--- a/src/apps/components/AppPage/index.ts
+++ b/src/apps/components/AppPage/index.ts
@@ -1,2 +1 @@
 export * from "./AppPage";
-export { default } from "./AppPage";
diff --git a/src/apps/components/AppPage/styles.ts b/src/apps/components/AppPage/styles.ts
index 98e2c9ecd..92a6f2a54 100644
--- a/src/apps/components/AppPage/styles.ts
+++ b/src/apps/components/AppPage/styles.ts
@@ -6,6 +6,7 @@ export const useStyles = makeStyles(
       height: "100%",
     },
     iframeContainer: {
+      position: "relative",
       lineHeight: 0, // It removes extra space between iframe and container
       height: "100%",
       "& > iframe": {
diff --git a/src/apps/index.tsx b/src/apps/index.tsx
index bc306d539..06936a2b5 100644
--- a/src/apps/index.tsx
+++ b/src/apps/index.tsx
@@ -15,7 +15,7 @@ import {
   appPath,
   appsListPath,
 } from "./urls";
-import AppView from "./views/App";
+import { AppView } from "./views/App";
 import AppDetailsView from "./views/AppDetails";
 import AppInstallView from "./views/AppInstall";
 import AppsListView from "./views/AppsList";
diff --git a/src/apps/views/App/App.tsx b/src/apps/views/App/AppView.tsx
similarity index 68%
rename from src/apps/views/App/App.tsx
rename to src/apps/views/App/AppView.tsx
index 99925e97e..043514821 100644
--- a/src/apps/views/App/App.tsx
+++ b/src/apps/views/App/AppView.tsx
@@ -3,18 +3,18 @@ import NotFoundPage from "@dashboard/components/NotFoundPage";
 import { useAppQuery } from "@dashboard/graphql";
 import useNavigator from "@dashboard/hooks/useNavigator";
 import useNotifier from "@dashboard/hooks/useNotifier";
-import React from "react";
+import React, { useCallback } from "react";
 import { useIntl } from "react-intl";
 import { useLocation } from "react-router";
 
-import AppPage from "../../components/AppPage";
+import { AppPage } from "../../components/AppPage";
 import { appsListPath, getAppCompleteUrlFromDashboardUrl } from "../../urls";
 
 interface AppProps {
   id: string;
 }
 
-export const App: React.FC<AppProps> = ({ id }) => {
+export const AppView: React.FC<AppProps> = ({ id }) => {
   const location = useLocation();
   const { data, refetch } = useAppQuery({
     displayLoader: true,
@@ -27,6 +27,15 @@ export const App: React.FC<AppProps> = ({ id }) => {
   const notify = useNotifier();
   const intl = useIntl();
 
+  const handleError = useCallback(
+    () =>
+      notify({
+        status: "error",
+        text: intl.formatMessage(appMessages.failedToFetchAppSettings),
+      }),
+    [intl, notify],
+  );
+
   if (!appExists) {
     return <NotFoundPage onBack={() => navigate(appsListPath)} />;
   }
@@ -37,19 +46,16 @@ export const App: React.FC<AppProps> = ({ id }) => {
     id,
   );
 
+  if (!data || !appCompleteUrl) {
+    return null;
+  }
+
   return (
     <AppPage
-      data={data?.app || null}
-      url={appCompleteUrl || ""}
+      data={data.app}
+      url={appCompleteUrl}
       refetch={refetch}
-      onError={() =>
-        notify({
-          status: "error",
-          text: intl.formatMessage(appMessages.failedToFetchAppSettings),
-        })
-      }
+      onError={handleError}
     />
   );
 };
-
-export default App;
diff --git a/src/apps/views/App/index.ts b/src/apps/views/App/index.ts
index 262af4fff..a7d580c35 100644
--- a/src/apps/views/App/index.ts
+++ b/src/apps/views/App/index.ts
@@ -1,2 +1 @@
-export * from "./App";
-export { default } from "./App";
+export * from "./AppView";
diff --git a/src/apps/views/AppSettings/AppSettings.tsx b/src/apps/views/AppSettings/AppSettings.tsx
index 0ab1e273c..37799e9f8 100644
--- a/src/apps/views/AppSettings/AppSettings.tsx
+++ b/src/apps/views/AppSettings/AppSettings.tsx
@@ -5,7 +5,7 @@ import useNotifier from "@dashboard/hooks/useNotifier";
 import React from "react";
 import { useIntl } from "react-intl";
 
-import AppPage from "../../components/AppPage";
+import { AppPage } from "../../components/AppPage";
 import { appsListPath } from "../../urls";
 
 interface AppSettingsProps {
diff --git a/src/new-apps/index.tsx b/src/new-apps/index.tsx
index af23c0f0e..fb2abdb14 100644
--- a/src/new-apps/index.tsx
+++ b/src/new-apps/index.tsx
@@ -2,7 +2,7 @@ import {
   AppDetailsUrlQueryParams,
   AppInstallUrlQueryParams,
 } from "@dashboard/apps/urls";
-import AppView from "@dashboard/apps/views/App";
+import { AppView } from "@dashboard/apps/views/App";
 import AppDetailsView from "@dashboard/apps/views/AppDetails";
 import AppInstallView from "@dashboard/apps/views/AppInstall";
 import { sectionNames } from "@dashboard/intl";
@@ -26,9 +26,9 @@ const AppDetails: React.FC<RouteComponentProps<{ id: string }>> = ({
   );
 };
 
-const App: React.FC<RouteComponentProps<{ id: string }>> = ({ match }) => (
-  <AppView id={decodeURIComponent(match.params.id)} />
-);
+const AppViewRoute: React.FC<RouteComponentProps<{ id: string }>> = ({
+  match,
+}) => <AppView id={decodeURIComponent(match.params.id)} />;
 
 const AppInstall: React.FC<RouteComponentProps> = props => {
   const qs = parseQs(location.search.substr(1));
@@ -58,7 +58,7 @@ const Apps = () => {
           path={AppPaths.resolveAppDetailsPath(":id")}
           component={AppDetails}
         />
-        <Route path={AppPaths.resolveAppPath(":id")} component={App} />
+        <Route path={AppPaths.resolveAppPath(":id")} component={AppViewRoute} />
       </Switch>
     </>
   );