#5618 Bug fix - Duplicate context menu items after marketplace activa…

…tion (#5621)

Co-authored-by: Ben Loe <ben@pixiebrix.com>

src/recipes/recipesHooks.ts

@@ -25,6 +25,13 @@ import { type UseCachedQueryResult } from "@/types/sliceTypes";
 
 /**
  * Lookup a recipe from the registry by ID, or null if it doesn't exist
+ *
+ * Note: This hook will return a new result object every time any piece of
+ *  the state changes. So, if you destructure the "data" field at a call-site,
+ *  and then you use the resulting recipe as a dependency for another hook,
+ *  you should assume that the recipe will change reference and fire your hook
+ *  any time any of the various fetching/loading flags change in the state of
+ *  this hook.
  */
 export function useRecipe(
   id: RegistryId

src/sidebar/activateRecipe/ActivateRecipePanel.tsx

@@ -15,7 +15,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import React, { useEffect, useReducer, useRef } from "react";
+import React, { useCallback, useEffect, useReducer, useRef } from "react";
 import { type RegistryId } from "@/types/registryTypes";
 import Loader from "@/components/Loader";
 import activationCompleteImage from "@img/blueprint-activation-complete.png";
@@ -41,7 +41,6 @@ import useWizard from "@/activation/useWizard";
 import RequireRecipe, {
   type RecipeState,
 } from "@/sidebar/activateRecipe/RequireRecipe";
-import { useAsyncEffect } from "use-async-effect";
 import { persistor } from "@/sidebar/store";
 
 const { actions } = sidebarSlice;
@@ -160,7 +159,7 @@ const ActivateRecipePanelContent: React.FC<RecipeState> = ({
     void checkPermissions();
   };
 
-  async function activateRecipe() {
+  const activateRecipe = useCallback(async () => {
     if (state.isActivating || state.isActivated) {
       return;
     }
@@ -178,15 +177,22 @@ const ActivateRecipePanelContent: React.FC<RecipeState> = ({
     } else {
       stateDispatch(activateError(error));
     }
-  }
+  }, [
+    marketplaceActivateRecipe,
+    recipe,
+    state.isActivated,
+    state.isActivating,
+  ]);
 
-  useAsyncEffect(async () => {
+  useEffect(() => {
     if (!state.needsPermissions && canAutoActivate) {
-      await activateRecipe();
+      // State is checked inside this function call to prevent duplicate calls,
+      // so we can simply dispatch asynchronously with "void" here.
+      void activateRecipe();
     } else {
       stateDispatch(initialize());
     }
-  }, [canAutoActivate, state.needsPermissions]);
+  }, [activateRecipe, canAutoActivate, state.needsPermissions]);
 
   if (!state.isInitialized || state.isActivating) {
     return <Loader />;

src/sidebar/activateRecipe/RequireRecipe.tsx

@@ -71,41 +71,46 @@ const RequireRecipe: React.FC<RequireRecipeProps> = ({
     listing?.package?.verbose_name ?? listing?.package?.name ?? "Unnamed mod";
 
   // Quick Bar & Permissions
-  const [
-    quickBarAndPermissions,
-    isLoadingQuickBarAndPermissions,
-    quickBarAndPermissionsError,
-  ] = useAsyncState(async () => {
-    if (!recipe) {
-      return null;
-    }
+  const [quickBarAndPermissions, , quickBarAndPermissionsError] =
+    useAsyncState(async () => {
+      if (!recipe) {
+        return null;
+      }
 
-    const resolvedRecipeConfigs = await resolveRecipe(
-      recipe,
-      recipe.extensionPoints
-    );
-    const includesQuickBar = await includesQuickBarExtensionPoint(
-      resolvedRecipeConfigs
-    );
-    const needsPermissions = async (formValues: WizardValues) => {
-      const serviceAuths = formValues.services.filter(({ config }) =>
-        Boolean(config)
+      const resolvedRecipeConfigs = await resolveRecipe(
+        recipe,
+        recipe.extensionPoints
       );
-      const collectedPermissions = await collectPermissions(
-        resolvedRecipeConfigs,
-        serviceAuths
+      const includesQuickBar = await includesQuickBarExtensionPoint(
+        resolvedRecipeConfigs
       );
-
-      if (isEmpty(collectedPermissions)) {
-        return false;
-      }
-
-      const hasPermissions = await containsPermissions(collectedPermissions);
-      return !hasPermissions;
-    };
-
-    return { includesQuickBar, needsPermissions };
-  }, [recipe]);
+      const needsPermissions = async (formValues: WizardValues) => {
+        const serviceAuths = formValues.services.filter(({ config }) =>
+          Boolean(config)
+        );
+        const collectedPermissions = await collectPermissions(
+          resolvedRecipeConfigs,
+          serviceAuths
+        );
+
+        if (isEmpty(collectedPermissions)) {
+          return false;
+        }
+
+        const hasPermissions = await containsPermissions(collectedPermissions);
+        return !hasPermissions;
+      };
+
+      return { includesQuickBar, needsPermissions };
+    }, [recipe]);
+
+  // The "fetching" flag on useAsyncState toggles back and forth when the recipe dependency updates
+  // from null to the loaded recipe. This causes the loader to flash from this component, which
+  // causes the children tree to completely un-mount and then mount again, which is not ideal
+  // for the children. Instead, we're just waiting until the state receives a value, and then
+  // we never set the loading indicator again for this piece of the state, even if the async state
+  // happens to fetch again for some reason.
+  const isLoadingQuickBarAndPermissions = quickBarAndPermissions == null;
 
   // Auth Options
   const { authOptions, isLoading: isLoadingAuthOptions } = useAuthOptions();