fix: address bugs B1–B8 from IMPROVEMENTS.md#213
Merged
Conversation
B1 (App.svelte): store media query handler in named var so removeEventListener removes the correct reference B2 (Menu.svelte): move ClipboardJS into onMount, narrow selector to [data-clipboard-text], destroy on cleanup B3 (Board.svelte): guard accepts callback against missing card during store mid-update B4 (Board.svelte): guard drop handler against missing card during store mid-update B5 (api.js): throw on non-OK HTTP status in requestJson so callers can distinguish failure B6 (firestore.js): optional-chain column/owner in normaliseCard to survive partial Firestore writes B7 (firestore.js): replace private _document field access with Date.now() fallback B8 (store.js): cancel stale passwordValid promises with a generation flag to prevent stale results Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ection - getBoard (onMount): wrap in try/catch so HTTP 404 navigates to /not-found instead of propagating as an unhandled rejection - updateBoard (board subscriber): use .catch() since the call is fire-and-forget and a sync try/catch cannot catch async errors - Remove resolved Bugs section from IMPROVEMENTS.md Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
Summary
Fixes all 8 bugs identified in the codebase review.
App.svelte): Memory leak — media query listener was added as an anonymous function butremoveEventListenerwas called with a different reference (darkModeChangeListener), so the listener was never removed. Fixed by storing the handler inhandleSchemeChangeand referencing it in both calls.Menu.svelte):ClipboardJSinstance created at module scope, never destroyed and used an overly broad'button'selector. Moved intoonMount, narrowed selector to[data-clipboard-text], destroyed in cleanup.Board.svelte): Null dereference in dragulaacceptscallback —.columnaccessed without checking if$cards.find()returned a result. Added null guard.Board.svelte): Same null dereference pattern in thedrophandler. Added early return guard.api.js):requestJsoncalled.json()on any response regardless of HTTP status, silently treating 4xx/5xx error bodies as valid data. Now throws on!response.ok.firestore.js):data.column.idanddata.owner.idaccessed without null checks innormaliseCard, crashing theonSnapshotcallback on partial Firestore documents. Added optional chaining.firestore.js):document._document.createTimeis a private, undocumented Firestore SDK field that can silently break on any SDK update. Replaced withDate.now()as the fallback.store.js): Race condition inpasswordValidderived store — rapid changes toboardorpasswordcould leave a stale promise result as the final value. Fixed with a cancellation flag.Test plan
🤖 Generated with Claude Code