Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#5618 Bug fix - Duplicate context menu items after marketplace activation #5621

Merged
merged 1 commit into from
Apr 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/recipes/recipesHooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 13 additions & 7 deletions src/sidebar/activateRecipe/ActivateRecipePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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;
Expand Down Expand Up @@ -160,7 +159,7 @@ const ActivateRecipePanelContent: React.FC<RecipeState> = ({
void checkPermissions();
};

async function activateRecipe() {
const activateRecipe = useCallback(async () => {
if (state.isActivating || state.isActivated) {
return;
}
Expand All @@ -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 />;
Expand Down
69 changes: 37 additions & 32 deletions src/sidebar/activateRecipe/RequireRecipe.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading