Skip to content

Commit

Permalink
refactor: minor refactoring to canvas hooks (except render) (#399)
Browse files Browse the repository at this point in the history
* refactor: minor refactoring to canvas hooks (except render)

* partial revert

* memoize widget container otherwise switch theme causes all components to rerender

* fix lint and some more refactoring of hooks

* minor
  • Loading branch information
Charlie-XIAO authored Feb 3, 2025
1 parent 71ee915 commit dc49f26
Show file tree
Hide file tree
Showing 12 changed files with 52 additions and 57 deletions.
14 changes: 8 additions & 6 deletions src/canvas/App.tsx
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
import WidgetContainer from "./components/WidgetContainer";
import useRenderListener from "./hooks/useRenderListener";
import useRemoveWidgetsListener from "./hooks/useRemoveWidgetsListener";
import useShowToastListener from "./hooks/useShowToastListener";
import { Toaster } from "sonner";
import { Theme as RadixTheme } from "@radix-ui/themes";
import useThemeListener from "./hooks/useThemeListener";
import { useWidgetsStore } from "./hooks/useWidgetsStore";
import { useShallow } from "zustand/shallow";
import {
useRemoveWidgetsListener,
useRenderWidgetsListener,
useShowToastListener,
useTheme,
} from "./hooks";

const App = () => {
const theme = useThemeListener();
const theme = useTheme();
const ids = useWidgetsStore(
useShallow((state) => Object.keys(state.widgets)),
);

useShowToastListener();
useRenderListener();
useRenderWidgetsListener();
useRemoveWidgetsListener();

return (
Expand Down
6 changes: 3 additions & 3 deletions src/canvas/components/WidgetContainer.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { RefObject, useCallback, useRef } from "react";
import { RefObject, memo, useCallback, useRef } from "react";
import Draggable, { DraggableData, DraggableEvent } from "react-draggable";
import { ErrorBoundary } from "react-error-boundary";
import ErrorDisplay from "../components/ErrorDisplay";
Expand All @@ -15,7 +15,7 @@ interface WidgetContainerProps {
id: string;
}

const WidgetContainer = ({ id }: WidgetContainerProps) => {
const WidgetContainer = memo(({ id }: WidgetContainerProps) => {
const { Component, width, height, x, y, opacity } = useWidgetsStore(
(state) => state.widgets[id],
);
Expand Down Expand Up @@ -83,6 +83,6 @@ const WidgetContainer = ({ id }: WidgetContainerProps) => {
</Box>
</Draggable>
);
};
});

export default WidgetContainer;
5 changes: 5 additions & 0 deletions src/canvas/hooks/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export * from "./useRemoveWidgetsListener";
export * from "./useRenderListener";
export * from "./useShowToastListener";
export * from "./useTheme";
export * from "./useWidgetsStore";
27 changes: 3 additions & 24 deletions src/canvas/hooks/useRemoveWidgetsListener.tsx
Original file line number Diff line number Diff line change
@@ -1,32 +1,11 @@
import { useEffect } from "react";
import { listenToRemoveWidgets } from "../../events";
import { removeWidgets, useWidgetsStore } from "./useWidgetsStore";
import { removeWidgets } from "./useWidgetsStore";

/**
* Listen and react to the "remove-widgets" event.
*/
export default function useRemoveWidgetsListener() {
export function useRemoveWidgetsListener() {
useEffect(() => {
const unlisten = listenToRemoveWidgets((event) => {
const { removedIds } = event.payload;
const widgets = useWidgetsStore.getState().widgets;

removedIds.forEach((id) => {
const state = widgets[id];
if (state === null) {
return; // This should not happen but let us be safe
}

// Revoke the blob URLs because they will not be automatically cleaned up, and
// being in the removed IDs means that they will be removed from the canvas
// states and the next time they show up, they will be assigned new blob URLs
URL.revokeObjectURL(state.apisBlobUrl);
if (state.moduleBlobUrl) {
URL.revokeObjectURL(state.moduleBlobUrl);
}
});

removeWidgets(removedIds);
removeWidgets(event.payload.ids);
});

return () => {
Expand Down
2 changes: 1 addition & 1 deletion src/canvas/hooks/useRenderListener.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const baseUrl = new URL(import.meta.url).origin;
/**
* Listen and react to the "render-widgets" event.
*/
export default function useRenderWidgetsListener() {
export function useRenderWidgetsListener() {
const hasInited = useRef(false);

const bundleWidget = useCallback(
Expand Down
5 changes: 1 addition & 4 deletions src/canvas/hooks/useShowToastListener.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ import { useEffect } from "react";
import { listenToShowToast } from "../../events";
import { toast } from "sonner";

/**
* Listen and react to the "show-toast" event.
*/
export default function useShowToastListener() {
export function useShowToastListener() {
useEffect(() => {
const unlisten = listenToShowToast((event) => {
if ("success" in event.payload) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,14 @@
import { useEffect, useState } from "react";
import { listenToSwitchTheme } from "../../events";

/**
* Handle the theme of the canvas.
*
* This hook works by listening to the "switch-theme" event and updating the
* theme state accordingly.
*
* @returns The current theme.
*/
export default function useAppearanceListener() {
export function useTheme() {
const [theme, setTheme] = useState(
window.__DESKULPT_CANVAS_INTERNALS__.initialSettings.app.theme,
);

useEffect(() => {
const unlisten = listenToSwitchTheme((event) => {
setTheme(event.payload);
setTheme(event.payload.theme);
});

return () => {
Expand Down
13 changes: 13 additions & 0 deletions src/canvas/hooks/useWidgetsStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,19 @@ export function updateWidgetSettings(
* Remove a batch of widgets from the store.
*/
export function removeWidgets(ids: string[]) {
const widgets = useWidgetsStore.getState().widgets;

ids.forEach((id) => {
const widget = widgets[id];
if (widget === undefined) {
return; // This should not happen but let us be safe
}
URL.revokeObjectURL(widget.apisBlobUrl);
if (widget.moduleBlobUrl !== undefined) {
URL.revokeObjectURL(widget.moduleBlobUrl);
}
});

useWidgetsStore.setState((state) => ({
widgets: Object.fromEntries(
Object.entries(state.widgets).filter(([id]) => !ids.includes(id)),
Expand Down
9 changes: 6 additions & 3 deletions src/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ import { EventCallback, emitTo, listen } from "@tauri-apps/api/event";
import {
RemoveWidgetsPayload,
RenderWidgetsPayload,
SwitchThemePayload,
UpdateSettingsPayload,
} from "./types/frontend";
import { ShowToastPayload, Theme } from "./types/backend";
import { ShowToastPayload } from "./types/backend";

/**
* Emit the "render-widgets" event to the canvas window.
Expand Down Expand Up @@ -83,7 +84,7 @@ export function listenToUpdateSettings(
*
* @param payload The payload of the event.
*/
export async function emitSwitchThemeToCanvas(payload: Theme) {
export async function emitSwitchThemeToCanvas(payload: SwitchThemePayload) {
await emitTo("canvas", "switch-theme", payload);
}

Expand All @@ -93,7 +94,9 @@ export async function emitSwitchThemeToCanvas(payload: Theme) {
* @param handler The callback function to handle the event.
* @returns A promise that resolves to a function to unlisten to the event.
*/
export function listenToSwitchTheme(handler: EventCallback<Theme>) {
export function listenToSwitchTheme(
handler: EventCallback<SwitchThemePayload>,
) {
return listen("switch-theme", handler);
}

Expand Down
2 changes: 1 addition & 1 deletion src/manager/components/ThemeToggler.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const AppearanceToggler = ({ theme, setTheme }: AppearanceTogglerProps) => {
const toggleTheme = () => {
const newTheme = theme === Theme.LIGHT ? Theme.DARK : Theme.LIGHT;
setTheme(newTheme);
emitSwitchThemeToCanvas(newTheme).catch(console.error);
emitSwitchThemeToCanvas({ theme: newTheme }).catch(console.error);
};

return (
Expand Down
6 changes: 3 additions & 3 deletions src/manager/hooks/useManagerWidgetStates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ export default function useManagerWidgetStates(): UseManagerWidgetStatesOutput {

// If a widget exists in the previous states but does not exist in the newdetected
// configurations, we consider it as removed from the collection
const removedIds = Object.keys(managerWidgetStates).filter(
const ids = Object.keys(managerWidgetStates).filter(
(id) => !(id in detectedConfigs),
);
if (removedIds.length > 0) {
if (ids.length > 0) {
// Notify the cacnvas to clean up resources allocated for removed widgets
await emitRemoveWidgetsToCanvas({ removedIds });
await emitRemoveWidgetsToCanvas({ ids });
}

const newManagerWidgetStates = Object.fromEntries(
Expand Down
8 changes: 6 additions & 2 deletions src/types/frontend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* without corresponding backend implementations.
*/

import { WidgetConfig, WidgetSettings } from "./backend";
import { Theme, WidgetConfig, WidgetSettings } from "./backend";

export type DeepReadonly<T> = {
readonly [P in keyof T]: T[P] extends object ? DeepReadonly<T[P]> : T[P];
Expand Down Expand Up @@ -33,7 +33,7 @@ export type RenderWidgetsPayload = {
*/
export interface RemoveWidgetsPayload {
/** The widget IDs to remove. */
removedIds: string[];
ids: string[];
}

/**
Expand All @@ -45,3 +45,7 @@ export interface UpdateSettingsPayload {
/** The widget-specific settings to update. */
settings: Partial<WidgetSettings>;
}

export interface SwitchThemePayload {
theme: Theme;
}

0 comments on commit dc49f26

Please sign in to comment.