Open
Conversation
Critical fixes for archived posts showing up after refresh: 1. **loadPostStates() race condition fix** - Don't clear state.archivedIds on Supabase error - Build new state in temp sets, only update on success - Add toast notification on load failure - Keep existing state if query fails 2. **Remove localStorage for logged-in users** - Deleted loadLocalArchivedState() and saveLocalArchivedState() - All archive state now in Supabase only - Anonymous users must sign in to archive posts - Cleaned up migrateLocalStorageToSupabase (now just cleanup) 3. **Require authentication** - Remove startPublicApp() - redirect to login if not authenticated - Sign out redirects to login immediately - Archive/unarchive requires state.user check 4. **Better loading state** - render() waits for supabaseReady before showing content - Show 'Loading your archive state...' message - Warn user if archive state fails to load 5. **Improved error handling** - startApp() checks if supabaseReady after loadPostStates() - User feedback when archive operations require login - Better logging of archive state load success/failure Fixes: Archived posts will no longer reappear on hard refresh or network issues
Contributor
|
Droid finished @factory-ben's task —— View job Code Review Complete ✅ Reviewed the changes in PR #31 which removes anonymous/public mode and simplifies archive state management to use Supabase only. The changes look generally solid:
Left one inline comment with a clarifying question about the migration logic change. |
![factory-droid[bot]](https://avatars.githubusercontent.com/in/358017?s=60&v=4){kind=small}
| console.error('[Migration] Error migrating localStorage to Supabase:', e); | ||
| console.error('[Cleanup] Error removing old localStorage:', e); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Could this cause data loss for users who had archived items in localStorage before this change is deployed? The old code would migrate localStorage archives to Supabase first, but now they're just deleted. If this is intentional to fix the "archive reappearing" bug, perhaps worth a comment explaining why migration was removed?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the critical issue where archived posts reappear after hard refresh or network issues.
Root Causes Identified
loadPostStates()- Function clearedstate.archivedIdsbefore checking if Supabase query succeeded, causing all posts to show as unarchived on errorfetchFeed()would render before archive state loadedChanges Made
Critical Fixes
state.archivedIdsif Supabase query succeedsBehavioral Changes
state.usercheckTesting
Before: Archived posts would reappear on hard refresh or if Supabase had any hiccup
After: Archive state preserved even on network errors; user notified if state fails to load
See
archive-reappear-issues.mdfor detailed technical analysis of all issues found.Breaking Changes
cc @bentossell