should prevent issues when loading website from cache#171
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAsync session refresh and project-cache validation were added for tab-visibility and bfcache restore events. A new bfcache handler initializes on startup; auth refresh logic became awaitable and a public forceRefreshSession() was exposed. Project list cache gained a validation utility. Changes
Sequence DiagramssequenceDiagram
participant Browser as Browser (bfcache restore)
participant Main as main.jsx (App Init)
participant Handler as bfcacheHandler
participant Auth as betterAuthStore
participant Project as projectStore
Note over Browser: Page restored from bfcache
Browser->>Handler: pageshow (event, persisted=true)
activate Handler
Handler->>Auth: wait for auth loading (<=5s)
activate Auth
Auth-->>Handler: auth ready / timed out
deactivate Auth
Handler->>Auth: forceRefreshSession()
activate Auth
Auth->>Auth: session().refetch()
Auth-->>Handler: session refreshed / error
deactivate Auth
Handler->>Project: validateProjectListCache(userId)
activate Project
alt user authenticated
Project->>Project: refresh project list
else unauthenticated
Project->>Project: clear project list & localStorage
end
Project-->>Handler: complete
deactivate Project
Handler->>Handler: remove listener (cleanup)
deactivate Handler
sequenceDiagram
participant Tab as Browser Tab (visibility)
participant Store as betterAuthStore
participant Auth as session()
participant Project as projectStore
Note over Tab: Tab becomes visible
Tab->>Store: visibilitychange (visible)
activate Store
Store->>Auth: await session().refetch()
activate Auth
Auth-->>Store: session data / error
deactivate Auth
Store->>Store: wait 100ms
alt user authenticated
Store->>Project: validateProjectListCache(userId)
activate Project
Project->>Project: refresh project list
Project-->>Store: done
deactivate Project
else no user
Store->>Project: clear project list
activate Project
Project-->>Store: cleared
deactivate Project
end
deactivate Store
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ⛔ Deployment terminated View logs |
corates | ebff60c | Commit Preview URL | Dec 26 2025, 09:41 PM |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/web/src/lib/bfcache-handler.js (3)
21-76: Code duplication with visibility change handler.This handler logic (lines 51-76) is nearly identical to
handleVisibilityChangeinbetter-auth-store.js(lines 198-226). Both:
- Force refresh the auth session
- Wait 100ms
- Call
validateProjectListCachethenrefreshProjectListif authenticated- Call
clearProjectListif not authenticatedConsider extracting shared logic into a reusable helper to reduce duplication and ensure consistent behavior.
Proposed helper extraction
// In a shared utility, e.g., @lib/auth-refresh-utils.js export async function refreshAuthAndProjectState(auth, projectStore) { try { await auth.forceRefreshSession(); } catch (err) { console.warn('[auth] Failed to refresh session:', err); } await new Promise(resolve => setTimeout(resolve, 100)); const currentUser = auth.user(); if (currentUser?.id) { projectStore.validateProjectListCache(currentUser.id); try { await projectStore.refreshProjectList(currentUser.id); } catch (err) { console.warn('[auth] Failed to refresh project list:', err); } } else { projectStore.clearProjectList(); } }
58-59: Magic delay without clear justification.The 100ms delay after
forceRefreshSessionlacks documentation explaining why it's needed. If the session signal updates synchronously afterrefetch()resolves, this delay may be unnecessary. If it's required for a specific reason (e.g., reactive propagation), consider adding a brief comment.
64-72: Potentially redundant cache validation before refresh.
validateProjectListCacheclears the cache if user IDs don't match, butrefreshProjectListimmediately fetches fresh data and overwrites the cache anyway. The validation may still be useful to prevent a brief flash of stale data before the refresh completes, but consider whether calling both is necessary or ifrefreshProjectListcould handle the user ID mismatch internally.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/web/src/api/better-auth-store.jspackages/web/src/lib/bfcache-handler.jspackages/web/src/main.jsxpackages/web/src/stores/projectStore.js
🧰 Additional context used
📓 Path-based instructions (14)
**/*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use emojis in code, comments, documentation, or commit messages
NEVER use emojis anywhere in code, comments, documentation, plan files, or commit messages. This includes unicode symbols. For UI icons, use solid-icons library or SVGs only.
Files:
packages/web/src/lib/bfcache-handler.jspackages/web/src/main.jsxpackages/web/src/api/better-auth-store.jspackages/web/src/stores/projectStore.js
packages/web/src/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/web/src/**/*.{js,jsx,ts,tsx}: For UI icons, use thesolid-iconslibrary or SVGs only. Do not use emojis
Ensure browser compatibility for all frontend code (Safari is usually problematic)
Keep files small, focused, and modular. If a file exceeds a high number of lines, consider refactoring by extracting sub-modules into a folder with index.jsx and helper components, moving complex logic into separate utility files or primitives, or splitting large forms into section components
Do NOT prop-drill application state. Shared or cross-feature state must live in external stores under packages/web/src/stores/ or relative to the component file
UsecreateMemofor derived values to ensure they update reactivelyUse import aliases from jsconfig.json instead of relative paths
Files:
packages/web/src/lib/bfcache-handler.jspackages/web/src/main.jsxpackages/web/src/api/better-auth-store.jspackages/web/src/stores/projectStore.js
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features in JavaScript/TypeScript code
Comments should explain why something is being done, not narrate what the code does. Avoid comments that repeat variable names or describe obvious code behavior.
Files:
packages/web/src/lib/bfcache-handler.jspackages/web/src/main.jsxpackages/web/src/api/better-auth-store.jspackages/web/src/stores/projectStore.js
packages/web/src/**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)
In SolidJS, use createMemo for derived values
Files:
packages/web/src/lib/bfcache-handler.jspackages/web/src/main.jsxpackages/web/src/api/better-auth-store.jspackages/web/src/stores/projectStore.js
packages/web/src/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
packages/web/src/**/*.{js,ts,jsx,tsx}: Always usehandleFetchErrorfrom@/lib/error-utils.jsfor fetch calls in frontend code with options like{ showToast: true }for error handling
UsecreateFormErrorSignalsfrom@/lib/form-errors.jsfor form validation error handling with field-level and global error management
Files:
packages/web/src/lib/bfcache-handler.jspackages/web/src/main.jsxpackages/web/src/api/better-auth-store.jspackages/web/src/stores/projectStore.js
packages/{web,workers}/src/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
packages/{web,workers}/src/**/*.{js,ts,jsx,tsx}: Never throw string literals; always throw Error objects or return domain errors from API routes
Use error utility functions likeisErrorCodefrom@corates/sharedor@/lib/error-utils.jsto check specific error types instead of manual string comparisons
Files:
packages/web/src/lib/bfcache-handler.jspackages/web/src/main.jsxpackages/web/src/api/better-auth-store.jspackages/web/src/stores/projectStore.js
{packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.cursor/rules/solidjs.mdc)
{packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts}: Never destructure props in SolidJS components - destructuring breaks reactivity. Access props directly (e.g.,props.name) or wrap in a function (e.g.,const name = () => props.name) to maintain reactivity.
Import stores directly in components rather than prop-drilling store data through component hierarchies.
Use separate read and write patterns for stores: import the store directly for reading data (e.g.,projectStore.getProjectList()) and import action stores separately for writing (e.g.,projectActionsStore.createProject()).
UsecreateSignalfrom solid-js for managing simple reactive values. Prefer derived state with signals or memo over effects when possible.
UsecreateStorefrom solid-js/store for managing complex objects and arrays that require granular reactivity, enabling fine-grained updates where only affected parts re-render.
UsecreateMemofrom solid-js for derived values that depend on reactive state, ensuring computed values update only when their dependencies change.
Always clean up effects that create subscriptions or timers using theonCleanupfunction from solid-js. Use effects sparingly, only when derived values won't work well.
Keep components lean and focused on rendering. Move business logic to stores (for shared state and operations), primitives (for reusable hooks/logic), or utilities (for pure functions).
Use theShowcomponent from solid-js for conditional rendering instead of JavaScript ternary operators or logical AND operators.
Use theForcomponent from solid-js for rendering lists. It provides better performance and keying compared to JavaScript's map function in JSX.
When manipulating children in wrapper components, use thechildrenhelper from solid-js to ensure proper reactivity and handling of child elements.
Files:
packages/web/src/lib/bfcache-handler.jspackages/web/src/main.jsxpackages/web/src/api/better-auth-store.jspackages/web/src/stores/projectStore.js
packages/{web,ui}/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
packages/{web,ui}/**/*.{js,jsx,ts,tsx}: Import UI components from '@corates/ui' package instead of local component files. Do not import Ark UI components from local paths like '@/components/zag/' or 'packages/web/src/components/zag/'
Always use 'solid-icons' library for icons. Never use emoji characters or text as icon replacements. Import from specific icon sets like 'solid-icons/bi', 'solid-icons/fi', 'solid-icons/ai', etc.
Files:
packages/web/src/lib/bfcache-handler.jspackages/web/src/main.jsxpackages/web/src/api/better-auth-store.jspackages/web/src/stores/projectStore.js
packages/web/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
Use import aliases from 'packages/web/jsconfig.json' instead of relative paths. Aliases include: '@/' (src/), '@components/' (src/components/), '@auth-ui/' (src/components/auth-ui/), '@checklist-ui/' (src/components/checklist-ui/), '@project-ui/' (src/components/project-ui/), '@routes/' (src/routes/), '@primitives/' (src/primitives/), '@api/' (src/api/), '@config/' (src/config/), and '@lib/' (src/lib/)
Files:
packages/web/src/lib/bfcache-handler.jspackages/web/src/main.jsxpackages/web/src/api/better-auth-store.jspackages/web/src/stores/projectStore.js
packages/{web,ui}/src/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)
Group related components in subdirectories with barrel exports
Files:
packages/web/src/main.jsx
packages/{web,landing}/src/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)
packages/{web,landing}/src/**/*.{jsx,tsx}: Use Ark UI components from @corates/ui package, not local component implementations
Use solid-icons library (e.g., solid-icons/bi, solid-icons/fi) for icon imports
Files:
packages/web/src/main.jsx
packages/web/src/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)
packages/web/src/**/*.{jsx,tsx}: In SolidJS, do NOT prop-drill application state. Import stores directly where needed instead.
In SolidJS, do NOT destructure props. Access props.field directly or wrap in a function: () => props.field
In SolidJS components, components should receive at most 1-5 props (local config only, not shared state)
Files:
packages/web/src/main.jsx
packages/{web,ui}/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)
Use Tailwind CSS classes for styling components
Files:
packages/web/src/main.jsx
packages/web/src/stores/**/*.{js,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Solid's
createStorefor complex state or state objects for better performance and reactivity
packages/web/src/stores/**/*.{js,ts}: In SolidJS, shared state should live in external stores under packages/web/src/stores/
In SolidJS, use createStore for complex state objects
Files:
packages/web/src/stores/projectStore.js
🧠 Learnings (2)
📚 Learning: 2025-12-24T17:23:10.082Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/solidjs.mdc:0-0
Timestamp: 2025-12-24T17:23:10.082Z
Learning: Applies to {packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts} : Import stores directly in components rather than prop-drilling store data through component hierarchies.
Applied to files:
packages/web/src/main.jsx
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/workers/src/**/*.{js,ts} : Use Better-Auth for authentication and user management
Applied to files:
packages/web/src/api/better-auth-store.js
🧬 Code graph analysis (2)
packages/web/src/main.jsx (2)
packages/web/src/lib/formStatePersistence.js (1)
cleanupExpiredStates(232-272)packages/web/src/lib/bfcache-handler.js (1)
initBfcacheHandler(15-85)
packages/web/src/stores/projectStore.js (1)
packages/web/src/components/sidebar/Sidebar.jsx (1)
currentUserId(23-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Workers Builds: corates
🔇 Additional comments (3)
packages/web/src/main.jsx (1)
13-16: LGTM!The bfcache handler initialization is correctly placed after form state cleanup, and the comment clearly explains the purpose. Since this is a singleton handler that should persist for the app's lifetime, not capturing the cleanup function is acceptable.
packages/web/src/api/better-auth-store.js (2)
679-691: LGTM -forceRefreshSessionis well-designed.Clean implementation that wraps the session refetch and properly re-throws errors so callers can handle failures. The warning log before re-throwing aids debugging without swallowing the error.
198-226: Visibility change handler improvements look good.The async handling with proper error catching for both session refresh and project list refresh is well-structured. The guards (
!authLoading() && isOnline()) prevent unnecessary work. The duplication withbfcache-handler.jswas noted in that file's review.
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.