Skip to content

feat(conversation): add onLoad callback for initial conversation list loading#304

Merged
hexqi merged 2 commits intoopentiny:developfrom
gene9831:feature/conversation-updates
Feb 12, 2026
Merged

feat(conversation): add onLoad callback for initial conversation list loading#304
hexqi merged 2 commits intoopentiny:developfrom
gene9831:feature/conversation-updates

Conversation

@gene9831
Copy link
Collaborator

@gene9831 gene9831 commented Feb 11, 2026

Summary by CodeRabbit

Release Notes

  • New Features
    • Introduced a new optional callback that triggers after conversations are loaded, enabling applications to react to the completion event and access the loaded conversation data for further processing.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 11, 2026

✅ Preview build completed successfully!

Click the image above to preview.
Preview will be automatically removed when this PR is closed.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 11, 2026

@coderabbitai
Copy link

coderabbitai bot commented Feb 11, 2026

Walkthrough

An optional onLoad callback has been added to the conversation use hook that triggers after the initial conversation list loads from storage. The load flow now returns the loaded conversations instead of void, enabling external code to react to loaded data.

Changes

Cohort / File(s) Summary
Type Definitions
packages/kit/src/vue/conversation/types.ts
Added optional onLoad callback property to UseConversationOptions interface, invoked with loaded conversations after storage completes.
Implementation
packages/kit/src/vue/conversation/useConversation.ts
Centralized localStorageStrategyFactory import, refactored load-conversations flow to return loaded/merged lists instead of void, and integrated onLoad callback invocation with the loaded list.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A callback awakens when conversations arrive,
From storage's depths, the list comes alive,
OnLoad whispers secrets of chats gone before,
Now external eyes can see what was stored!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding an onLoad callback to handle initial conversation list loading, which is the primary feature added across both modified files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/kit/src/vue/conversation/useConversation.ts (1)

116-149: ⚠️ Potential issue | 🟡 Minor

onLoad receives the merged list in the merge branch, not the storage-loaded list.

When in-memory conversations already exist (the merge path on Line 134–140), the callback receives conversations.value (the merged result), not the original list from storage. This contradicts the JSDoc which says "Called when the initial conversation list has been loaded from storage."

If the intent is to always pass the conversations as loaded from storage (i.e., list), every branch should return list. If the intent is to pass the final state after load, the JSDoc/param name should be updated accordingly (e.g., rename to something like onReady).

Option A: Always pass the storage-loaded list
       if (!list?.length) {
-          return []
+          return list ?? []
        }

        if (conversations.value.length === 0) {
          conversations.value = list
-          return conversations.value
+          return list
        }

        // merge logic …
        conversations.value = Array.from(merged.values())
-        return conversations.value
+        return list
      })
Option B: Keep current behavior but update docs

Update the JSDoc in types.ts:

   /**
-   * Called when the initial conversation list has been loaded from storage.
-   * Only fired when storage.loadConversations is available and has completed.
+   * Called after the initial conversation list has been loaded from storage and merged
+   * with any in-memory conversations. Receives the final conversation list.
+   * Only fired when storage.loadConversations is available and has completed.
    */
-  onLoad?: (conversations: ConversationInfo[]) => void
+  onLoad?: (conversations: ConversationInfo[]) => void
🧹 Nitpick comments (1)
packages/kit/src/vue/conversation/useConversation.ts (1)

147-149: onLoad is silently skipped on storage errors — consider whether this is intentional.

If loadConversations() rejects, the .catch on Line 150 logs the error but onLoad is never invoked. Consumers have no way to know loading finished (with failure). If this is intentional, a brief comment would help; otherwise, consider calling onLoad?.([]) in the catch block so consumers always get a signal.

@hexqi hexqi merged commit 84bbfa5 into opentiny:develop Feb 12, 2026
4 checks passed
@github-actions
Copy link
Contributor

🧹 Preview Cleaned Up

The preview deployment has been removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments