Roam: Remove flag and use async.q #90
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe changes eliminate the "Use Backend Query (Beta)" checkbox and its related state logic in the settings component. In addition, the query execution function in the query utility has been simplified by removing the conditional check for asynchronous queries, directly invoking the asynchronous query call via the Roam API. Redundant import and debug logging statements have also been removed. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User Code
participant FQ as fireQuery Function
participant API as Roam API
U->>FQ: Call fireQuery with query parameters
FQ->>API: Execute query via window.roamAlphaAPI.data.async.q(query)
API-->>FQ: Return query results or errors
FQ-->>U: Return the query results
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/roam/src/utils/fireQuery.ts (1)
338-338: Address TODO: Add TypeScript types for async.q.The TODO comment indicates missing TypeScript types for
async.qin roamjs-components.Would you like me to help create an issue to track adding the TypeScript types for
async.qto roamjs-components?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/roam/src/components/settings/QuerySettings.tsx(0 hunks)apps/roam/src/utils/fireQuery.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/roam/src/components/settings/QuerySettings.tsx
🔇 Additional comments (3)
apps/roam/src/utils/fireQuery.ts (3)
331-331: LGTM! Simplified logging.The logging statement has been streamlined while maintaining essential query debugging information.
339-342: LGTM! Simplified query execution.The code now directly uses the async query API, removing the conditional logic for async vs sync queries. This simplification makes the code more maintainable and easier to understand.
305-356:❓ Verification inconclusive
Verify the impact of removing the query flag.
The changes standardize on using async queries, which is a good simplification. However, we should verify that:
- All consumers are ready for async-only queries
- There are no performance implications from always using async queries
Run the following script to find potential consumers that might be affected:
🏁 Script executed:
#!/bin/bash # Search for direct usage of fireQuery rg "fireQuery\(" --type ts # Search for potential sync query usage patterns rg "fireQuerySync" --type tsLength of output: 1167
Ensure consistent usage of async queries across consumers.
The updated implementation in
apps/roam/src/utils/fireQuery.tssimplifies the logic by standardizing on async queries and removing the feature flag complexity. Our codebase search confirms that most consumers call the asyncfireQuery(e.g., inrunQuery.ts,getExportTypes.ts,QueryBuilder.tsx,QueryDrawer.tsx, andLabelDialog.tsx). However, there is still a synchronous version (fireQuerySync) in use (notably inindex.tsandregisterDiscourseDatalogTranslators.ts).Please verify that:
- All consumers intended to use async queries are compatible and have been updated to handle async behavior.
- The synchronous usage via
fireQuerySyncis deliberate and won’t lead to inconsistencies or missed optimizations.- There are no performance implications from unconditionally using async queries, especially in development logging contexts.
Summary by CodeRabbit
Bug Fixes
Refactor