refactor(react): fix a few rules of hooks violations

2025-11-12 16:25:51 +01:00 · 2025-08-25 18:00:10 +03:00
parent e386b03b90
commit 733ec2c145
9 changed files with 125 additions and 162 deletions
--- a/apps/client/src/widgets/dialogs/call_to_action.tsx
+++ b/apps/client/src/widgets/dialogs/call_to_action.tsx
@@ -6,11 +6,6 @@ import { t } from "../../services/i18n";

 export default function CallToActionDialog() {
    const activeCallToActions = useMemo(() => getCallToActions(), []);        
-    
-    if (!activeCallToActions.length) {
-        return <></>;
-    }
-    
    const [ activeIndex, setActiveIndex ] = useState(0);
    const [ shown, setShown ] = useState(true);
    const activeItem = activeCallToActions[activeIndex];
@@ -23,7 +18,7 @@ export default function CallToActionDialog() {
        }
    }

-    return (
+    return (activeCallToActions.length &&
        <Modal
            className="call-to-action"
            size="md"
--- a/apps/client/src/widgets/dialogs/note_type_chooser.tsx
+++ b/apps/client/src/widgets/dialogs/note_type_chooser.tsx
@@ -36,7 +36,6 @@ export default function NoteTypeChooserDialogComponent() {
        setShown(true);
    });

-    if (!noteTypes.length) {
    useEffect(() => {
        note_types.getNoteTypeItems().then(noteTypes => {
            let index = -1;
@@ -53,8 +52,7 @@ export default function NoteTypeChooserDialogComponent() {
                return item;
            }));
        });
-        });
-    }
+    }, []);

    function onNoteTypeSelected(value: string) {
        const [ noteType, templateNoteId ] = value.split(",");
--- a/apps/client/src/widgets/dialogs/recent_changes.tsx
+++ b/apps/client/src/widgets/dialogs/recent_changes.tsx
@@ -18,21 +18,15 @@ import { useTriliumEvent } from "../react/hooks";
 export default function RecentChangesDialog() {
    const [ ancestorNoteId, setAncestorNoteId ] = useState<string>();
    const [ groupedByDate, setGroupedByDate ] = useState<Map<string, RecentChangeRow[]>>();
-    const [ needsRefresh, setNeedsRefresh ] = useState(false);
+    const [ refreshCounter, setRefreshCounter ] = useState(0);
    const [ shown, setShown ] = useState(false);

    useTriliumEvent("showRecentChanges", ({ ancestorNoteId }) => {        
-        setNeedsRefresh(true);
        setAncestorNoteId(ancestorNoteId ?? hoisted_note.getHoistedNoteId());
        setShown(true);
    });

-    if (!groupedByDate || needsRefresh) {
    useEffect(() => {
-            if (needsRefresh) {
-                setNeedsRefresh(false);   
-            }
-
        server.get<RecentChangeRow[]>(`recent-changes/${ancestorNoteId}`)
            .then(async (recentChanges) => {
                // preload all notes into cache
@@ -44,8 +38,7 @@ export default function RecentChangesDialog() {
                const groupedByDate = groupByDate(recentChanges);
                setGroupedByDate(groupedByDate);
            });
-        })
-    }
+    }, [ shown, refreshCounter ])

    return (
        <Modal
@@ -60,7 +53,7 @@ export default function RecentChangesDialog() {
                    style={{ padding: "0 10px" }}
                    onClick={() => {
                        server.post("notes/erase-deleted-notes-now").then(() => {
-                            setNeedsRefresh(true);
+                            setRefreshCounter(refreshCounter + 1);
                            toast.showMessage(t("recent_changes.deleted_notes_message"));
                        });
                    }}
@@ -113,10 +106,6 @@ function RecentChangesTimeline({ groupedByDate, setShown }: { groupedByDate: Map
 }

 function NoteLink({ notePath, title }: { notePath: string, title: string }) {
-    if (!notePath || !title) {
-        return null;
-    }
-
    const [ noteLink, setNoteLink ] = useState<JQuery<HTMLElement> | null>(null);
    useEffect(() => {
        link.createLink(notePath, {
--- a/apps/client/src/widgets/dialogs/revisions.tsx
+++ b/apps/client/src/widgets/dialogs/revisions.tsx
@@ -201,17 +201,9 @@ function RevisionContent({ revisionItem, fullRevision }: { revisionItem?: Revisi
        return <></>;
    }

-
    switch (revisionItem.type) {
-        case "text": {
-            const contentRef = useRef<HTMLDivElement>(null);
-            useEffect(() => {
-                if (contentRef.current?.querySelector("span.math-tex")) {
-                    renderMathInElement(contentRef.current, { trust: true });
-                }
-            });
-            return <div ref={contentRef} className="ck-content" dangerouslySetInnerHTML={{ __html: content as string }}></div>
-        }
+        case "text":
+            return <RevisionContentText content={content} />
        case "code":
            return <pre style={CODE_STYLE}>{content}</pre>;
        case "image":            
@@ -263,6 +255,16 @@ function RevisionContent({ revisionItem, fullRevision }: { revisionItem?: Revisi
    }
 }

+function RevisionContentText({ content }: { content: string | Buffer<ArrayBufferLike> | undefined }) {
+    const contentRef = useRef<HTMLDivElement>(null);
+    useEffect(() => {
+        if (contentRef.current?.querySelector("span.math-tex")) {
+            renderMathInElement(contentRef.current, { trust: true });
+        }
+    }, [content]);
+    return <div ref={contentRef} className="ck-content" dangerouslySetInnerHTML={{ __html: content as string }}></div>
+}
+
 function RevisionFooter({ note }: { note?: FNote }) {
    if (!note) {
        return <></>;
--- a/apps/client/src/widgets/dialogs/upload_attachments.tsx
+++ b/apps/client/src/widgets/dialogs/upload_attachments.tsx
@@ -23,12 +23,12 @@ export default function UploadAttachmentsDialog() {
        setShown(true);
    });

-    if (parentNoteId) {
    useEffect(() => {
+        if (!parentNoteId) return;
+
        tree.getNoteTitle(parentNoteId).then((noteTitle) =>
            setDescription(t("upload_attachments.files_will_be_uploaded", { noteTitle })));
    }, [parentNoteId]);
-    }

    return (
        <Modal
--- a/apps/client/src/widgets/react/Modal.tsx
+++ b/apps/client/src/widgets/react/Modal.tsx
@@ -71,7 +71,6 @@ export default function Modal({ children, className, size, title, header, footer
    const parentWidget = useContext(ParentComponent);
    const elementToFocus = useRef<Element | null>();

-    if (onShown || onHidden) {
    useEffect(() => {
        const modalElement = modalRef.current;
        if (!modalElement) {
@@ -92,8 +91,7 @@ export default function Modal({ children, className, size, title, header, footer
            }
            modalElement.removeEventListener("hidden.bs.modal", onHidden);
        };
-        }, [ ]);
-    }    
+    }, [ onShown, onHidden ]);

    useEffect(() => {
        if (!parentWidget) {
--- a/apps/client/src/widgets/react/hooks.tsx
+++ b/apps/client/src/widgets/react/hooks.tsx
@@ -216,15 +216,13 @@ export function useNoteContext() {
        setNote(noteContext?.note);
    });

-    useLegacyImperativeHandlers({
-        setNoteContextEvent({ noteContext }: EventData<"setNoteContext">) {
+    const parentComponent = useContext(ParentComponent) as ReactWrappedWidget;
+    (parentComponent as ReactWrappedWidget & { setNoteContextEvent: (data: EventData<"setNoteContext">) => void }).setNoteContextEvent = ({ noteContext }: EventData<"setNoteContext">) => {
        setNoteContext(noteContext);
    }
-    }, true);

    useDebugValue(() => `notePath=${notePath}, ntxId=${noteContext?.ntxId}`);
    
-    const parentComponent = useContext(ParentComponent) as ReactWrappedWidget;

    return {
        note: note,
@@ -249,25 +247,21 @@ export function useNoteContext() {
 * @returns the value of the requested property.
 */
 export function useNoteProperty<T extends keyof FNote>(note: FNote | null | undefined, property: T, componentId?: string) {
-    if (!note) {
-        return null;
-    }
-
-    const [, setValue ] = useState<FNote[T]>(note[property]);
-    const refreshValue = () => setValue(note[property]);
+    const [, setValue ] = useState<FNote[T] | undefined>(note?.[property]);
+    const refreshValue = () => setValue(note?.[property]);

    // Watch for note changes.
-    useEffect(() => refreshValue(), [ note, note[property] ]);
+    useEffect(() => refreshValue(), [ note, note?.[property] ]);

    // Watch for external changes.
    useTriliumEvent("entitiesReloaded", ({ loadResults }) => {
-        if (loadResults.isNoteReloaded(note.noteId, componentId)) {
+        if (loadResults.isNoteReloaded(note?.noteId, componentId)) {
            refreshValue();
        }
    });

    useDebugValue(property);
-    return note[property];
+    return note?.[property];
 }

 export function useNoteRelation(note: FNote | undefined | null, relationName: string): [string | null | undefined, (newValue: string) => void] {
@@ -362,10 +356,6 @@ export function useNoteLabelBoolean(note: FNote | undefined | null, labelName: s
 }

 export function useNoteBlob(note: FNote | null | undefined): [ FBlob | null | undefined ] {
-    if (!note) {
-        return [ undefined ];
-    }
-
    const [ blob, setBlob ] = useState<FBlob | null>();
    
    function refresh() {
@@ -379,7 +369,7 @@ export function useNoteBlob(note: FNote | null | undefined): [ FBlob | null | un
        }
    });

-    useDebugValue(note.noteId);
+    useDebugValue(note?.noteId);

    return [ blob ] as const;
 }
@@ -514,13 +504,9 @@ export function useTooltip(elRef: RefObject<HTMLElement>, config: Partial<Toolti
 }

 // eslint-disable-next-line @typescript-eslint/no-unsafe-function-type
-export function useLegacyImperativeHandlers(handlers: Record<string, Function>, force?: boolean) {
+export function useLegacyImperativeHandlers(handlers: Record<string, Function>) {
    const parentComponent = useContext(ParentComponent);
-    if (!force) {
    useEffect(() => {
        Object.assign(parentComponent as never, handlers);
-        }, [ handlers ])
-    } else {
-        Object.assign(parentComponent as never, handlers);
-    }
+    }, [ handlers ]);
 }
--- a/apps/client/src/widgets/ribbon/NoteActions.tsx
+++ b/apps/client/src/widgets/ribbon/NoteActions.tsx
@@ -20,16 +20,16 @@ interface NoteActionsProps {
  noteContext?: NoteContext;
 }

-export default function NoteActions(props: NoteActionsProps) {
+export default function NoteActions({ note, noteContext }: NoteActionsProps) {
  return (
    <>
-      <RevisionsButton {...props} />
-      <NoteContextMenu {...props} />
+      {note && <RevisionsButton note={note} />}
+      {note && note.type !== "launcher" && <NoteContextMenu note={note as FNote} noteContext={noteContext}/>}
    </>
  );
 }

-function RevisionsButton({ note }: NoteActionsProps) {
+function RevisionsButton({ note }: { note: FNote }) {
  const isEnabled = !["launcher", "doc"].includes(note?.type ?? "");

  return (isEnabled &&
@@ -42,12 +42,7 @@ function RevisionsButton({ note }: NoteActionsProps) {
  );
 }

-function NoteContextMenu(props: NoteActionsProps) {
-  const { note, noteContext } = props;
-  if (!note || note.type === "launcher") {
-    return <></>;
-  }
-
+function NoteContextMenu({ note, noteContext }: { note: FNote, noteContext?: NoteContext }) {
  const parentComponent = useContext(ParentComponent);
  const canBeConvertedToAttachment = note?.isEligibleForConversionToAttachment();
  const isSearchable = ["text", "code", "book", "mindMap", "doc"].includes(note.type);
@@ -65,7 +60,7 @@ function NoteContextMenu(props: NoteActionsProps) {
      hideToggleArrow
      noSelectButtonStyle
    >
-      {canBeConvertedToAttachment && <ConvertToAttachment {...props} /> }
+      {canBeConvertedToAttachment && <ConvertToAttachment note={note} /> }
      {note.type === "render" && <CommandItem command="renderActiveNote" icon="bx bx-extension" text={t("note_actions.re_render_note")} />}
      <CommandItem command="findInText" icon="bx bx-search" disabled={!isSearchable} text={t("note_actions.search_in_note")} />
      <CommandItem command="printActiveNote" icon="bx bx-printer" disabled={!isPrintable} text={t("note_actions.print_note")} />
@@ -110,7 +105,7 @@ function CommandItem({ icon, text, title, command, disabled }: { icon: string, t
  >{text}</FormListItem>
 }

-function ConvertToAttachment({ note }: NoteActionsProps) {
+function ConvertToAttachment({ note }: { note: FNote }) {
  return (
    <FormListItem
        icon="bx bx-paperclip"
--- a/apps/client/src/widgets/type_widgets/options/etapi.tsx
+++ b/apps/client/src/widgets/type_widgets/options/etapi.tsx
@@ -77,10 +77,6 @@ export default function EtapiSettings() {
 }

 function TokenList({ tokens }: { tokens: EtapiToken[] }) {
-    if (!tokens.length) {
-        return <div>{t("etapi.no_tokens_yet")}</div>;
-    }    
-    
    const renameCallback = useCallback<RenameTokenCallback>(async (tokenId: string, oldName: string) => {
        const tokenName = await dialog.prompt({
            title: t("etapi.rename_token_title"),
@@ -104,6 +100,7 @@ function TokenList({ tokens }: { tokens: EtapiToken[] }) {
    }, []);

    return (
+        tokens.length ? (
            <div style={{ overflow: "auto", height: "500px"}}>
                <table className="table table-stripped">
                    <thead>
@@ -140,5 +137,8 @@ function TokenList({ tokens }: { tokens: EtapiToken[] }) {
                    </tbody>
                </table>
            </div>
+        ) : (
+            <div>{t("etapi.no_tokens_yet")}</div>
        )
+    );
 }