Migrate to async #40
Migrate to async #40
Conversation
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThis pull request updates the package version and refactors multiple aspects of the codebase. Numerous components, DOM utilities, query functions, and utility methods have been converted from synchronous to asynchronous implementations using async/await. New useEffect hooks and state management patterns have been introduced into components, while many query and DOM functions now return Promises. Additionally, type declarations and the mock environment were adjusted to support promise-based APIs. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Component
participant U as Utility/Query
participant API as Roam API
C->>U: Trigger async data retrieval
U->>API: Await asynchronous API call
API-->>U: Return fetched data
U->>C: Update state with received result
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
src/util/addInputSetting.ts (1)
18-31: 💡 Verification agent🧩 Analysis chain
Missing await for createBlock calls
The function is now async but doesn't await the
createBlockcalls. IfcreateBlockhas been migrated to return a Promise as part of this PR, you should add await to these calls to ensure proper async execution.
🏁 Script executed:
#!/bin/bash # Check if createBlock is async grep -A 10 "const createBlock" src/writes/createBlock.tsLength of output: 355
Action: Add missing await for createBlock calls
The verification confirms that
createBlockreturns a Promise. Since the function using it is async, both calls should useawaitto ensure proper asynchronous execution.For example, update the code in
src/util/addInputSetting.ts(lines 18-31) as follows:
- Change:
if (keyNode) { return createBlock({ node: { text: value }, order: keyNode.children.length, parentUid: keyNode.uid, }); } else { return createBlock({ parentUid: blockUid, order: index, node: { text: key, children: [{ text: value }] }, }); }- To:
if (keyNode) { return await createBlock({ node: { text: value }, order: keyNode.children.length, parentUid: keyNode.uid, }); } else { return await createBlock({ parentUid: blockUid, order: index, node: { text: key, children: [{ text: value }] }, }); }This change will ensure that the Promise is properly resolved before proceeding.
src/util/setInputSettings.ts (1)
17-35: 💡 Verification agent🧩 Analysis chain
Verify if additional async operations need await.
The await is correctly added for
getBasicTreeByParentUid, but the function also callsdeleteBlockandcreateBlock. If these functions have also been updated to be async elsewhere in the PR, they should be awaited here as well.
🏁 Script executed:
#!/bin/bash # Check if deleteBlock and createBlock have been updated to return promises echo "Checking deleteBlock implementation..." rg -A 3 "const deleteBlock" --type ts src/writes/ echo "Checking createBlock implementation..." rg -A 3 "const createBlock" --type ts src/writes/Length of output: 778
Async Operations Need Proper Awaiting
The
deleteBlockandcreateBlockfunctions now clearly return promises (i.e.Promise<string>), so their invocations should be awaited to ensure proper async handling. In the current implementation insrc/util/setInputSettings.ts(lines 17–35), consider updating the code as follows:
- Use a loop (or
Promise.all) when invokingdeleteBlockon filtered children to guarantee that each asynchronous deletion completes (e.g., replace.forEach(() => deleteBlock(uid))with an approach that usesawait).- Similarly, await the promise returned by
createBlockwhen creating a new block.These adjustments will help manage potential errors from these asynchronous operations and ensure that the intended order of execution is maintained.
🧹 Nitpick comments (28)
src/components/ConfigPanels/MultiChildPanel.tsx (1)
25-36: Good async implementation with useEffect.The component has been properly refactored to handle async data loading using React's useEffect hook. The implementation correctly:
- Initializes state with an empty array
- Loads data asynchronously when the component mounts or uid changes
- Properly awaits the async call and updates state
Consider adding a loading state to improve user experience during data fetching.
const [uid, setUid] = useState(initialUid); const [texts, setTexts] = useState<{ text: string; uid: string }[]>([]); +const [loading, setLoading] = useState(false); const [value, setValue] = useState(""); useEffect(() => { const loadTexts = async () => { if (uid) { + setLoading(true); const items = await getShallowTreeByParentUid(uid); setTexts(items); + setLoading(false); } }; loadTexts(); }, [uid]);src/util/setInputSetting.ts (1)
6-44: Consider adding error handling for async operationsWhile the migration to async is correct, there's no error handling for the asynchronous operation. Consider adding try/catch blocks to gracefully handle potential failures when retrieving the tree data.
const setInputSetting = async ({ blockUid, value, key, index = 0, }: { blockUid: string; value: string; key: string; index?: number; }): Promise<string> => { + try { const tree = await getBasicTreeByParentUid(blockUid); const keyNode = tree.find((t) => toFlexRegex(key).test(t.text)); if (keyNode && keyNode.children.length) { return updateBlock({ uid: keyNode.children[0].uid, text: value, }); } else if (!keyNode) { const uid = window.roamAlphaAPI.util.generateUID(); return createBlock({ parentUid: blockUid, order: index, node: { text: key, uid }, }).then(() => createBlock({ parentUid: uid, order: 0, node: { text: value }, }) ); } else { return createBlock({ parentUid: keyNode.uid, order: 0, node: { text: value }, }); } + } catch (error) { + console.error("Error in setInputSetting:", error); + throw error; + } };🧰 Tools
🪛 Biome (1.9.4)
[error] 19-19: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/dom/getBlockUidFromTarget.ts (1)
5-37: Consider consistent return pattern for async functionThere's an inconsistency in how values are returned. Some paths return immediately (lines 8, 13, etc.) while the kanban path awaits an async operation. Since the function is now async, consider making all return paths consistent in how they handle promises.
const getBlockUidFromTarget = async (target: HTMLElement): Promise<string> => { const ref = target.closest(".rm-block-ref") as HTMLSpanElement; if (ref) { - return ref.getAttribute("data-uid") || ""; + return Promise.resolve(ref.getAttribute("data-uid") || ""); } const customView = target.closest(".roamjs-block-view") as HTMLDivElement; if (customView) { - return getUids(customView).blockUid; + return Promise.resolve(getUids(customView).blockUid); } const aliasTooltip = target.closest(".rm-alias-tooltip__content"); if (aliasTooltip) { const aliasRef = document.querySelector( ".bp3-popover-open .rm-alias--block" ) as HTMLAnchorElement; - return getReferenceBlockUid(aliasRef, "rm-alias--block"); + return Promise.resolve(getReferenceBlockUid(aliasRef, "rm-alias--block")); } const { blockUid } = getUids(target.closest(".roam-block") as HTMLDivElement); const kanbanTitle = target.closest(".kanban-title"); if (kanbanTitle) { const container = kanbanTitle.closest(".kanban-column-container"); if (container) { const column = kanbanTitle.closest(".kanban-column"); const order = Array.from(container.children).findIndex( (d) => d === column ); return await getNthChildUidByBlockUid({ blockUid, order }); } } - return blockUid; + return Promise.resolve(blockUid); };Alternatively, you could remove the unnecessary
awaitand just return the promise:const order = Array.from(container.children).findIndex( (d) => d === column ); - return await getNthChildUidByBlockUid({ blockUid, order }); + return getNthChildUidByBlockUid({ blockUid, order });src/queries/getAllBlockUidsAndTexts.ts (1)
1-8: Consider adding error handling for API callsThe function doesn't include error handling for the asynchronous API call. Consider adding a try/catch block to handle potential failures gracefully.
const getAllBlockUidsAndTexts = async (): Promise< { uid: string; text: string }[] > => { + try { const result = await window.roamAlphaAPI.data.backend.q( `[:find ?u ?s :where [?e :block/uid ?u] [?e :block/string ?s]]` ); return result.map((f) => ({ uid: f[0] as string, text: f[1] as string })); + } catch (error) { + console.error("Error fetching block UIDs and texts:", error); + throw error; + } };src/queries/getBlockUidsByPageTitle.ts (1)
3-10: Consider adding error handling for the query operationThe function doesn't include error handling for the asynchronous API call. Consider adding a try/catch block to handle potential query failures gracefully.
const getBlockUidsByPageTitle = async (title: string): Promise<string[]> => { + try { const result = await window.roamAlphaAPI.data.backend.q( `[:find ?u :where [?b :block/uid ?u] [?b :block/page ?e] [?e :node/title "${normalizePageTitle( title )}"]]` ); return result.map((b) => b[0] as string); + } catch (error) { + console.error(`Error fetching block UIDs for page "${title}":`, error); + throw error; + } };src/queries/getCreateTimeByBlockUid.ts (1)
6-6: Consider more robust error handling than fallback to 0The fallback value of
0when the create time doesn't exist could hide issues in code that expects valid timestamps.Consider throwing an error when the block doesn't exist or using a more distinctive fallback value that won't be confused with a valid timestamp:
- return result?.[":create/time"] || 0; + if (!result) { + console.warn(`No block found with UID: ${uid}`); + } + return result?.[":create/time"] || 0;src/queries/getBlockUidsReferencingPage.ts (1)
7-11: Consider adding error handlingThe function currently doesn't handle potential errors from the API call. Consider adding a try/catch block to handle failures gracefully.
const getBlockUidsReferencingPage = async ( title: string ): Promise<string[]> => { + try { const result = (await window.roamAlphaAPI.data.backend.q( `[:find (pull ?r [:block/uid]) :where [?p :node/title "${normalizePageTitle( title )}"] [?r :block/refs ?p]]` )) as [PullBlock][]; return result.map((s) => s[0][":block/uid"] || ""); + } catch (error) { + console.error(`Failed to get blocks referencing page "${title}":`, error); + return []; + } };src/queries/getEditTimeByBlockUid.ts (1)
1-7: Consider adding error handlingThe function should handle potential errors from the API call to prevent unhandled promise rejections.
const getEditTimeByBlockUid = async (uid: string): Promise<number> => { + try { const result = await window.roamAlphaAPI.data.pull(`[:edit/time]`, [ ":block/uid", uid, ]); return result?.[":edit/time"] || 0; + } catch (error) { + console.error(`Failed to get edit time for block "${uid}":`, error); + return 0; + } };src/queries/getDisplayNameByEmail.ts (1)
1-6: Add error handling for robustnessThe function should handle potential errors from the API call to prevent unhandled promise rejections.
const getDisplayNameByEmail = async (email: string): Promise<string> => { + try { const result = await window.roamAlphaAPI.data.backend.q( `[:find ?name :where[?e :user/display-name ?name] [?e :user/email "${email}"]]` ); return (result?.[0]?.[0] as string) || ""; + } catch (error) { + console.error(`Failed to get display name for email "${email}":`, error); + return ""; + } };src/queries/getChildrenLengthByPageUid.ts (1)
2-8: Add error handling for robustnessThe function should include error handling to gracefully manage potential failures from the API call.
// @deprecated - use getChildrenLengthByParentUid instead const getChildrenLengthByPageUid = async (uid: string): Promise<number> => { + try { const result = await window.roamAlphaAPI.data.pull("[:block/children]", [ ":block/uid", uid, ]); return result?.[":block/children"]?.length || 0; + } catch (error) { + console.error(`Failed to get children length for page "${uid}":`, error); + return 0; + } };src/queries/getBlockUidsReferencingBlock.ts (1)
3-8: LGTM: Properly migrated to async implementation.The function has been correctly converted to an asynchronous implementation with proper Promise typing. The switch from
window.roamAlphaAPI.data.fast.qtowindow.roamAlphaAPI.data.backend.qis consistent with the PR objective to migrate to async functions.Consider adding error handling for potential API failures:
const getBlockUidsReferencingBlock = async (uid: string): Promise<string[]> => { - const result = (await window.roamAlphaAPI.data.backend.q( - `[:find (pull ?r [:block/uid]) :where [?b :block/uid "${uid}"] [?r :block/refs ?b]]` - )) as [PullBlock][]; - return result.map((s) => s[0][":block/uid"] || ""); + try { + const result = (await window.roamAlphaAPI.data.backend.q( + `[:find (pull ?r [:block/uid]) :where [?b :block/uid "${uid}"] [?r :block/refs ?b]]` + )) as [PullBlock][]; + return result.map((s) => s[0][":block/uid"] || ""); + } catch (error) { + console.error(`Error fetching block UIDs referencing ${uid}:`, error); + return []; + } };src/queries/getParentUidsOfBlockUid.ts (1)
3-8: LGTM: Proper async migration.The function has been successfully converted to an asynchronous implementation with correct Promise typing and await syntax. The migration pattern is consistent with other query function changes in the PR.
Consider adding error handling to gracefully handle API failures:
const getParentUidsOfBlockUid = async (uid: string): Promise<string[]> => { - const result = (await window.roamAlphaAPI.data.backend.q( - `[:find (pull ?p [:block/uid]) :where [?b :block/uid "${uid}"] [?b :block/parents ?p] ]` - )) as [PullBlock][]; - return result.map((r) => r[0][":block/uid"] || ""); + try { + const result = (await window.roamAlphaAPI.data.backend.q( + `[:find (pull ?p [:block/uid]) :where [?b :block/uid "${uid}"] [?b :block/parents ?p] ]` + )) as [PullBlock][]; + return result.map((r) => r[0][":block/uid"] || ""); + } catch (error) { + console.error(`Error fetching parent UIDs of block ${uid}:`, error); + return []; + } };src/queries/getParentUidByBlockUid.ts (1)
3-8: LGTM: Successfully migrated to async pattern.The function has been properly converted to an asynchronous implementation with correct Promise typing. The use of optional chaining and default empty string return is a good defensive programming practice.
Consider adding error handling to make the function more robust:
const getParentUidByBlockUid = async (blockUid: string): Promise<string> => { - const result = await window.roamAlphaAPI.data.backend.q( - `[:find (pull ?p [:block/uid]) :where [?e :block/uid "${blockUid}"] [?p :block/children ?e]]` - ); - return (result?.[0]?.[0] as PullBlock)?.[":block/uid"] || ""; + try { + const result = await window.roamAlphaAPI.data.backend.q( + `[:find (pull ?p [:block/uid]) :where [?e :block/uid "${blockUid}"] [?p :block/children ?e]]` + ); + return (result?.[0]?.[0] as PullBlock)?.[":block/uid"] || ""; + } catch (error) { + console.error(`Error fetching parent UID of block ${blockUid}:`, error); + return ""; + } };src/queries/getParentTextByBlockUid.ts (1)
1-6: LGTM: Clean async migration.The function has been successfully converted to an asynchronous implementation with proper Promise typing and API usage. The use of optional chaining and default empty string return provides good fallback behavior.
Consider adding error handling for robustness:
const getParentTextByBlockUid = async (blockUid: string): Promise<string> => { - const result = await window.roamAlphaAPI.data.backend.q( - `[:find ?s :where [?e :block/uid "${blockUid}"] [?p :block/children ?e] [?p :block/string ?s]]` - ); - return (result?.[0]?.[0] as string) || ""; + try { + const result = await window.roamAlphaAPI.data.backend.q( + `[:find ?s :where [?e :block/uid "${blockUid}"] [?p :block/children ?e] [?p :block/string ?s]]` + ); + return (result?.[0]?.[0] as string) || ""; + } catch (error) { + console.error(`Error fetching parent text of block ${blockUid}:`, error); + return ""; + } };src/dom/addKeyboardTriggers.ts (1)
4-38: Good implementation of async/await pattern for keyboard triggersThe function has been properly migrated to use async/await, with
Promise.alleffectively handling concurrent processing of multiple triggers. The asynchronous handling ofgetSettingValueFromTreeallows the function to properly wait for settings to be retrieved.I noticed a typo in the function name:
addKeybordTriggersshould beaddKeyboardTriggers.-const addKeybordTriggers = async ({ +const addKeyboardTriggers = async ({Also, you would need to update the export at the bottom of the file:
-export default addKeybordTriggers; +export default addKeyboardTriggers;src/dom/addBlockCommand.ts (1)
54-54: Consider using async/await consistently instead of mixing with .then()While the current implementation works correctly, it mixes async/await with promise chaining. For better code consistency, consider using async/await throughout.
- callback: () => { - const parentUid = getCurrentPageUid(); - loadBlockUid(parentUid).then(callback); - }, + callback: async () => { + const parentUid = getCurrentPageUid(); + const uid = await loadBlockUid(parentUid); + callback(uid); + },src/dom/createPageObserver.ts (1)
30-37: Consider updating to async/await patternWhile the code correctly handles the now-asynchronous
getBlockUidsByPageTitle, it uses.then()instead of theasync/awaitpattern used elsewhere in the codebase. Consider refactoring for consistency.- getBlockUidsByPageTitle(name).then((blockUids) => { - const blockUidSet = new Set(blockUids); - [...removedNodes, ...addedNodes] - .filter(({ blockUid }) => blockUidSet.has(blockUid)) - .forEach(({ blockUid, added }) => callback(blockUid, added)); - }); + (async () => { + const blockUids = await getBlockUidsByPageTitle(name); + const blockUidSet = new Set(blockUids); + [...removedNodes, ...addedNodes] + .filter(({ blockUid }) => blockUidSet.has(blockUid)) + .forEach(({ blockUid, added }) => callback(blockUid, added)); + })();Any code that relies on this observer should be checked to ensure it doesn't depend on synchronous callback execution, as callbacks are now executed asynchronously after the Promise resolves.
src/components/PageInput.tsx (1)
15-25: Consider adding loading state feedbackSince page names are loaded asynchronously, users might initially see an empty or limited set of options. Consider adding a loading indicator or placeholder to improve user experience.
const PageInput = ({ extra = DEFAULT_EXTRA, ...rest }: { extra?: string[]; } & Omit<AutocompleteInputProps, "options">): React.ReactElement => { const [pageNames, setPageNames] = useState<string[]>([]); + const [isLoading, setIsLoading] = useState(true); useEffect(() => { const loadPageNames = async () => { try { + setIsLoading(true); const names = await getAllPageNames(); setPageNames(names); } catch (error) { console.error("Failed to load page names:", error); + } finally { + setIsLoading(false); } }; loadPageNames(); }, []); const options = useMemo(() => [...pageNames, ...extra], [pageNames, extra]); - return <AutocompleteInput {...rest} options={options} />; + return <AutocompleteInput + {...rest} + options={options} + placeholder={isLoading ? "Loading page names..." : rest.placeholder} + />;src/components/ConfigPanels/BlocksPanel.tsx (1)
31-49: Code structure could be improved for readability.While the promise chain has been correctly updated to handle the async nature of
getFirstChildUidByBlockUid, the indentation and nesting make the code harder to follow. Consider refactoring to improve readability.- getFirstChildUidByBlockUid(formatUid).then((childUid) => - childUid - ? formatUid - : (defaultValue?.length - ? Promise.all( - defaultValue.map((node, order) => - createBlock({ - node, - parentUid: formatUid, - order, - }) - ) - ) - : createBlock({ - node: { text: " " }, - parentUid: formatUid, - }) - ).then(() => formatUid) - ) + getFirstChildUidByBlockUid(formatUid).then((childUid) => { + if (childUid) { + return formatUid; + } + + const createDefaultContent = defaultValue?.length + ? Promise.all( + defaultValue.map((node, order) => + createBlock({ + node, + parentUid: formatUid, + order, + }) + ) + ) + : createBlock({ + node: { text: " " }, + parentUid: formatUid, + }); + + return createDefaultContent.then(() => formatUid); + })src/queries/getSettingsByEmail.ts (1)
3-7: Consider handling potential query failures and returning a defaultUserSettingsshape.Currently, any errors thrown by the asynchronous query call may remain unhandled, and the function defaults to returning an empty object. This can lead to subtle runtime issues if consuming code expects a well-defined
UserSettingsstructure. You could wrap the query in atry/catchblock and return a default shape for safer fallbacks:const getSettingsByEmail = async (email: string): Promise<UserSettings> => { + try { const result = await window.roamAlphaAPI.data.backend.q( `[:find ?settings :where[?e :user/settings ?settings] [?e :user/email "${email}"]]` ); return (result?.[0]?.[0] as UserSettings) || {}; + } catch (err) { + console.error("Failed to query user settings:", err); + // Provide a default shape to avoid unexpected runtime errors + return { + "global-filters": { + includes: [], + removes: [], + }, + }; + } };src/dom/resolveRefs.ts (2)
28-34: Consider gracefully handling missing block UIDs in the alias reference loop.Similar to the alias tag loop, if a block reference can’t be resolved,
getRoamUrl(blockUid)may produce an invalid URL. As a failsafe, you might want to substitute a fallback link or skip the replacement:const url = blockUid ? getRoamUrl(blockUid) : "#";
36-43: Improve variable naming and handle block reference failures.
- Using
_for the entire match can be confusing. Consider a more expressive identifier likefullMatch.- If
getTextByBlockUid(blockUid)returnsundefined, you already fall back toblockUid, which is helpful. However, you could optionally log or track missing references for debugging.for (const match of blockRefMatches) { - const [_, blockUid] = match; + const [fullMatch, blockUid] = match; const reference = await getTextByBlockUid(blockUid); result = result.replace(fullMatch, reference || blockUid); }src/components/ComponentContainer.tsx (1)
52-67: Properly handled asynchronous block UID retrievalThe conversion to async handling of
getBlockUidFromTargetis well implemented with appropriate checks for DOM element validity after the async operation completes.Consider adding error handling for the promise chain to gracefully handle any failures in the
getBlockUidFromTargetfunction.getBlockUidFromTarget(b).then((blockUid) => { if (!b.parentElement) return; // Rest of the function +}).catch(error => { + console.error("Failed to get block UID from target", error); });src/queries/getBlockUidsAndTextsReferencingPage.ts (1)
4-16: Successfully migrated to async implementationThe function has been properly converted to use async/await with the backend API. The implementation correctly awaits the query result and maintains the same data transformation logic.
Two notes for consideration:
- The type casting to
[PullBlock][]is correct based on the imported type definition.- Consider adding error handling for the API call to gracefully handle potential failures.
const getBlockUidsAndTextsReferencingPage = async ( title: string ): Promise<{ uid: string; text: string }[]> => { + try { const result = (await window.roamAlphaAPI.data.backend.q( `[:find (pull ?r [:block/uid :block/string]) :where [?p :node/title "${normalizePageTitle( title )}"] [?r :block/refs ?p]]` )) as [PullBlock][]; return result.map(([node]) => ({ uid: node[":block/uid"] || "", text: node[":block/string"] || "", })); + } catch (error) { + console.error(`Error fetching blocks referencing page "${title}":`, error); + return []; + } };src/queries/getBlockUidAndTextIncludingText.ts (1)
3-13: Function successfully converted to async patternThe implementation now properly uses async/await with the backend API. The function effectively maintains its original behavior while migrating to asynchronous execution.
Consider adding error handling to gracefully manage potential failures:
const getBlockUidAndTextIncludingText = async ( t: string ): Promise<{ uid: string; text: string }[]> => { + try { const result = (await window.roamAlphaAPI.data.backend.q( `[:find (pull ?block [:block/uid :block/string]) :where [?block :block/string ?contents] [(clojure.string/includes? ?contents "${t}")]]` )) as [PullBlock][]; return result.map(([block]) => ({ uid: block[":block/uid"] || "", text: block[":block/string"] || "", })); + } catch (error) { + console.error(`Error finding blocks including text "${t}":`, error); + return []; + } };src/util/getSettingValueFromTree.ts (1)
16-20: Good handling of asynchronous tree resolutionThe implementation now correctly handles the case where
treeis not provided by awaitinggetBasicTreeByParentUid. The comment about type errors indicates awareness of TypeScript issues that were avoided with the current implementation.Consider removing the commented-out code since it's no longer needed and the comment explains why it was problematic.
- // results in type errors via `npx tsc --noEmit` - // const resolvedTree = tree || (await getBasicTreeByParentUid(parentUid));src/components/ConfigPage.tsx (2)
71-87: Consider adding loading states for better UX.While the async implementation is correct, there's no indication to the user when data is loading. Consider adding loading states to improve user experience.
+ const [isLoading, setIsLoading] = useState(true); useEffect(() => { + setIsLoading(true); const loadChildUids = async () => { try { const tree = await getShallowTreeByParentUid(parentUid); setChildUids( Object.fromEntries( tree.map(({ text, uid }) => [text.toLowerCase().trim(), uid]) ) ); + setIsLoading(false); } catch (error) { console.error("Failed to load child UIDs:", error); + setIsLoading(false); } }; loadChildUids(); }, [parentUid]);Similar pattern could be applied to the tree loading effect.
Also applies to: 143-147
81-84: Error handling could be improved.The current error handling only logs to console but doesn't communicate errors to users. Consider implementing a more robust error handling strategy.
} catch (error) { console.error("Failed to load child UIDs:", error); + // Consider showing an error message to the user + // or implementing a retry mechanism }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (80)
package.json(1 hunks)src/components/BlockInput.tsx(2 hunks)src/components/ComponentContainer.tsx(1 hunks)src/components/ConfigPage.tsx(8 hunks)src/components/ConfigPanels/BlocksPanel.tsx(1 hunks)src/components/ConfigPanels/MultiChildPanel.tsx(2 hunks)src/components/ConfigPanels/OauthPanel.tsx(2 hunks)src/components/ConfigPanels/useSingleChildValue.tsx(2 hunks)src/components/ExternalLogin.tsx(1 hunks)src/components/FormDialog.tsx(5 hunks)src/components/OauthSelect.tsx(2 hunks)src/components/PageInput.tsx(2 hunks)src/date/parseNlpDate.ts(1 hunks)src/dom/addBlockCommand.ts(2 hunks)src/dom/addKeyboardTriggers.ts(2 hunks)src/dom/createPageObserver.ts(1 hunks)src/dom/createPageTitleObserver.ts(1 hunks)src/dom/getBlockUidFromTarget.ts(2 hunks)src/dom/getDropUidOffset.ts(2 hunks)src/dom/getReferenceBlockUid.ts(1 hunks)src/dom/getRoamUrlByPage.ts(1 hunks)src/dom/resolveRefs.ts(1 hunks)src/queries/getAllBlockUids.ts(1 hunks)src/queries/getAllBlockUidsAndTexts.ts(1 hunks)src/queries/getAllPageNames.ts(1 hunks)src/queries/getAttributeValueByBlockAndName.ts(1 hunks)src/queries/getBasicTreeByParentUid.ts(1 hunks)src/queries/getBlockUidAndTextIncludingText.ts(1 hunks)src/queries/getBlockUidByTextOnPage.ts(1 hunks)src/queries/getBlockUidsAndTextsReferencingPage.ts(1 hunks)src/queries/getBlockUidsByPageTitle.ts(1 hunks)src/queries/getBlockUidsReferencingBlock.ts(1 hunks)src/queries/getBlockUidsReferencingPage.ts(1 hunks)src/queries/getBlockUidsWithParentUid.ts(1 hunks)src/queries/getChildrenLengthByPageUid.ts(1 hunks)src/queries/getChildrenLengthByParentUid.ts(1 hunks)src/queries/getCreateTimeByBlockUid.ts(1 hunks)src/queries/getCurrentUserDisplayName.ts(1 hunks)src/queries/getDisplayNameByEmail.ts(1 hunks)src/queries/getDisplayNameByUid.ts(1 hunks)src/queries/getEditTimeByBlockUid.ts(1 hunks)src/queries/getEditedUserEmailByBlockUid.ts(1 hunks)src/queries/getFirstChildTextByBlockUid.ts(1 hunks)src/queries/getFirstChildUidByBlockUid.ts(1 hunks)src/queries/getFullTreeByParentUid.ts(2 hunks)src/queries/getLinkedPageTitlesUnderUid.ts(1 hunks)src/queries/getNthChildUidByBlockUid.ts(1 hunks)src/queries/getOrderByBlockUid.ts(1 hunks)src/queries/getPageTitleByBlockUid.ts(1 hunks)src/queries/getPageTitleByPageUid.ts(1 hunks)src/queries/getPageTitleReferencesByPageTitle.ts(1 hunks)src/queries/getPageTitlesAndBlockUidsReferencingPage.ts(1 hunks)src/queries/getPageTitlesAndUidsDirectlyReferencingPage.ts(1 hunks)src/queries/getPageTitlesReferencingBlockUid.ts(1 hunks)src/queries/getPageTitlesStartingWithPrefix.ts(1 hunks)src/queries/getPageUidByBlockUid.ts(1 hunks)src/queries/getPageUidByPageTitle.ts(1 hunks)src/queries/getPageViewType.ts(1 hunks)src/queries/getParentTextByBlockUid.ts(1 hunks)src/queries/getParentTextByBlockUidAndTag.ts(1 hunks)src/queries/getParentUidByBlockUid.ts(1 hunks)src/queries/getParentUidsOfBlockUid.ts(1 hunks)src/queries/getSettingsByEmail.ts(1 hunks)src/queries/getShallowTreeByParentUid.ts(1 hunks)src/queries/getTextByBlockUid.ts(1 hunks)src/queries/isLiveBlock.ts(1 hunks)src/queries/isTagOnPage.ts(1 hunks)src/testing/mockRoamEnvironment.ts(1 hunks)src/types/index.ts(3 hunks)src/util/addInputSetting.ts(2 hunks)src/util/extensionDeprecatedWarning.ts(1 hunks)src/util/getAuthorizationHeader.ts(1 hunks)src/util/getOauth.ts(3 hunks)src/util/getOauthAccounts.ts(2 hunks)src/util/getSettingValueFromTree.ts(1 hunks)src/util/getSubTree.ts(1 hunks)src/util/getToken.ts(1 hunks)src/util/handleFetch.ts(1 hunks)src/util/setInputSetting.ts(2 hunks)src/util/setInputSettings.ts(2 hunks)
🧰 Additional context used
🧬 Code Definitions (22)
src/queries/getPageViewType.ts (1)
src/types/native.ts (1)
ViewType(236-236)
src/queries/getAttributeValueByBlockAndName.ts (1)
src/types/native.ts (1)
PullBlock(171-232)
src/queries/getBlockUidsAndTextsReferencingPage.ts (1)
src/types/native.ts (1)
PullBlock(171-232)
src/queries/getBasicTreeByParentUid.ts (1)
src/types/native.ts (2)
RoamBasicNode(146-150)PullBlock(171-232)
src/util/extensionDeprecatedWarning.ts (1)
src/types/native.ts (1)
PullBlock(171-232)
src/queries/getBlockUidsReferencingPage.ts (1)
src/types/native.ts (1)
PullBlock(171-232)
src/util/handleFetch.ts (1)
src/util/env.ts (1)
getApiUrlEnv(24-34)
src/queries/getShallowTreeByParentUid.ts (1)
src/types/native.ts (1)
PullBlock(171-232)
src/queries/getParentUidsOfBlockUid.ts (1)
src/types/native.ts (1)
PullBlock(171-232)
src/queries/getFullTreeByParentUid.ts (1)
src/types/native.ts (1)
TreeNode(266-291)
src/queries/getPageTitleByPageUid.ts (1)
src/types/native.ts (1)
PullBlock(171-232)
src/queries/getPageTitleReferencesByPageTitle.ts (1)
src/types/native.ts (1)
PullBlock(171-232)
src/queries/getSettingsByEmail.ts (1)
src/types/native.ts (1)
UserSettings(363-368)
src/queries/getBlockUidsReferencingBlock.ts (1)
src/types/native.ts (1)
PullBlock(171-232)
src/queries/getPageTitlesReferencingBlockUid.ts (1)
src/types/native.ts (1)
PullBlock(171-232)
src/types/index.ts (1)
src/types/native.ts (1)
PullBlock(171-232)
src/queries/getBlockUidAndTextIncludingText.ts (1)
src/types/native.ts (1)
PullBlock(171-232)
src/queries/getPageTitlesAndBlockUidsReferencingPage.ts (1)
src/types/native.ts (1)
PullBlock(171-232)
src/queries/getNthChildUidByBlockUid.ts (1)
src/types/native.ts (1)
PullBlock(171-232)
src/queries/getPageTitlesAndUidsDirectlyReferencingPage.ts (1)
src/types/native.ts (1)
PullBlock(171-232)
src/queries/getParentUidByBlockUid.ts (1)
src/types/native.ts (1)
PullBlock(171-232)
src/components/ConfigPage.tsx (1)
src/types/native.ts (1)
RoamBasicNode(146-150)
🔇 Additional comments (115)
package.json (1)
4-4: Appropriate version bump for async migration.The version change from "0.85.1" to "1.0.0-beta.2" correctly follows semantic versioning principles for a breaking change like async migration. The major version bump (1.0.0) signals to consumers that they'll need to update their implementation, while the beta tag appropriately indicates this is a pre-release version.
src/dom/getRoamUrlByPage.ts (1)
4-6: Well-implemented async conversion.The function has been properly converted to use async/await pattern with the correct Promise return type. Consumers of this function will need to update their code to await the result or handle the Promise.
src/queries/getOrderByBlockUid.ts (1)
1-6: Clean async implementation.The function has been properly converted to async/await with the correct Promise return type. The implementation correctly awaits the API call and explicitly returns the result. Consumers of this function will need to be updated to await the result.
src/queries/getPageViewType.ts (1)
4-11: LGTM! Good async conversion.The function has been properly converted to async, with the correct return type update to
Promise<ViewType>and properly awaited API call. The result handling is clear and maintains the same behavior as before.src/queries/getPageUidByPageTitle.ts (1)
1-7: Well-implemented async conversion.The function has been properly converted to async with the appropriate Promise return type. The early return for falsy title values is maintained, and the API call is correctly awaited.
src/queries/getAllBlockUids.ts (1)
1-6: Good async implementation with updated API call.The function has been properly converted to async with the correct return type. Notable change is the API method update from
roamAlphaAPI.qtoroamAlphaAPI.data.backend.qwhich is properly awaited.src/queries/getTextByBlockUid.ts (1)
1-8: Well-structured async conversion with improved error handling.The async conversion is complete with appropriate Promise return type. Improvements include:
- Early return for falsy UIDs
- More readable result handling
- Updated API method from
roamAlphaAPI.pulltoroamAlphaAPI.data.pullThese changes enhance the function's robustness while maintaining its core functionality.
src/util/setInputSetting.ts (2)
6-16: Function correctly converted to async with proper return typeThe function signature has been updated to include the
asynckeyword and the return type now correctly specifiesPromise<string>. This change aligns with the PR objective to migrate functions to asynchronous implementations.
17-17: Correctly awaits the asynchronous operationThe function now properly awaits the result from
getBasicTreeByParentUidbefore proceeding with the rest of the logic. This ensures that the tree data is available before making decisions based on it.src/dom/getBlockUidFromTarget.ts (2)
5-5: Function signature correctly updated for async operationThe function signature has been properly updated to include the
asynckeyword and the return type now correctly specifiesPromise<string>. This aligns with the PR objective to migrate to asynchronous implementations.
33-33: Properly awaits asynchronous operationThe function now correctly awaits the result from
getNthChildUidByBlockUidbefore returning it, ensuring that the operation completes before proceeding.src/queries/getAllBlockUidsAndTexts.ts (1)
1-8: Function correctly converted to use async/await with backend APIThe function has been properly converted to an asynchronous implementation using the
asynckeyword and now returns a Promise. The API call has been updated to usewindow.roamAlphaAPI.data.backend.qwithawait, which is correct for this migration.src/queries/getBlockUidsByPageTitle.ts (2)
3-10: Successfully migrated to async implementation with backend APIThe function has been properly converted to use async/await and now correctly uses the backend API with proper awaiting of the result. The code structure is clean and the return type has been updated appropriately to
Promise<string[]>.
3-10:Details
✅ Verification successful
Verify callers have been updated to handle Promises
Since this function now returns a Promise instead of an immediate value, ensure that all callers of this function have been updated to either await the result or handle the Promise appropriately.
🏁 Script executed:
#!/bin/bash # Search for direct imports and usages of getBlockUidsByPageTitle echo "Searching for imports of getBlockUidsByPageTitle:" rg "import.*getBlockUidsByPageTitle" --type ts echo -e "\nSearching for usages of getBlockUidsByPageTitle:" rg "getBlockUidsByPageTitle\(" --type ts -A 2 -B 2Length of output: 785
Promise Handling for getBlockUidsByPageTitle Verified
The asynchronous function is now correctly handled. The search confirms that in
src/dom/createPageObserver.tsthe function is imported and called using.then(), ensuring all callers properly handle the returned Promise.
- Verified usage in
src/dom/createPageObserver.tsemploys.then()to handle the Promise.- No instances were found where the returned Promise is mishandled.
src/queries/getChildrenLengthByParentUid.ts (1)
1-7: Good job migrating this function to asyncThe conversion to async/await is clean and correctly implemented with proper Promise return typing. This change aligns well with the PR objectives of migrating to async operations.
src/queries/getPageUidByBlockUid.ts (1)
1-6: Properly converted to async with appropriate Promise typingGood implementation of the async migration. The function now correctly awaits the Roam API call and maintains the same error handling with the fallback to an empty string.
src/queries/getCurrentUserDisplayName.ts (1)
4-7:Details
✅ Verification successful
Consider potential inconsistency with getCurrentUserUid
The function has been properly converted to async, but I notice that
getCurrentUserUid()is not awaited. IfgetCurrentUserUidhas also been migrated to async in this PR, it should be awaited as well.
🏁 Script executed:
#!/bin/bash # Check if getCurrentUserUid is async grep -A 10 "const getCurrentUserUid" src/queries/getCurrentUserUid.tsLength of output: 326
Verified: No async inconsistency in getCurrentUserUid
The
getCurrentUserUidfunction is synchronous (returns a string), so there is no need to useawaitwhen calling it withingetCurrentUserDisplayName. The implementation is correct as written.src/util/addInputSetting.ts (1)
5-17: Successfully migrated to async with proper awaiting of dependenciesThe function has been properly updated to be async and correctly awaits the result from
getBasicTreeByParentUid.src/dom/getDropUidOffset.ts (2)
5-7: Function signature properly updated to asyncThe function has been correctly modified to be asynchronous, with the return type properly updated to return a Promise.
26-29: Await added correctly for asynchronous callThe
awaitkeyword is properly applied to thegetPageUidByPageTitlecall, which is now an async function based on the PR objectives.src/queries/getFirstChildUidByBlockUid.ts (1)
3-4: Function correctly migrated to async implementationThe function has been properly converted to use async/await pattern with correct return type specification and awaiting the nested function call.
src/queries/getCreateTimeByBlockUid.ts (1)
1-7: Function migrated to async with API changeThe function has been properly converted to async with the correct Promise return type. The implementation now uses the newer
window.roamAlphaAPI.data.pullmethod with proper awaiting.Note that the query changed from
[:edit/time](based on the summary) to[:create/time], which appears to be a functional correction rather than just an async migration.Please verify that changing from
[:edit/time]to[:create/time]is intentional and aligns with the function's purpose of retrieving creation time rather than edit time.src/queries/getPageTitlesStartingWithPrefix.ts (1)
1-8: Function correctly migrated to async with updated API callThe function has been properly converted to async with the Promise return type. The implementation now uses
window.roamAlphaAPI.data.backend.qinstead of the olderwindow.roamAlphaAPI.qmethod and correctly awaits the result before mapping it.src/dom/createPageTitleObserver.ts (1)
18-35: Well-executed conversion to asynchronous pattern.The code properly refactors the synchronous call to
getPageUidByPageTitleinto an asynchronous implementation using a Promise chain. All the operations that depend on the UID value are now correctly contained within the Promise's.then()handler, ensuring they execute only after the UID is available.The setTimeout inside the Promise handler is still necessary as it addresses a different concern - handling React's re-rendering behavior for DOM elements, as noted in the comment.
src/util/extensionDeprecatedWarning.ts (2)
18-24: Good implementation of async/await pattern.The function was previously declared as async, and now properly uses
awaitwith all asynchronous operations. The code maintains its logical flow while properly waiting for each operation to complete before continuing.
25-30: Correct async implementation for Roam API query.The addition of
awaitfor the Roam API query call is appropriate since database operations are inherently asynchronous. The type casting to[PullBlock][]ensures proper typing of the query results.src/util/getOauthAccounts.ts (3)
7-7: Function signature correctly updated for async.The function signature has been properly updated to be async and return a Promise<string[]> instead of string[], which correctly reflects its new asynchronous nature.
16-18: Proper async implementation for data retrieval.Both calls to retrieve data are now correctly awaited, ensuring the pageUid and tree data are fully loaded before being used.
22-23: Consistent async pattern application.The async pattern is consistently applied throughout the function, with the final data retrieval and transformation also using await before mapping the results.
src/util/setInputSettings.ts (1)
6-16: Function signature correctly updated for async operation.The function signature has been properly updated to be async and return a Promise, which correctly reflects the function's new asynchronous nature.
src/queries/getBlockUidsReferencingPage.ts (1)
4-13: Implementation migrated to async correctlyThe function has been properly converted to an asynchronous implementation using
async/awaitwith the Roam API. The return type is correctly updated toPromise<string[]>to reflect this change.src/queries/getEditTimeByBlockUid.ts (1)
1-7: Function correctly migrated to asyncThe function has been properly converted to an asynchronous implementation, with the correct return type
Promise<number>and appropriate use ofawaitwith the API call.src/queries/getDisplayNameByEmail.ts (1)
1-6: Function successfully migrated to asyncThe function has been correctly converted to an asynchronous implementation with the proper return type and await syntax.
src/queries/getChildrenLengthByPageUid.ts (2)
2-8: Function correctly migrated to asyncThe function has been properly converted to use the async pattern with the correct return type and await syntax.
1-1:Details
❓ Verification inconclusive
Consider updating the deprecated function's replacement
This function is marked as deprecated in favor of
getChildrenLengthByParentUid. Make sure that the recommended alternative has also been migrated to async for consistency.
🏁 Script executed:
#!/bin/bash # Check if the replacement function has been updated to async as well echo "Checking getChildrenLengthByParentUid implementation:" rg "const getChildrenLengthByParentUid" --type tsLength of output: 168
I'm generating an additional shell script to search for async declaration patterns of
getChildrenLengthByParentUidto verify whether it has been migrated to async. This should help us confirm if the replacement function is consistent with the new async conventions.
🏁 Script executed:
#!/bin/bash echo "Searching for async function declaration of getChildrenLengthByParentUid using named function syntax:" rg "export async function getChildrenLengthByParentUid" --type ts echo "Searching for async arrow function declaration of getChildrenLengthByParentUid:" rg "export const getChildrenLengthByParentUid = async" --type ts echo "Searching for any async usage in declarations of getChildrenLengthByParentUid:" rg "async.*getChildrenLengthByParentUid" --type tsLength of output: 736
Action Required: Verify and Update the Async Consistency of
getChildrenLengthByParentUidOur automated searches for async declarations of
getChildrenLengthByParentUid(both named and arrow function patterns) returned no matches. This indicates that the replacement function may not have been migrated to async, which conflicts with the consistency goal noted in the original deprecation comment. Please manually verify the implementation ofgetChildrenLengthByParentUidand update it to use async syntax if appropriate.src/util/getToken.ts (1)
6-15: Well-implemented asynchronous conversionThis function has been correctly updated to use async/await, matching the PR objective of migrating to asynchronous implementations. The function now properly awaits the results from both
getPageUidByPageTitleandgetBasicTreeByParentUid, which were likely also converted to async functions.Good job preserving the existing logic flow while adding proper Promise handling:
- First checking local storage (which remains synchronous)
- Then proceeding with asynchronous operations if needed
- Returning a Promise with the final result
src/queries/getAttributeValueByBlockAndName.ts (1)
4-18: Good migration to async with improved API usageThe function has been properly converted to an async implementation with the following improvements:
- Function signature updated with
asynckeyword and explicitPromise<string>return type- Switched from
fast.qtobackend.qAPI for asynchronous query execution- Added proper
awaitfor the query result- Simplified the result extraction while maintaining the same functionality
This change aligns well with the PR objective of migrating to async implementations. The function maintains its original parameter structure and behavior while becoming asynchronous.
src/components/BlockInput.tsx (4)
8-14: Correctly added useEffect importAdded
useEffectto the React imports, which is necessary for the new async data fetching implementation.
38-38: Properly updated prop type to reflect async functionThe type definition for
getAllBlockshas been correctly updated to returnPromise<{ text: string; uid: string }[]>instead of a synchronous array return.
42-44: Added state for async data managementAdded a new state variable
allBlocksto store the data fetched asynchronously. This is a good pattern for handling async data in React components.
54-54: Correctly updated useMemo dependenciesAdded
isOpento the dependency list for theitemsuseMemo hook, which will ensure the items are recalculated when the dropdown's open state changes. This is a good optimization to avoid unnecessary calculations when closed.src/queries/isLiveBlock.ts (1)
1-7: Clean migration to async implementationThis function has been properly converted to an asynchronous implementation:
- Added
asynckeyword andPromise<boolean>return type- Added
awaitfor the API call towindow.roamAlphaAPI.data.pull- Preserved the same logical flow and return value behavior
This implementation aligns well with the PR objective of migrating to async and follows best practices for Promise-based async code.
src/queries/getEditedUserEmailByBlockUid.ts (1)
1-8: Proper implementation of async API callThe function has been correctly converted to async, properly handling the asynchronous nature of the Roam API call. The return type is appropriately updated to
Promise<string>and the function correctly awaits the API response before processing the result.src/queries/getDisplayNameByUid.ts (1)
1-6: Good implementation of async data queryThe function has been successfully migrated to an async implementation, properly returning a Promise and awaiting the API call result. The result handling logic remains consistent with the synchronous version.
src/queries/getBasicTreeByParentUid.ts (1)
12-19: Method successfully converted to async with proper Promise handling.The function has been correctly converted to an asynchronous implementation using the async/await pattern. The function now properly returns a Promise and uses await when calling the Roam API. The change from
window.roamAlphaAPI.data.fast.qtowindow.roamAlphaAPI.data.backend.qaligns with the PR's goal of migrating to async functionality.src/queries/getAllPageNames.ts (1)
1-6: Successfully converted to an async implementation.The function has been properly refactored to use async/await syntax. The return type is correctly specified as
Promise<string[]>, and the implementation properly awaits the API call. The change fromwindow.roamAlphaAPI.qtowindow.roamAlphaAPI.data.backend.qis consistent with the migration to async APIs mentioned in the PR objectives.src/dom/addBlockCommand.ts (1)
21-43: Successfully converted loadBlockUid to async implementation.The inner function has been properly changed to use async/await syntax, with all necessary dependent calls being awaited appropriately. This enables proper handling of the asynchronous operations throughout the function.
src/util/getAuthorizationHeader.ts (1)
4-9: Successfully converted to async with code optimization.The function has been properly converted to an async implementation with the correct Promise return type. The code is also optimized by retrieving the token once and reusing it, rather than potentially calling
getToken()twice as in the previous implementation. Both branches of the conditional now properly handle asynchronous operations.src/queries/getBlockUidsWithParentUid.ts (1)
1-6: Well-executed conversion to asyncThe function has been properly converted to an async implementation, returning a Promise and using the
awaitkeyword with the Roam API. The change fromwindow.roamAlphaAPI.qtowindow.roamAlphaAPI.data.backend.qis aligned with the migration to async pattern.The result mapping logic is correctly preserved, ensuring the same data structure is returned.
src/queries/getBlockUidByTextOnPage.ts (1)
1-12: Clean conversion to async with proper error handlingThe function has been well-converted to an async implementation with the appropriate Promise return type. The use of optional chaining (
?.[0]?.[0]) combined with the empty string fallback provides good error handling for cases where the query returns no results.The query logic remains intact while properly implementing the async pattern.
src/queries/getPageTitlesAndUidsDirectlyReferencingPage.ts (1)
4-16: Good async implementation with API consistencyThe function has been properly converted to an async implementation with the correct Promise return type. The change from using
window.roamAlphaAPI.data.fast.qtowindow.roamAlphaAPI.data.backend.qis consistent with the async migration pattern across the codebase.The type casting of the result as
[PullBlock][]is appropriate and matches the expected return structure, ensuring type safety when mapping the results.src/queries/isTagOnPage.ts (1)
3-18: Well-implemented async conversion with boolean handlingThe function has been properly converted to an async implementation with the appropriate Promise return type. The use of the double-bang operator (
!!result?.[0]?.[0]) is an elegant way to ensure a boolean return value while handling potential undefined results.The query structure remains unchanged while correctly implementing the async pattern.
src/queries/getPageTitleByBlockUid.ts (1)
1-6: Function successfully converted to async pattern 👍The function has been properly refactored to an asynchronous implementation using the correct Promise return type and await syntax. The change correctly updates the API call to use
window.roamAlphaAPI.data.backend.qwhile maintaining the same logical behavior and error handling with optional chaining.src/components/ConfigPanels/useSingleChildValue.tsx (3)
23-24: State initialization simplifiedGood change to initialize the states directly rather than deriving them. This approach is cleaner and easier to follow with the asynchronous implementation.
26-34: Well-structured async effect hookThe useEffect hook is properly implemented to handle async data loading when the uid changes:
- Correctly handles conditional execution when uid exists
- Properly uses an inner async function
- Appropriate dependency array
This aligns well with React's recommended patterns for async operations in effects.
36-44: Good separation of concerns in effectsThis second useEffect properly separates the concerns of loading the value based on valueUid changes. The implementation follows best practices by:
- Creating a dedicated effect for this operation
- Using an inner async function
- Adding appropriate dependencies (valueUid and transform)
This separation makes the code more maintainable and easier to reason about.
src/queries/getPageTitlesReferencingBlockUid.ts (1)
3-10: Successfully converted to async implementationThe function has been properly converted to an async implementation with the correct Promise return type. The API call now uses
window.roamAlphaAPI.data.backend.qwith await syntax, while maintaining the same mapping logic to extract page titles from the result.The typing with PullBlock is preserved, ensuring type safety throughout the query and result processing.
src/queries/getFirstChildTextByBlockUid.ts (1)
1-8: Successfully migrated to async patternThe function has been properly converted to an asynchronous implementation:
- Return type correctly updated to Promise
- Added async keyword to the function declaration
- Using await with the API call
- Result handling is maintained with the same optional chaining pattern
This change aligns well with the PR's objective to migrate functions to async implementations.
src/queries/getFullTreeByParentUid.ts (1)
39-58: Well-executed async conversionThe function has been properly converted to an async implementation, aligning with the migration objective. The return type is correctly updated to
Promise<TreeNode>, and the function now usesawaitwith the Roam API call.src/queries/getPageTitleReferencesByPageTitle.ts (1)
4-13:Details
✅ Verification successful
Verify API change from fast.q to backend.q
While the async conversion is well implemented, there's also a change from
window.roamAlphaAPI.data.fast.qtowindow.roamAlphaAPI.data.backend.q. Ensure this API change was intentional as it might have different performance characteristics.
🏁 Script executed:
#!/bin/bash # Search for other occurrences of fast.q and backend.q to see if this change is consistent across the codebase echo "Searching for fast.q usage:" rg "fast\.q" --type ts -A 2 -B 2 echo -e "\nSearching for backend.q usage:" rg "backend\.q" --type ts -A 2 -B 2 echo -e "\nSearching for API documentation comments that might explain the difference:" rg -i "backend\.q|fast\.q" --type ts -A 5 -B 5 | grep -i "comment\|doc\|//"Length of output: 15615
API Change Verified: Backend Query API Consistent
After a thorough search of the codebase, no remaining references to the deprecated
fast.qwere found. The use ofwindow.roamAlphaAPI.data.backend.qis consistent across multiple query files (e.g.,src/queries/getLinkedPageTitlesUnderUid.ts,src/queries/getParentUidByBlockUid.ts, etc.), indicating that the API switch was intentional. No further changes are required.src/components/PageInput.tsx (2)
1-1: Well-implemented React async patternThe component has been properly updated to handle asynchronous data loading using React hooks. Good job implementing error handling and only loading the data once when the component mounts.
Also applies to: 13-26
27-27: Correctly updated useMemo dependenciesThe dependency array for useMemo has been properly updated to include both
pageNamesandextra, ensuring the options are recalculated when either changes.src/util/getOauth.ts (6)
8-8: Function signature properly converted to asyncThe function has been correctly updated to use
asyncand return aPromise<string>to align with the asynchronous migration.
21-22: Properly awaiting asynchronous page and tree retrieval operationsThese calls to
getPageUidByPageTitleandgetShallowTreeByParentUidhave been properly converted to useawait, ensuring the function correctly waits for the page UID and tree data before proceeding.
27-27: Correctly awaiting node children retrievalThe call to
getShallowTreeByParentUidhas been properly awaited to fetch node children asynchronously.
39-39: Properly awaiting edit time retrievalThe call to
getEditTimeByBlockUidis now awaited, correctly handling the asynchronous operation.
43-43: Correctly awaiting data node retrievalThe retrieval of the data node via
getShallowTreeByParentUidis properly awaited and the result is accessed correctly.
51-51: Properly handling conditional awaiting of edit timeThe conditional awaiting of
getEditTimeByBlockUidis correctly implemented, ensuring the edit time is only retrieved when a UID exists.src/queries/getLinkedPageTitlesUnderUid.ts (1)
1-6: Function correctly migrated to async implementationThe function has been properly converted to use
async/awaitwith the correct return type ofPromise<string[]>. The implementation now uses the asynchronouswindow.roamAlphaAPI.data.backend.qmethod, which is correctly awaited before mapping the results.src/queries/getNthChildUidByBlockUid.ts (1)
3-14: Function correctly migrated to async implementationThe function has been properly converted to use
async/awaitwith the appropriate return type ofPromise<string>. The implementation now correctly awaits the result fromwindow.roamAlphaAPI.data.backend.qand properly handles the result using optional chaining to safely access the block UID.src/dom/getReferenceBlockUid.ts (2)
5-8: Function signature properly converted to asyncThe function has been correctly updated to use
asyncand return aPromise<string>to align with the asynchronous migration.
14-14: Properly awaiting reference block retrievalThe call to
getBlockUidsReferencingBlockis now correctly awaited, ensuring the function properly handles the asynchronous operation before using the result.src/queries/getParentTextByBlockUidAndTag.ts (1)
1-12: Successfully migrated to async/await pattern ✅The function has been properly converted to an asynchronous implementation with the correct Promise return type and await syntax. The code also handles potential undefined values correctly with the null coalescing operator.
src/queries/getPageTitlesAndBlockUidsReferencingPage.ts (1)
4-16:Details
✅ Verification successful
API change alongside async conversion
The function has been properly converted to an async implementation with the correct Promise return type. However, note that the implementation also changed from using
window.roamAlphaAPI.data.fast.qtowindow.roamAlphaAPI.data.backend.q. This may have performance implications that should be considered.
🏁 Script executed:
#!/bin/bash # Check for other usages of fast.q vs backend.q to ensure consistency echo "Checking for usage patterns of data.fast.q vs data.backend.q in the codebase:" rg -A 1 "roamAlphaAPI\.data\.(fast|backend)\.q" --type tsLength of output: 8738
Verified Async Conversion & Consistent API Usage
The conversion to an async implementation with the correct Promise return type is spot on. Furthermore, switching from using
window.roamAlphaAPI.data.fast.qtowindow.roamAlphaAPI.data.backend.qis consistent with the rest of the codebase (as seen in multiple query modules). That said, be mindful that the backend API may perform differently compared to the fast version, so if this function is part of a high-load path, consider monitoring its performance.
- File Affected:
src/queries/getPageTitlesAndBlockUidsReferencingPage.ts(lines 4-16)- Key Observation: Other queries in the repository consistently use
backend.q, indicating that this change is intentional.- Recommendation: Keep an eye on performance in critical usage scenarios to ensure the new API meets your needs.
src/components/ExternalLogin.tsx (1)
84-120: Improved error handling with async/await 👍The code now properly uses try/catch around the async operations and provides clear error messages. This is a significant improvement in error handling and aligns well with the async migration objective.
Some key improvements:
- Properly awaits
getBasicTreeByParentUidandcreateBlockfunctions- Detailed error messages that handle both Error objects and other error types
- Early return on error to prevent further execution
src/dom/resolveRefs.ts (2)
16-18: Async update looks good.Transitioning to
async/awaitaligns with the overall pull request goal of asynchronous migration. Ensure any existing call sites are updated to handle the returned promise correctly.
19-26: Handle missing or undefined page UIDs in the alias tag loop.If
getPageUidByPageTitle(pageName)returnsundefined,urlmight end up being malformed or empty. You could add an extra safety check and conditionally replace the alias to avoid potential breaks:const blockUid = await getPageUidByPageTitle(pageName); - const url = getRoamUrl(blockUid); + const url = blockUid ? getRoamUrl(blockUid) : "#"; const newAlias = alias.replace(del, url); result = result.replace(alias, newAlias);src/util/handleFetch.ts (3)
25-25: Async function declaration is proper.Declaring
handleFetchasasyncis consistent with the new approach. This helps ensure the rest of the code is ready toawaitthe returned promise.
30-31: Verify error handling fromgetAuthorizationHeader.If
getAuthorizationHeader()fails (e.g., missing or invalid token), the error might bubble up here. You could wrap it in a try/catch to return a more descriptive error or proceed without authorization:const authHeader = !anonymous ? { + Authorization: authorization || (await (async () => { + try { + return await getAuthorizationHeader(); + } catch (err) { + console.error("Failed to retrieve authorization header:", err); + return ""; + } + })()), } : ({} as HeadersInit);
36-36: Header merging strategy looks good.We’re merging
authHeaderfirst and then user-suppliedheaders, letting user-supplied headers override any defaults. This approach is sensible for maximum flexibility.src/util/getSubTree.ts (2)
6-19: Successfully migrated function to async pattern with improved parameter handlingThe function has been properly converted to use async/await while maintaining its original behavior. The removal of the default parameter value and resolving it inside the function body is a good practice for async operations.
The implementation correctly:
- Marks the function as async and updates its return type
- Handles the optional tree parameter properly
- Awaits the asynchronous call to getBasicTreeByParentUid
22-29: Properly awaiting block creationThe call to
createBlockis now correctly awaited, ensuring that the function doesn't return until the block is created.Note that since this function returns a node with the newly generated UID, it's important that the await is present to ensure the block exists in the database before returning.
src/testing/mockRoamEnvironment.ts (1)
1390-1407: Proper async adaptation for roamAlphaAPI.pull methodThe changes wrap the existing synchronous implementation with
Promise.resolve()to make it return a Promise, aligning with the PR's goal of migrating to async implementations. The refactored code provides better readability and error handling through a cleaner structure using ternary operators and IIFEs.src/date/parseNlpDate.ts (5)
13-14: Good type definition for the days of the weekThe addition of the
WeekDaytype creates a clear and strongly-typed representation of valid days of the week, enhancing type safety.
15-16: Appropriate caching mechanism for week settingsUsing a cached value for
weekStartsOnis a good optimization to avoid repeated API calls for a value that rarely changes.
17-26: Well-implemented async function for retrieving week start preferenceThe
getWeekStartsOnfunction properly implements async/await pattern to fetch user settings from the API. It includes good fallback to default value (0) if the setting isn't found.
28-34: Appropriate handling of cached values in startOfWeekThe function now correctly handles the asynchronous nature of fetching the week start preference. It attempts to load the preference if not cached, but continues with a default value to avoid blocking the UI.
36-42: Consistent implementation in endOfWeek matching startOfWeekThe changes to
endOfWeekmirror those instartOfWeek, maintaining consistency between the two related functions.src/queries/getShallowTreeByParentUid.ts (1)
3-15: Good conversion to async implementationThe function has been properly converted to an async implementation, using
awaitwith the Roam API call. The return type now correctly reflects that it returns a Promise.One notable change is the switch from
window.roamAlphaAPI.data.fast.qtowindow.roamAlphaAPI.data.backend.q, which suggests that the backend version supports async operations.src/util/getSettingValueFromTree.ts (1)
5-15: Appropriate update of function signature to asyncThe function signature has been properly updated to be async, including changing the return type to
Promise<string>. The removal of the default value fortreeis appropriate since that calculation is now awaited asynchronously.src/components/OauthSelect.tsx (3)
2-2: Import updated to include useEffect.The addition of
useEffectis appropriate for the asynchronous data loading pattern introduced in this component.
14-16: State management refactored for async operation.Good refactoring of state management using separate state variables with empty initial states to accommodate asynchronous loading of data.
17-29: Implemented async data loading with useEffect.The implementation of asynchronous data loading using
useEffectis well done. The function correctly:
- Fetches the page UID asynchronously
- Retrieves the tree data asynchronously
- Processes the data to extract account labels
- Updates state with the fetched data
- Has a proper dependency array
[id]to reload when the ID changessrc/components/ConfigPanels/OauthPanel.tsx (5)
2-2: Import updated to include useEffect.The addition of
useEffectis appropriate for implementing the new asynchronous data loading pattern in this component.
13-14: Added type definition for Accounts.Good addition of a type definition for
Accounts, which improves code readability and type safety throughout the component.
18-20: Simplified state initialization.The state initialization has been appropriately simplified for the new async pattern, initializing to an empty array when
useLocalis false.
22-34: Implemented async data loading with useEffect.Well-implemented useEffect hook for asynchronous data loading. It:
- Runs when either
uidoruseLocalchanges- Only fetches data when needed (when
!useLocal && uid)- Uses promise-based approach to handle the asynchronous data
- Maintains the same data transformation logic
42-44: Updated onCheck to use promises.The
onCheckcallback has been properly updated to use promise-based pattern for handling the asynchronousgetShallowTreeByParentUidfunction, ensuring that block deletion happens after data is fetched.src/components/FormDialog.tsx (5)
28-28: Added InputTextNode import.Added necessary import for the type used in the component's props and state.
138-142: Refactored EmbedInput to use async pattern.Good refactoring of the tree data loading to use async/await pattern, properly separating the async logic into a named function
updateTreeData.
172-181: Updated onKeyDown handler to be async.The event handler has been properly converted to an async function to correctly await the result of the now-asynchronous
window.roamAlphaAPI.pullcall.
215-229: Created new AsyncBlockInput component.Good implementation of a new component to handle asynchronous block input. The component:
- Takes the same props as BlockInput with modified setValue function type
- Manages its own text state that's initialized asynchronously
- Fetches the block text using getTextByBlockUid when the value changes
- Renders the original BlockInput with the fetched text
415-424: Updated to use AsyncBlockInput component.The implementation correctly uses the new AsyncBlockInput component and handles the asynchronous setValue logic appropriately:
- Properly awaits the result of the API call
- Handles the conditional logic for setting the value based on whether a UID is provided
- Checks if the result exists before setting the value
src/types/index.ts (4)
85-88: Updated pull method return type to Promise.Correctly updated the return type of the global
roamAlphaAPI.pullmethod to bePromise<PullBlock>to reflect its new asynchronous nature.
105-110: Added async and backend query methods.Good addition of new asynchronous query methods in the data namespace:
async.q- for promise-based queryingbackend.q- for promise-based backend queryingBoth methods follow the same parameter pattern as the synchronous version but return promises.
125-128: Updated data.pull return type to Promise.Correctly updated the return type of
roamAlphaAPI.data.pullto bePromise<PullBlock>to maintain consistency with the global pull method.
125-136:Details
❓ Verification inconclusive
Verify corresponding changes in pull_many method.
The
pull_manymethod (line 129) still returnsPullBlock[]synchronously, while thepullmethod now returns a Promise. Check ifpull_manyshould also be updated to return a Promise for consistency.
🏁 Script executed:
#!/bin/bash # Search for usages of pull_many to determine if it needs to be async rg -A 5 -B 5 "pull_many" --type tsLength of output: 668
Action Required: Verify Asynchronous Consistency for
pull_manyAfter reviewing the usage of
pull_many, it’s confirmed that the signature insrc/types/index.tsstill returns a synchronousPullBlock[], while thepullmethod now returns aPromise<PullBlock>. Please verify whether this difference is intentional. Ifpull_manyis expected to perform asynchronous operations (similar topull), consider updating its return type toPromise<PullBlock[]>and adjust its implementation and calls accordingly.
- Location:
src/types/index.ts(Lines 125-136)- Current Signature:
pull_many: (pattern: string, eid: string[][]) => PullBlock[];- Suggestion if Asynchronous Handling is Required:
pull_many: (pattern: string, eid: string[][]) => Promise<PullBlock[]>;src/components/ConfigPage.tsx (8)
2-8: Added useEffect for async data fetching support.Good addition of useEffect to the imports list, which is necessary for the new async data loading patterns implemented in this file.
20-20: Updated import to include RoamBasicNode type.The RoamBasicNode type is now properly imported and will be used for state typing in the ConfigPage component.
71-87: Good implementation of async loading for child UIDs.The changes properly convert synchronous data loading to an async pattern using useState and useEffect. The implementation includes:
- State initialization with an empty object
- Async data loading with proper error handling
- Dependency array with parentUid to reload when needed
This is aligned with the migration to async functions in the PR objectives.
143-147: Implemented async tree loading with useState and useEffect.Good conversion from synchronous to asynchronous data loading. The code now:
- Initializes tree state with an empty array
- Uses useEffect to fetch data asynchronously
- Updates state with the fetched data
- Dependencies are properly set to re-fetch when pageUid changes
This follows React best practices for handling asynchronous operations.
275-279: Updated render function to be async and handle promises correctly.The render function has been properly converted to an async function. The pageUid parameter is now optional with a fallback that awaits the result of getPageUidByPageTitle. This change aligns with the PR's objective to migrate to async functions.
Also applies to: 284-287
299-299: Updated pageUid prop passing in ConfigPage component.With the changes to the render function, the uid variable is now correctly passed to the ConfigPage component.
312-318: Added async/await for getPageUidByPageTitle.The code now properly awaits the result of getPageUidByPageTitle, which has been converted to an async function as part of the migration.
335-338: Updated return statement with consistent formatting.The updated return statements maintain consistent behavior while adapting to the async changes.
| const result = await window.roamAlphaAPI.data.backend.q( | ||
| `[:find ?name :where[?e :user/display-name ?name] [?e :user/email "${email}"]]` | ||
| )?.[0]?.[0] as string) || ""; | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Security concern: Unsanitized input in query
The email parameter is directly interpolated into the query string without any sanitization, which could potentially lead to injection attacks if the email contains special characters that could modify the query.
Consider escaping special characters in the email parameter:
const getDisplayNameByEmail = async (email: string): Promise<string> => {
+ const sanitizedEmail = email.replace(/["\\]/g, '\\$&');
const result = await window.roamAlphaAPI.data.backend.q(
- `[:find ?name :where[?e :user/display-name ?name] [?e :user/email "${email}"]]`
+ `[:find ?name :where[?e :user/display-name ?name] [?e :user/email "${sanitizedEmail}"]]`
);
return (result?.[0]?.[0] as string) || "";
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const result = await window.roamAlphaAPI.data.backend.q( | |
| `[:find ?name :where[?e :user/display-name ?name] [?e :user/email "${email}"]]` | |
| )?.[0]?.[0] as string) || ""; | |
| ); | |
| const getDisplayNameByEmail = async (email: string): Promise<string> => { | |
| const sanitizedEmail = email.replace(/["\\]/g, '\\$&'); | |
| const result = await window.roamAlphaAPI.data.backend.q( | |
| `[:find ?name :where[?e :user/display-name ?name] [?e :user/email "${sanitizedEmail}"]]` | |
| ); | |
| return (result?.[0]?.[0] as string) || ""; | |
| }; |
| useEffect(() => { | ||
| getAllBlocks().then(setAllBlocks); | ||
| }, [getAllBlocks]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Error handling recommended for useEffect
While the implementation correctly uses useEffect to fetch data asynchronously when the component mounts or when getAllBlocks changes, it lacks error handling for the Promise.
- useEffect(() => {
- getAllBlocks().then(setAllBlocks);
- }, [getAllBlocks]);
+ useEffect(() => {
+ getAllBlocks()
+ .then(setAllBlocks)
+ .catch(error => {
+ console.error("Failed to load blocks:", error);
+ // Consider setting an error state or providing a fallback
+ });
+ }, [getAllBlocks]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| getAllBlocks().then(setAllBlocks); | |
| }, [getAllBlocks]); | |
| useEffect(() => { | |
| getAllBlocks() | |
| .then(setAllBlocks) | |
| .catch(error => { | |
| console.error("Failed to load blocks:", error); | |
| // Consider setting an error state or providing a fallback | |
| }); | |
| }, [getAllBlocks]); |
| export const getPageTitleByPageUid = async ( | ||
| pageUid: string | ||
| ): Promise<string> => { | ||
| const result = (await window.roamAlphaAPI.data.pull(`[:node/title]`, [ | ||
| ":block/uid", | ||
| pageUid, | ||
| ])) as PullBlock; | ||
| return result?.[":node/title"] || ""; | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Appropriate migration to async with API update
The function has been correctly converted to async and now uses the updated API method window.roamAlphaAPI.data.pull instead of window.roamAlphaAPI.pull. This is consistent with the migration to async pattern while also incorporating API updates.
🏁 Script executed:
#!/bin/bash
# Check if there are any other uses of the old API pattern that need to be updated
rg "roamAlphaAPI\.pull" --type tsLength of output: 480
API Migration Approved in this File – Additional Updates Needed Elsewhere
The changes in src/queries/getPageTitleByPageUid.ts correctly replace the old synchronous call with the async window.roamAlphaAPI.data.pull method. However, our repository-wide check indicates that the legacy API call (window.roamAlphaAPI.pull) is still in use in other files:
src/queries/getPageUidByPageTitle.tssrc/date/parseNlpDate.tstests/index.test.tsxsrc/components/FormDialog.tsx
Please review and update these usages as needed for consistency with the async migration.
| const result = (await window.roamAlphaAPI.data.backend.q( | ||
| `[:find (pull ?block [:block/uid :block/string]) :where [?block :block/string ?contents] [(clojure.string/includes? ?contents "${t}")]]` | ||
| )) as [PullBlock][]; |
There was a problem hiding this comment.
Ensure text parameter is properly escaped
The query uses the raw text parameter t directly in the query string, which could lead to injection vulnerabilities if t contains special characters like quotes or backslashes.
Consider sanitizing or escaping the text parameter:
const getBlockUidAndTextIncludingText = async (
t: string
): Promise<{ uid: string; text: string }[]> => {
+ const sanitizedText = t.replace(/"/g, '\\"').replace(/\\/g, '\\\\');
const result = (await window.roamAlphaAPI.data.backend.q(
- `[:find (pull ?block [:block/uid :block/string]) :where [?block :block/string ?contents] [(clojure.string/includes? ?contents "${t}")]]`
+ `[:find (pull ?block [:block/uid :block/string]) :where [?block :block/string ?contents] [(clojure.string/includes? ?contents "${sanitizedText}")]]`
)) as [PullBlock][];
return result.map(([block]) => ({
uid: block[":block/uid"] || "",
text: block[":block/string"] || "",
}));
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const result = (await window.roamAlphaAPI.data.backend.q( | |
| `[:find (pull ?block [:block/uid :block/string]) :where [?block :block/string ?contents] [(clojure.string/includes? ?contents "${t}")]]` | |
| )) as [PullBlock][]; | |
| const getBlockUidAndTextIncludingText = async ( | |
| t: string | |
| ): Promise<{ uid: string; text: string }[]> => { | |
| const sanitizedText = t.replace(/"/g, '\\"').replace(/\\/g, '\\\\'); | |
| const result = (await window.roamAlphaAPI.data.backend.q( | |
| `[:find (pull ?block [:block/uid :block/string]) :where [?block :block/string ?contents] [(clojure.string/includes? ?contents "${sanitizedText}")]]` | |
| )) as [PullBlock][]; | |
| return result.map(([block]) => ({ | |
| uid: block[":block/uid"] || "", | |
| text: block[":block/string"] || "", | |
| })); | |
| }; |
query builderworkbenchroamjs-componentssmartblocksSummary by CodeRabbit
Version Update
Asynchronous Improvements
UI/UX Enhancements