From c5889f916adc2acdae4e7ab1bf72179ab3ed8aef Mon Sep 17 00:00:00 2001 From: Michael Gartner Date: Thu, 28 Aug 2025 19:12:30 -0600 Subject: [PATCH 1/2] Refactor LabelDialogAutocomplete to use async/await for fetching options - Replaced promise chaining with async/await and try/catch for improved error handling. - Simplified the fetching logic for main and referenced node options. - Updated useEffect dependencies for better clarity and performance. - Changed referencedNode retrieval to use useMemo for optimization. --- .../src/components/canvas/LabelDialog.tsx | 96 +++++++++---------- 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/apps/roam/src/components/canvas/LabelDialog.tsx b/apps/roam/src/components/canvas/LabelDialog.tsx index 8881fb7a4..822eb2b0e 100644 --- a/apps/roam/src/components/canvas/LabelDialog.tsx +++ b/apps/roam/src/components/canvas/LabelDialog.tsx @@ -61,57 +61,58 @@ const LabelDialogAutocomplete = ({ const [isAddReferencedNode, setAddReferencedNode] = useState(false); const [isEditExistingLabel, setIsEditExistingLabel] = useState(false); const [content, setContent] = useState(label); - useEffect(() => { - setIsLoading(true); - const conditionUid = window.roamAlphaAPI.util.generateUID(); + const fetchOptions = async () => { + setIsLoading(true); - setTimeout(() => { - if (nodeType) { - void fireQuery({ - returnNode: "node", - selections: [], - conditions: [ - { - source: "node", - relation: "is a", - target: nodeType, - uid: conditionUid, - type: "clause", - }, - ], - }).then((results) => { + try { + // Fetch main options + if (nodeType) { + const conditionUid = window.roamAlphaAPI.util.generateUID(); + const results = await fireQuery({ + returnNode: "node", + selections: [], + conditions: [ + { + source: "node", + relation: "is a", + target: nodeType, + uid: conditionUid, + type: "clause", + }, + ], + }); setOptions(results); - }); - } - if (referencedNode) { - void fireQuery({ - returnNode: "node", - selections: [], - conditions: [ - { - source: "node", - relation: "is a", - target: referencedNode.nodeType, - uid: conditionUid, - type: "clause", - }, - ], - }).then((results) => { + } + + // Fetch referenced node options if needed + if (isAddReferencedNode && referencedNode) { + const conditionUid = window.roamAlphaAPI.util.generateUID(); + const results = await fireQuery({ + returnNode: "node", + selections: [], + conditions: [ + { + source: "node", + relation: "is a", + target: referencedNode.nodeType, + uid: conditionUid, + type: "clause", + }, + ], + }); setReferencedNodeOptions(results); - setIsLoading(false); - }); - } else { + } + } catch (error) { + console.error("Error fetching options:", error); + } finally { setIsLoading(false); } - }, 100); - }, [ - nodeType, - referencedNode?.nodeType, - setOptions, - setReferencedNodeOptions, - referencedNode, - ]); + }; + + void fetchOptions(); + }, [nodeType, isAddReferencedNode, referencedNode]); + const inputDivRef = useRef(null); useEffect(() => { if (isAddReferencedNode && inputDivRef.current) { @@ -324,7 +325,7 @@ const LabelDialog = ({ const [loading, setLoading] = useState(false); const isCreateCanvasNode = !isLiveBlock(initialUid); const { format } = discourseContext.nodes[nodeType]; - const getReferencedNodeInFormat = () => { + const referencedNode = useMemo(() => { const regex = /{([\w\d-]*)}/g; const matches = [...format.matchAll(regex)]; @@ -342,8 +343,7 @@ const LabelDialog = ({ } return null; - }; - const referencedNode = getReferencedNodeInFormat(); + }, [format, discourseContext.nodes]); const renderCalloutText = () => { let title = "Please provide a label"; From 7d39d91327f1408d5be0e66beb3eebd1ca5ee045 Mon Sep 17 00:00:00 2001 From: Michael Gartner Date: Thu, 28 Aug 2025 20:40:03 -0600 Subject: [PATCH 2/2] Enhance LabelDialogAutocomplete with request ID tracking and cleanup - Introduced request ID tracking to prevent stale state updates during asynchronous fetching. - Added cleanup logic in useEffect to manage component lifecycle and avoid memory leaks. - Improved error handling to ensure console errors are logged only for the latest request. - Maintained existing async/await structure for fetching options, enhancing clarity and performance. --- .../src/components/canvas/LabelDialog.tsx | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/apps/roam/src/components/canvas/LabelDialog.tsx b/apps/roam/src/components/canvas/LabelDialog.tsx index 822eb2b0e..4e4173293 100644 --- a/apps/roam/src/components/canvas/LabelDialog.tsx +++ b/apps/roam/src/components/canvas/LabelDialog.tsx @@ -52,6 +52,7 @@ const LabelDialogAutocomplete = ({ format: string; label: string; }) => { + const requestIdRef = useRef(0); const [isLoading, setIsLoading] = useState(false); const [options, setOptions] = useState([]); const [referencedNodeOptions, setReferencedNodeOptions] = useState( @@ -62,9 +63,10 @@ const LabelDialogAutocomplete = ({ const [isEditExistingLabel, setIsEditExistingLabel] = useState(false); const [content, setContent] = useState(label); useEffect(() => { + let alive = true; + const req = ++requestIdRef.current; + setIsLoading(true); const fetchOptions = async () => { - setIsLoading(true); - try { // Fetch main options if (nodeType) { @@ -82,7 +84,7 @@ const LabelDialogAutocomplete = ({ }, ], }); - setOptions(results); + if (requestIdRef.current === req && alive) setOptions(results); } // Fetch referenced node options if needed @@ -101,16 +103,23 @@ const LabelDialogAutocomplete = ({ }, ], }); - setReferencedNodeOptions(results); + if (requestIdRef.current === req && alive) { + setReferencedNodeOptions(results); + } } } catch (error) { - console.error("Error fetching options:", error); + if (requestIdRef.current === req && alive) { + console.error("Error fetching options:", error); + } } finally { - setIsLoading(false); + if (requestIdRef.current === req && alive) setIsLoading(false); } }; void fetchOptions(); + return () => { + alive = false; + }; }, [nodeType, isAddReferencedNode, referencedNode]); const inputDivRef = useRef(null);