ENG-744 Overlay Init Bug Fix #342
Conversation
…ndListeners.ts to ensure config tree refresh occurs after relevant setup steps.
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughMoved initializeDiscourseNodes() and refreshConfigTree() earlier in the Roam app onload sequence and removed their later duplicate call. In observers/listeners setup, added an immediate refreshConfigTree() after creating the Discourse config observer. No public API changes. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Roam App
participant Discourse as Discourse Init
participant Config as Config Tree
participant UI as Graph View Styling
participant Obs as Observers/Listeners
App->>App: onload()
App->>App: showDevToast()
App->>Discourse: initializeDiscourseNodes()
App->>Config: refreshConfigTree()
App->>UI: addGraphViewNodeStyling()
App->>Obs: initializeObserversAndListeners()
sequenceDiagram
participant Init as initializeObserversAndListeners()
participant CObs as Config Observer
participant Config as Config Tree
participant Router as Hash-change Handler
Init->>CObs: create Discourse config observer
Init->>Config: refreshConfigTree() (immediate)
Init->>Router: setup hash-change handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 0
🧹 Nitpick comments (1)
apps/roam/src/index.ts (1)
89-91: Optional: await refreshConfigTree for future-proofing
Looking atapps/roam/src/utils/refreshConfigTree.ts,refreshConfigTreeis currently a synchronous function (const refreshConfigTree = () => { … }). Addingawaittoday is a no-op, but it’s safe and ensures the correct ordering if it ever becomes async in the future.Recommended change in
apps/roam/src/index.ts:await initializeDiscourseNodes(); - refreshConfigTree(); + await refreshConfigTree();[optional_refactors_recommended]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/roam/src/index.ts(1 hunks)apps/roam/src/utils/initializeObserversAndListeners.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-23T11:49:45.457Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#220
File: apps/roam/src/utils/conceptConversion.ts:11-40
Timestamp: 2025-06-23T11:49:45.457Z
Learning: In the DiscourseGraphs/discourse-graph codebase, direct access to `window.roamAlphaAPI` is the established pattern throughout the codebase. The team prefers to maintain this pattern consistently rather than making piecemeal changes, and plans to address dependency injection as a global refactor when scheduled.
Applied to files:
apps/roam/src/index.ts
📚 Learning: 2025-06-22T10:40:21.679Z
Learnt from: sid597
PR: DiscourseGraphs/discourse-graph#232
File: apps/roam/src/utils/getAllDiscourseNodesSince.ts:15-16
Timestamp: 2025-06-22T10:40:21.679Z
Learning: In the getAllDiscourseNodesSince function in apps/roam/src/utils/getAllDiscourseNodesSince.ts, date validation is performed before the function is called, so additional date validation within the function is not needed.
Applied to files:
apps/roam/src/index.ts
🔇 Additional comments (2)
apps/roam/src/utils/initializeObserversAndListeners.ts (1)
149-150: No changes needed:refreshConfigTreeis synchronousThe
refreshConfigTreefunction is declared as a plain arrow function (noasynckeyword orPromisereturn) and contains only synchronous operations. There’s no need toawaitit—leaving the calls as-is is correct.apps/roam/src/index.ts (1)
89-91: Moving initialization earlier addresses the overlay init raceInitializing discourse nodes and refreshing config before wiring observers/listeners is the right direction for fixing the overlay init bug; removing the later duplicate call reduces redundant work. No API surface changes—LGTM.
Introduced in #288
Summary by CodeRabbit