refactor(library): fix SSR bug by removing data fetch from init function

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.
2026-03-12 17:58:36 -04:00
parent ddcd8c62e2
commit 1b9bc64b28
2 changed files with 152 additions and 86 deletions
@@ -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", () => ({
 <body x-data="docs" x-init="initializeSearch" class="theme-{ user.Theme }">
 ```

-### 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 `<body>` tag** and add x-data and x-init:
+**Template Update:** `templates/admin_library.templ`

+**Current state (line 11):**
 ```html
-<!-- BEFORE -->
-<body class="theme-{ user.Theme }">
-
-<!-- AFTER -->
-<body class="theme-{ user.Theme }" x-data="library" x-init="reloadLibraries">
+<body x-data="library" x-init="initializeLibraryAdmin" class="theme-{ user.Theme }">
 ```

+**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
@@ -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,
 }));