Skip to content

refactor: overhaul RecentSongs store architecture and improve Docker dev scripts (#87)#88

Open
tomast1337 wants to merge 2 commits intodevelopfrom
fix/#87
Open

refactor: overhaul RecentSongs store architecture and improve Docker dev scripts (#87)#88
tomast1337 wants to merge 2 commits intodevelopfrom
fix/#87

Conversation

@tomast1337
Copy link
Copy Markdown
Member

@tomast1337 tomast1337 commented Apr 26, 2026

Copilot is making a valid architectural point. Mixing infrastructure changes (Docker
This PR primarily restructures the RecentSongs state management to prioritize predictable data flow and resolve structural bugs related to React hydration and concurrent network requests. It moves away from a global singleton store to a Context-per-tree architecture to eliminate race conditions at the root.

Additionally, this PR includes developer experience (DX) improvements for the local Docker environment.

Closes #87.

Key Changes

Frontend Architecture (RecentSongs):

  • Replaced the global create() store with a RecentSongsContext. The store is now instantiated per-tree and hydrated synchronously with initialRecentSongs. This eliminates the useEffect initialization flash and hydration mismatches.
  • Fixed a desynchronization bug where the page counter would eagerly increment before the fetch completed. The page state is now treated strictly: it only updates upon a successful network response.
  • Concurrency and race conditions are now strictly handled by AbortController, seamlessly short-circuiting stale requests.
  • Removed the useRecentSongsCategoriesLoader auto-loader hook. Category fetching is now an explicit action invoked by consuming components.

Infrastructure & DX:

  • Added robust package.json scripts (docker:up, docker:down, docker:reset, docker:reset:fresh) to streamline local environment setup.
  • Integrated docker:minio-init directly into the startup sequence to ensure storage buckets are provisioned automatically.
  • Added healthcheck and profile behaviors for MongoDB, Maildev, and MinIO in Docker Compose to ensure dependent services are fully ready before the application boots.

…nd MinIO services, and update npm scripts for improved Docker management.
@tomast1337 tomast1337 changed the title fix/#87 refactor: overhaul RecentSongs store architecture to fix hydration and pagination (fixes #87) Apr 26, 2026
@tomast1337 tomast1337 marked this pull request as ready for review April 26, 2026 16:01
@tomast1337 tomast1337 requested review from Bentroen and Copilot April 26, 2026 16:01
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the frontend RecentSongs state management to use a per-tree Context-backed Zustand store, aiming to fix hydration flashes and pagination desync/race issues (issue #87), and also updates the local Docker Compose workflow to support compose up --wait with MinIO initialization.

Changes:

  • Replaced the global RecentSongs Zustand store with a Context-per-tree store, adding explicit actions for pagination, category selection, and category fetching.
  • Updated the browse home page components to consume the new recentItems feed model (song/ad/loading) and to explicitly trigger category loading.
  • Added Docker Compose healthchecks and new root scripts to support docker compose up --wait plus a one-shot MinIO bucket/CORS initializer profile.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
package.json Adds Docker helper scripts for compose up/down/reset and MinIO init.
docker-compose.yml Adds healthchecks and introduces a profile-based one-shot mc init service.
apps/frontend/src/modules/browse/components/client/context/RecentSongs.context.tsx Replaces singleton store with Context store; introduces feed item model + abort-based concurrency.
apps/frontend/src/modules/browse/components/client/context/HomePage.context.tsx Simplifies provider composition; removes legacy combined hook.
apps/frontend/src/modules/browse/components/client/CategoryButton.tsx Switches to selector-based store access and explicitly fetches categories.
apps/frontend/src/modules/browse/components/HomePageComponent.tsx Renders the new recentItems feed model and removes implicit loader hooks.
apps/frontend/src/app/(content)/page.tsx Adjusts initial recent-song fetch limit.
Comments suppressed due to low confidence (1)

docker-compose.yml:85

  • The mc service defines entrypoint: ['/bin/sh','-c'] but the command string also starts with -c '...'. That results in the container running /bin/sh -c "-c '...'", which typically tries to execute a command literally named -c and will fail, preventing MinIO bucket/CORS initialization. Remove the extra leading -c from command (or change the entrypoint to just /bin/sh) so the init script actually runs.
    entrypoint: ['/bin/sh', '-c']
    depends_on:
      minio:
        condition: service_healthy
    environment:
      - MINIO_ROOT_USER=minioadmin
      - MINIO_ROOT_PASSWORD=minioadmin
    command: >
      -c '
      while ! mc alias set minio http://minio:9000 minioadmin minioadmin; do
        echo "Waiting for MinIO to be available..."

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread package.json
@tomast1337 tomast1337 changed the title refactor: overhaul RecentSongs store architecture to fix hydration and pagination (fixes #87) refactor: overhaul RecentSongs store architecture and improve Docker dev scripts (#87) Apr 26, 2026
… pagination

- Migrate singleton Zustand store to React Context to allow synchronous SSR hydration and prevent initial render flashes.
- Fix pagination desynchronization by incrementing the page state only after a successful network response.
- Remove redundant ID-based concurrency checks, relying strictly on AbortController for deterministic network cancellation.
- Delete implicit `useRecentSongsCategoriesLoader` side-effect hook in favor of explicit component-level data fetching.

Fixes #87
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +171 to +177
const didLoad = await fetchRecentSongs(nextPage);
if (!didLoad) {
set((state) => ({
recentItems: state.recentItems.filter(
(item) => item.type !== 'loading',
),
}));
Copy link

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

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

increasePageRecent treats any fetchRecentSongs failure the same, but fetchRecentSongs returns false both for real errors and for intentional aborts. If a user changes category while a “load more” request is in-flight, the aborted increasePageRecent path can run this cleanup and strip the new category’s loading placeholders (and potentially cause a brief empty/flash state). Consider distinguishing aborted vs failed (e.g., return a status enum or throw on abort) and/or only removing the specific placeholders appended by this call (track count/requestId and validate selectedCategory/page before mutating state).

Copilot uses AI. Check for mistakes.
Comment thread package.json
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.

Recent songs stuck in loading state after filtering by category

2 participants