From 1b9bc64b2860f4ae27df989fa57162d964b70e69 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Thu, 12 Mar 2026 17:58:36 -0400 Subject: [PATCH] refactor(library): fix SSR bug by removing data fetch from init function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CRITICAL FIX: initializeLibraryAdmin() was calling reloadLibraries() which fetched data from the API and replaced the SSR-rendered library list on page load, defeating the purpose of server-side rendering. Changes in web/src/library.ts: - Remove DOMContentLoaded listener (now uses Alpine x-init in template) - Remove void reloadLibraries() call from initializeLibraryAdmin() - Add comment explaining SSR provides initial data - Add initializeLibraryAdmin to export statement - Add initializeLibraryAdmin to Alpine.data() registration - Keep reloadLibraries() as standalone function for use after CRUD ops Rationale: - SSR provides fast initial page load with library list - x-init should ONLY setup event listeners, not fetch data - reloadLibraries() is called after create/delete/update operations - Follows SSR-first architecture: different pages have different SSR/JS ratios (analytics is 80% JS, most pages are 80% SSR) Documentation: - Update COLLECTIONS_CLEANUP_GUIDE.md with SSR-first strategy - Document page-by-page review status (dashboard βœ“, collections πŸ”„) - Fix template references (library.templ β†’ admin_library.templ) - Explain why analytics fetches data (intentional for dynamic page) This ensures the admin library page maintains SSR benefits while still providing interactive features via Alpine.js. --- COLLECTIONS_CLEANUP_GUIDE.md | 181 ++++++++++++++++++++++++----------- web/src/library.ts | 57 +++++------ 2 files changed, 152 insertions(+), 86 deletions(-) diff --git a/COLLECTIONS_CLEANUP_GUIDE.md b/COLLECTIONS_CLEANUP_GUIDE.md index b590a51..7bc6292 100644 --- a/COLLECTIONS_CLEANUP_GUIDE.md +++ b/COLLECTIONS_CLEANUP_GUIDE.md @@ -1,11 +1,52 @@ -# Collections Cleanup: Fix Alpine.js and DOMContentLoaded Issues +# Alpine.js and SSR Architecture Cleanup Guide ## Overview -This guide fixes console errors caused by: +This guide provides a systematic, page-by-page approach to fixing Alpine.js issues while maintaining SSR-first architecture. + +### Problem Statement + +Console errors caused by: 1. Dead Alpine.js exports (functions that don't exist) 2. DOMContentLoaded listeners running on wrong pages 3. Missing x-init calls in templates +4. **CRITICAL:** x-init functions that fetch data and replace SSR content + +### SSR-First Alpine.js Strategy + +**Different pages have different SSR/JavaScript ratios:** + +- **Analytics Page (80% JavaScript):** Dynamic charts/data that fetch on page load + - βœ… `x-init="loadAnalytics"` fetches data and replaces content + - βœ… This is intentional - analytics is a JavaScript-heavy page + +- **Dashboard Page (SSR + Interactive):** SSR for speed, JS for drag-drop/library switching + - βœ… SSR provides initial dashboard data + - βœ… x-init ONLY sets up event listeners (drag-drop, modals) + - βœ… x-init does NOT fetch data (data already SSR'd) + - βœ… `switchLibrary()` fetches only when user changes library dropdown + +- **Admin Library Page (SSR + Modals/CRUD):** SSR list, JS for management + - βœ… SSR provides initial library list + - βœ… x-init ONLY sets up event listeners and modals + - ❌ BUG: `initializeLibraryAdmin()` was calling `reloadLibraries()` which fetches data + - ❌ This replaces the SSR content on page load - MUST FIX + +- **Most Pages (80% SSR):** Server-rendered with interactive areas + - βœ… Use x-init for setup only (event listeners, modals) + - ❌ NEVER fetch data in x-init if data is already SSR'd + +**Key Principle:** +- βœ… DO in x-init: Set up event listeners, initialize modals, setup drag-drop +- ❌ DON'T in x-init: Fetch data via API, replace SSR innerHTML + +### Review Status + +**Pages methodically reviewed so far:** +- βœ… **Dashboard** - Completed, working correctly +- πŸ”„ **Collections** - In progress (fixes in progress) +- ⏸️ **Admin Library** - Identified bug, fix documented in this guide +- ⏸️ **Other pages** - Not yet reviewed, will be done page-by-page **Goal:** Clean, working Alpine.js integration with proper SSR architecture. @@ -326,44 +367,61 @@ Alpine.data("docs", () => ({ ``` -### Step 3.3: library.ts +### Step 3.3: library.ts - SSR-FIRST FIX REQUIRED + +**⚠️ CRITICAL SSR BUG:** The `initializeLibraryAdmin()` function currently calls `reloadLibraries()` which fetches data from the API and replaces the SSR-rendered library list. This defeats SSR for the admin library page! **File:** `web/src/library.ts` -**Current (around line 655-662):** +**Problem (around line 653-655):** ```typescript -// Initialize on DOM ready -if (document.readyState === "loading") { - document.addEventListener("DOMContentLoaded", initializeLibraryAdmin); -} else { - initializeLibraryAdmin(); +function initializeLibraryAdmin(): void { + // Setup event listeners + const librariesList = document.getElementById("libraries-list"); + if (librariesList) { + librariesList.addEventListener("click", handleLibraryListClick); + } + + // ... more setup ... + + // ❌ BUG: This fetches data and replaces SSR content on page load! + void reloadLibraries(); } - -// ... later in file ... ``` -**Change to:** +**Fix - Remove data fetch from init:** ```typescript -// REMOVE this entire block: -// if (document.readyState === "loading") { -// document.addEventListener("DOMContentLoaded", initializeLibraryAdmin); -// } else { -// initializeLibraryAdmin(); -// } +function initializeLibraryAdmin(): void { + // Setup event listeners + const librariesList = document.getElementById("libraries-list"); + if (librariesList) { + librariesList.addEventListener("click", handleLibraryListClick); + } + + // ... keep all existing setup code ... + + // βœ… REMOVED: void reloadLibraries(); + // SSR provides initial library list - no need to fetch on page load + // reloadLibraries() is still available to call AFTER create/delete/update operations +} ``` -**Template Update:** `templates/library.templ` +**Why this fix matters:** +- SSR renders the library list on the server for fast initial load +- Calling `reloadLibraries()` in x-init replaces this with a slower API call +- `reloadLibraries()` should ONLY be called after CRUD operations (create/delete/update) +- The `initializeLibraryAdmin()` should ONLY setup event listeners and modals -**Find the `` tag** and add x-data and x-init: +**Template Update:** `templates/admin_library.templ` +**Current state (line 11):** ```html - - - - - + ``` +**Status:** βœ… Already correct - template already has x-data and x-init setup +**Action:** No template change needed - just fix library.ts to not fetch data in init + ### Step 3.4: dashboard.ts **File:** `web/src/dashboard.ts` @@ -556,48 +614,61 @@ go build ./cmd/server ## Summary -### Changes Made +### Progress So Far -**TypeScript Files (5 files):** -- `web/src/collections.ts` - Removed dead exports -- `web/src/analytics.ts` - Removed DOMContentLoaded -- `web/src/docs.ts` - Removed DOMContentLoaded -- `web/src/library.ts` - Removed DOMContentLoaded -- `web/src/dashboard.ts` - Created initDashboard wrapper, removed DOMContentLoaded +**Pages methodically reviewed:** +- βœ… **Dashboard** - Completed (drag-drop works, SSR maintained) +- πŸ”„ **Collections** - In progress (dead exports removed, more fixes needed) +- ⏸️ **Admin Library** - SSR bug identified, fix documented in Step 3.3 +- ⏸️ **All other pages** - Not yet reviewed, will be done page-by-page -**Template Files (5 files):** -- `templates/analytics.templ` - Added x-init="loadAnalytics" -- `templates/docs.templ` - Added x-init="initializeSearch" -- `templates/dashboard.templ` - Added x-data="dashboard" x-init="initDashboard()" -- `templates/library.templ` - Added x-data="library" x-init="reloadLibraries" -- `templates/collections.templ` - Removed dead function calls +### Completed Changes -**Generated Files:** -- All `_templ.go` files regenerated +**TypeScript Files (3 files):** +- `web/src/collections.ts` - βœ… Removed dead Alpine.js exports +- `web/src/analytics.ts` - βœ… Removed DOMContentLoaded, SSR-first (intentional data fetch) +- `web/src/admin.ts` - βœ… Removed DOMContentLoaded, WebSocket moved to template + +**Template Files (1 file):** +- `templates/analytics.templ` - βœ… Added x-init="loadAnalytics" (JS-heavy page, correct) + +### Pending Critical Fixes + +**SSR Bug in library.ts (Step 3.3):** +- ❌ `initializeLibraryAdmin()` calls `reloadLibraries()` which fetches data and replaces SSR +- βœ… Fix documented: Remove data fetch from init, only setup event listeners +- ⚠️ Template already correct: `templates/admin_library.templ` has x-data/x-init ### What Was NOT Changed - **"Add Books" modal** - Still client-side Alpine.js (quick fix decision) -- **WebSocket code** - Already moved to templates (previous commit) +- **WebSocket code** - Already moved to templates with server-side token injection - **API endpoints** - Already exist and work correctly - **HTMX modals** - Already implemented for collection CRUD +- **Search** - Skipped pending user's planned revamp -### Result +### SSR-First Principles Applied -βœ… No more Alpine.js errors -βœ… No more DOMContentLoaded pollution -βœ… Pages run only their own initialization code -βœ… Console is clean -βœ… SSR architecture maintained +βœ… **Analytics (80% JS):** x-init fetches data - intentional for dynamic page +βœ… **Dashboard (SSR + JS):** x-init ONLY sets up event listeners, no data fetch +βœ… **Admin Library:** Will fix to only setup event listeners, SSR provides data +βœ… **Collections:** SSR provides data, JS for interactivity (in progress) -### Next Steps (Optional) +### Result So Far -If you want to convert the "Add Books" modal to HTMX (future enhancement), that would require: +βœ… Collections dead exports removed +βœ… Analytics loads correctly with SSR-first approach +βœ… Admin WebSocket fixed with server-side token injection +⏳ Admin library SSR bug identified, fix ready to implement +⏳ Other pages will be reviewed methodically, one by one -1. Create `templates/add_books_modal.templ` -2. Add route in `internal/router/collections.go` -3. Create handler in `internal/handlers/collections.go` -4. Update `collections.templ` to use HTMX modal instead of inline -5. Remove all book selection JavaScript from `collections.ts` +### Next Steps -But for now, the client-side approach works fine. +**Immediate (Critical SSR Bug):** +1. Fix `web/src/library.ts` Step 3.3 - Remove `reloadLibraries()` from `initializeLibraryAdmin()` +2. Test admin library page to verify SSR content is not replaced + +**Continue Page-by-Page Review:** +3. Complete collections page fixes +4. Review and fix remaining pages one at a time +5. Update this guide as each page is completed diff --git a/web/src/library.ts b/web/src/library.ts index 644f8ea..43eb437 100644 --- a/web/src/library.ts +++ b/web/src/library.ts @@ -650,50 +650,45 @@ function initializeLibraryAdmin(): void { createLibraryForm.addEventListener("submit", handleCreateLibrarySubmit); } - // Load libraries from API on page load - void reloadLibraries(); -} - -// Initialize on DOM ready -if (document.readyState === "loading") { - document.addEventListener("DOMContentLoaded", initializeLibraryAdmin); -} else { - initializeLibraryAdmin(); + // SSR provides initial library list - no need to fetch on page load + // reloadLibraries() is called after create/delete/update operations } // Export functions for global access export { - deleteLibrary, - showLibraryFolders, addLibraryFolder, - removeLibraryFolder, - setLibraryVisibility, - loadUserVisibility, + confirmDeleteLibrary, + deleteLibrary, editLibrary, handleCreateLibrarySubmit, - showFolderBrowser, - navigateFolderBrowser, - selectBrowseFolder, - hideFolderBrowser, - showDeleteModal, hideDeleteModal, - confirmDeleteLibrary, + hideFolderBrowser, + initializeLibraryAdmin, + loadUserVisibility, + navigateFolderBrowser, + removeLibraryFolder, + selectBrowseFolder, + setLibraryVisibility, + showDeleteModal, + showFolderBrowser, + showLibraryFolders, }; Alpine.data("library", () => ({ - deleteLibrary, - showLibraryFolders, addLibraryFolder, - removeLibraryFolder, - setLibraryVisibility, - loadUserVisibility, + confirmDeleteLibrary, + deleteLibrary, editLibrary, handleCreateLibrarySubmit, - showFolderBrowser, - navigateFolderBrowser, - selectBrowseFolder, - hideFolderBrowser, - showDeleteModal, hideDeleteModal, - confirmDeleteLibrary, + hideFolderBrowser, + initializeLibraryAdmin, + loadUserVisibility, + navigateFolderBrowser, + removeLibraryFolder, + selectBrowseFolder, + setLibraryVisibility, + showDeleteModal, + showFolderBrowser, + showLibraryFolders, }));