Fix UI plugin race conditions (#5523)

* useScript to return load state of scripts
* Wait for scripts to load before rendering

Also moves plugin code into plugins.tsx

ui/v2.5/src/App.tsx

@@ -18,7 +18,6 @@ import locales, { registerCountry } from "src/locales";
 import {
   useConfiguration,
   useConfigureUI,
-  usePlugins,
   useSystemStatus,
 } from "src/core/StashService";
 import flattenMessages from "./utils/flattenMessages";
@@ -40,12 +39,9 @@ import { releaseNotes } from "./docs/en/ReleaseNotes";
 import { getPlatformURL } from "./core/createClient";
 import { lazyComponent } from "./utils/lazyComponent";
 import { isPlatformUniquelyRenderedByApple } from "./utils/apple";
-import useScript, { useCSS } from "./hooks/useScript";
-import { useMemoOnce } from "./hooks/state";
 import Event from "./hooks/event";
-import { uniq } from "lodash-es";
 
-import { PluginRoutes } from "./plugins";
+import { PluginRoutes, PluginsLoader } from "./plugins";
 
 // import plugin_api to run code
 import "./pluginApi";
@@ -97,54 +93,6 @@ function languageMessageString(language: string) {
   return language.replace(/-/, "");
 }
 
-type PluginList = NonNullable<Required<GQL.PluginsQuery["plugins"]>>;
-
-// sort plugins by their dependencies
-function sortPlugins(plugins: PluginList) {
-  type Node = { id: string; afters: string[] };
-
-  let nodes: Record<string, Node> = {};
-  let sorted: PluginList = [];
-  let visited: Record<string, boolean> = {};
-
-  plugins.forEach((v) => {
-    let from = v.id;
-
-    if (!nodes[from]) nodes[from] = { id: from, afters: [] };
-
-    v.requires?.forEach((to) => {
-      if (!nodes[to]) nodes[to] = { id: to, afters: [] };
-      if (!nodes[to].afters.includes(from)) nodes[to].afters.push(from);
-    });
-  });
-
-  function visit(idstr: string, ancestors: string[] = []) {
-    let node = nodes[idstr];
-    const { id } = node;
-
-    if (visited[idstr]) return;
-
-    ancestors.push(id);
-    visited[idstr] = true;
-    node.afters.forEach(function (afterID) {
-      if (ancestors.indexOf(afterID) >= 0)
-        throw new Error("closed chain : " + afterID + " is in " + id);
-      visit(afterID.toString(), ancestors.slice());
-    });
-
-    const plugin = plugins.find((v) => v.id === id);
-    if (plugin) {
-      sorted.unshift(plugin);
-    }
-  }
-
-  Object.keys(nodes).forEach((n) => {
-    visit(n);
-  });
-
-  return sorted;
-}
-
 const AppContainer: React.FC<React.PropsWithChildren<{}>> = PatchFunction(
   "App",
   (props: React.PropsWithChildren<{}>) => {
@@ -215,46 +163,6 @@ export const App: React.FC = () => {
     setLocale();
   }, [customMessages, language]);
 
-  const {
-    data: plugins,
-    loading: pluginsLoading,
-    error: pluginsError,
-  } = usePlugins();
-
-  const sortedPlugins = useMemoOnce(() => {
-    return [
-      sortPlugins(plugins?.plugins ?? []),
-      !pluginsLoading && !pluginsError,
-    ];
-  }, [plugins?.plugins, pluginsLoading, pluginsError]);
-
-  const pluginJavascripts = useMemoOnce(() => {
-    return [
-      uniq(
-        sortedPlugins
-          ?.filter((plugin) => plugin.enabled && plugin.paths.javascript)
-          .map((plugin) => plugin.paths.javascript!)
-          .flat() ?? []
-      ),
-      !!sortedPlugins && !pluginsLoading && !pluginsError,
-    ];
-  }, [sortedPlugins, pluginsLoading, pluginsError]);
-
-  const pluginCSS = useMemoOnce(() => {
-    return [
-      uniq(
-        sortedPlugins
-          ?.filter((plugin) => plugin.enabled && plugin.paths.css)
-          .map((plugin) => plugin.paths.css!)
-          .flat() ?? []
-      ),
-      !!sortedPlugins && !pluginsLoading && !pluginsError,
-    ];
-  }, [sortedPlugins, pluginsLoading, pluginsError]);
-
-  useScript(pluginJavascripts ?? [], !pluginsLoading && !pluginsError);
-  useCSS(pluginCSS ?? [], !pluginsLoading && !pluginsError);
-
   const location = useLocation();
   const history = useHistory();
   const setupMatch = useRouteMatch(["/setup", "/migrate"]);
@@ -365,43 +273,45 @@ export const App: React.FC = () => {
   const titleProps = makeTitleProps();
 
   return (
-    <AppContainer>
+    <ErrorBoundary>
-      <ErrorBoundary>
+      {messages ? (
-        {messages ? (
+        <IntlProvider
-          <IntlProvider
+          locale={language}
-            locale={language}
+          messages={messages}
-            messages={messages}
+          formats={intlFormats}
-            formats={intlFormats}
+        >
-          >
+          <PluginsLoader>
-            <ConfigurationProvider
+            <AppContainer>
-              configuration={config.data?.configuration}
+              <ConfigurationProvider
-              loading={config.loading}
+                configuration={config.data?.configuration}
-            >
+                loading={config.loading}
-              {maybeRenderReleaseNotes()}
+              >
-              <ToastProvider>
+                {maybeRenderReleaseNotes()}
-                <ConnectionMonitor />
+                <ToastProvider>
-                <Suspense fallback={<LoadingIndicator />}>
+                  <ConnectionMonitor />
-                  <LightboxProvider>
+                  <Suspense fallback={<LoadingIndicator />}>
-                    <ManualProvider>
+                    <LightboxProvider>
-                      <InteractiveProvider>
+                      <ManualProvider>
-                        <Helmet {...titleProps} />
+                        <InteractiveProvider>
-                        {maybeRenderNavbar()}
+                          <Helmet {...titleProps} />
-                        <div
+                          {maybeRenderNavbar()}
-                          className={`main container-fluid ${
+                          <div
-                            appleRendering ? "apple" : ""
+                            className={`main container-fluid ${
-                          }`}
+                              appleRendering ? "apple" : ""
-                        >
+                            }`}
-                          {renderContent()}
+                          >
-                        </div>
+                            {renderContent()}
-                      </InteractiveProvider>
+                          </div>
-                    </ManualProvider>
+                        </InteractiveProvider>
-                  </LightboxProvider>
+                      </ManualProvider>
-                </Suspense>
+                    </LightboxProvider>
-              </ToastProvider>
+                  </Suspense>
-            </ConfigurationProvider>
+                </ToastProvider>
-          </IntlProvider>
+              </ConfigurationProvider>
-        ) : null}
+            </AppContainer>
-      </ErrorBoundary>
+          </PluginsLoader>
-    </AppContainer>
+        </IntlProvider>
+      ) : null}
+    </ErrorBoundary>
   );
 };

ui/v2.5/src/hooks/useScript.tsx

@@ -1,6 +1,9 @@
-import { useEffect, useMemo } from "react";
+import { useEffect, useMemo, useState } from "react";
+
+const useScript = (urls: string | string[], condition: boolean = true) => {
+  // array of booleans to track the loading state of each script
+  const [loadStates, setLoadStates] = useState<boolean[]>();
 
-const useScript = (urls: string | string[], condition?: boolean) => {
   const urlArray = useMemo(() => {
     if (!Array.isArray(urls)) {
       return [urls];
@@ -10,12 +13,25 @@ const useScript = (urls: string | string[], condition?: boolean) => {
   }, [urls]);
 
   useEffect(() => {
+    if (condition) {
+      setLoadStates(urlArray.map(() => false));
+    }
+
     const scripts = urlArray.map((url) => {
       const script = document.createElement("script");
 
       script.src = url;
       script.async = false;
       script.defer = true;
+
+      function onLoad() {
+        setLoadStates((prev) =>
+          prev!.map((state, i) => (i === urlArray.indexOf(url) ? true : state))
+        );
+      }
+      script.addEventListener("load", onLoad);
+      script.addEventListener("error", onLoad); // handle error as well
+
       return script;
     });
 
@@ -33,6 +49,12 @@ const useScript = (urls: string | string[], condition?: boolean) => {
       }
     };
   }, [urlArray, condition]);
+
+  return (
+    condition &&
+    loadStates &&
+    (loadStates.length === 0 || loadStates.every((state) => state))
+  );
 };
 
 export const useCSS = (urls: string | string[], condition?: boolean) => {

ui/v2.5/src/locales/en-GB.json

@@ -1121,7 +1121,8 @@
   "last_played_at": "Last Played At",
   "library": "Library",
   "loading": {
-    "generic": "Loading…"
+    "generic": "Loading…",
+    "plugins": "Loading plugins…"
   },
   "marker_count": "Marker Count",
   "markers": "Markers",

ui/v2.5/src/plugins.tsx

@@ -1,5 +1,122 @@
 import React from "react";
 import { PatchFunction } from "./patch";
+import { usePlugins } from "./core/StashService";
+import { useMemoOnce } from "./hooks/state";
+import { uniq } from "lodash-es";
+import useScript, { useCSS } from "./hooks/useScript";
+import { PluginsQuery } from "./core/generated-graphql";
+import { LoadingIndicator } from "./components/Shared/LoadingIndicator";
+import { FormattedMessage } from "react-intl";
+
+type PluginList = NonNullable<Required<PluginsQuery["plugins"]>>;
+
+// sort plugins by their dependencies
+function sortPlugins(plugins: PluginList) {
+  type Node = { id: string; afters: string[] };
+
+  let nodes: Record<string, Node> = {};
+  let sorted: PluginList = [];
+  let visited: Record<string, boolean> = {};
+
+  plugins.forEach((v) => {
+    let from = v.id;
+
+    if (!nodes[from]) nodes[from] = { id: from, afters: [] };
+
+    v.requires?.forEach((to) => {
+      if (!nodes[to]) nodes[to] = { id: to, afters: [] };
+      if (!nodes[to].afters.includes(from)) nodes[to].afters.push(from);
+    });
+  });
+
+  function visit(idstr: string, ancestors: string[] = []) {
+    let node = nodes[idstr];
+    const { id } = node;
+
+    if (visited[idstr]) return;
+
+    ancestors.push(id);
+    visited[idstr] = true;
+    node.afters.forEach(function (afterID) {
+      if (ancestors.indexOf(afterID) >= 0)
+        throw new Error("closed chain : " + afterID + " is in " + id);
+      visit(afterID.toString(), ancestors.slice());
+    });
+
+    const plugin = plugins.find((v) => v.id === id);
+    if (plugin) {
+      sorted.unshift(plugin);
+    }
+  }
+
+  Object.keys(nodes).forEach((n) => {
+    visit(n);
+  });
+
+  return sorted;
+}
+
+// load all plugins and their dependencies
+// returns true when all plugins are loaded, regardess of success or failure
+function useLoadPlugins() {
+  const {
+    data: plugins,
+    loading: pluginsLoading,
+    error: pluginsError,
+  } = usePlugins();
+
+  const sortedPlugins = useMemoOnce(() => {
+    return [
+      sortPlugins(plugins?.plugins ?? []),
+      !pluginsLoading && !pluginsError,
+    ];
+  }, [plugins?.plugins, pluginsLoading, pluginsError]);
+
+  const pluginJavascripts = useMemoOnce(() => {
+    return [
+      uniq(
+        sortedPlugins
+          ?.filter((plugin) => plugin.enabled && plugin.paths.javascript)
+          .map((plugin) => plugin.paths.javascript!)
+          .flat() ?? []
+      ),
+      !!sortedPlugins && !pluginsLoading && !pluginsError,
+    ];
+  }, [sortedPlugins, pluginsLoading, pluginsError]);
+
+  const pluginCSS = useMemoOnce(() => {
+    return [
+      uniq(
+        sortedPlugins
+          ?.filter((plugin) => plugin.enabled && plugin.paths.css)
+          .map((plugin) => plugin.paths.css!)
+          .flat() ?? []
+      ),
+      !!sortedPlugins && !pluginsLoading && !pluginsError,
+    ];
+  }, [sortedPlugins, pluginsLoading, pluginsError]);
+
+  const pluginJavascriptLoaded = useScript(
+    pluginJavascripts ?? [],
+    !!pluginJavascripts && !pluginsLoading && !pluginsError
+  );
+  useCSS(pluginCSS ?? [], !pluginsLoading && !pluginsError);
+
+  return !pluginsLoading && !!pluginJavascripts && pluginJavascriptLoaded;
+}
+
+export const PluginsLoader: React.FC<React.PropsWithChildren<{}>> = ({
+  children,
+}) => {
+  const loaded = useLoadPlugins();
+
+  if (!loaded)
+    return (
+      <LoadingIndicator message={<FormattedMessage id="loading.plugins" />} />
+    );
+
+  return <>{children}</>;
+};
 
 export const PluginRoutes: React.FC<React.PropsWithChildren<{}>> =
   PatchFunction("PluginRoutes", (props: React.PropsWithChildren<{}>) => {