fix: Fix loading queues in the background - #88
Conversation
Use a phantom queue to temporarily load songs while the actual queue is resolved in the background.
📝 WalkthroughWalkthroughAdds a public ChangesBackground queue
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/queue.rs (1)
37-39: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd a stable key to each queue item.
Use a unique
song.idas the item key. The queue changes between phantom and loaded entries, and it may reorder. Without a stable key, Dioxus cannot reliably preserve item identity across renders. The Dioxus 0.7 documentation requires stable keys for list items. (dioxuslabs.com)Proposed fix
for song in display_songs.iter() { - div { class: "queue-item", "{song.title} - {song.artist}" } + div { + key: "{song.id}", + class: "queue-item", + "{song.title} - {song.artist}" + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/queue.rs` around lines 37 - 39, Update the queue item loop in the display_songs rendering to assign each item a stable unique key derived from song.id. Preserve the existing title-and-artist content while ensuring Dioxus can retain item identity when entries load or reorder.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/queue.rs`:
- Around line 24-28: Replace the placeholder vector inside the use_resource
closure with the actual queue-loading operation, locating and returning the
songs currently queued before the component merges or renders them. Remove the
phantom Song::phantom entries and preserve the resource’s async resolution
behavior.
- Around line 30-33: Update the queue data handling around queue_data to store
owned Song values rather than Vec<&Song> references. Convert returned songs into
owned values in the Some branch, and collect owned Song::phantom results in the
None branch, ensuring no references outlive the temporary data.
---
Nitpick comments:
In `@src/queue.rs`:
- Around line 37-39: Update the queue item loop in the display_songs rendering
to assign each item a stable unique key derived from song.id. Preserve the
existing title-and-artist content while ensuring Dioxus can retain item identity
when entries load or reorder.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| let queue_data = use_resource(move || async move { | ||
| // TODO: Replace with actual queue fetching logic | ||
| // Placeholder resolves to real songs once the background task completes | ||
| vec![Song::phantom("real-1"), Song::phantom("real-2")] | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Load the actual queue before merging.
The resource returns two phantom songs immediately. It never locates the actual queued songs. After resolution, the component still renders fake songs, so the phantom queue is not temporary. Replace the TODO with the real queue-loading operation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/queue.rs` around lines 24 - 28, Replace the placeholder vector inside the
use_resource closure with the actual queue-loading operation, locating and
returning the songs currently queued before the component merges or renders
them. Remove the phantom Song::phantom entries and preserve the resource’s async
resolution behavior.
| let display_songs: Vec<&Song> = match queue_data() { | ||
| Some(ref songs) => songs.iter().collect(), | ||
| None => (0..3).map(|i| &Song::phantom(&i.to_string())).collect(), | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Store owned songs instead of references to temporaries.
Some(ref songs) borrows from the temporary value returned by queue_data(). The None branch also returns references to temporary Song::phantom values. These references do not live long enough, so this function cannot compile.
Proposed fix
- let display_songs: Vec<&Song> = match queue_data() {
- Some(ref songs) => songs.iter().collect(),
- None => (0..3).map(|i| &Song::phantom(&i.to_string())).collect(),
+ let display_songs: Vec<Song> = match queue_data() {
+ Some(songs) => songs,
+ None => (0..3).map(|i| Song::phantom(&i.to_string())).collect(),
};📝 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.
| let display_songs: Vec<&Song> = match queue_data() { | |
| Some(ref songs) => songs.iter().collect(), | |
| None => (0..3).map(|i| &Song::phantom(&i.to_string())).collect(), | |
| }; | |
| let display_songs: Vec<Song> = match queue_data() { | |
| Some(songs) => songs, | |
| None => (0..3).map(|i| Song::phantom(&i.to_string())).collect(), | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/queue.rs` around lines 30 - 33, Update the queue data handling around
queue_data to store owned Song values rather than Vec<&Song> references. Convert
returned songs into owned values in the Some branch, and collect owned
Song::phantom results in the None branch, ensuring no references outlive the
temporary data.
1 participant
Closes #85
Summary by CodeRabbit