-
Notifications
You must be signed in to change notification settings - Fork 40
feat(tasks): Implement local-first architecture for task modifications #168
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(tasks): Implement local-first architecture for task modifications #168
Conversation
When a task is added, edited, completed, or deleted, the change is
now saved immediately to the local IndexedDB (Dexie) database.
This provides an instant, responsive UI and allows us to track the
"dirty" state of tasks that have not yet been synced with the backend.
Key changes:
- Updates `TasksDatabase` (hooks.ts) to v2, adding an `isUnsynced`
index to the 'tasks' table.
- Updates the `Task` interface (types.ts) to include the optional
`isUnsynced?: boolean` field.
- Refactors `handleAddTask`, `handleSaveClick`, `handleSaveTags`, and the
"Mark Completed" / "Mark Deleted" onClick handlers to:
1. Save the change to IndexedDB *first* with `isUnsynced: true`.
2. Refresh the UI state from the local DB for an instant update.
3. Move the backend API call to a non-blocking "fire-and-forget"
step with its own error handling.
- Adds a red, pulsing "Unsynced" badge to the task row that appears
conditionally based on the `isUnsynced` flag.
- Updates the ID column to display a '-' for newly created tasks
(which have a temporary negative ID) for a cleaner UI.
- Updates `syncTasksWithTwAndDb` to set `isUnsynced: false` on all
tasks during a full sync, which clears all "Unsynced" badges.
- Updates `Tasks.test.tsx` to mock unsynced/new tasks and adds new
tests to validate the "Unsynced" badge and the '-' ID display.
|
@its-me-abhishek Could you please review this PR? I’m open to any feedback or suggestions 🙂 |
its-me-abhishek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m not really in favor of this approach. It’s better to keep the database and its fields unchanged, as modifying them often leads to unexpected issues and ends up being more of a duct-tape fix.
Instead, I’d suggest maintaining a separate set that holds only the UUIDs of tasks that haven’t been synced yet, applied solely to tasks added or edited on the frontend. This would be a cleaner, non-invasive solution. The memory impact would still be minimal comparable to adding a new field; while avoiding interference with existing functionality, making it a more better approach overall.
|
@its-me-abhishek you're right, thanks for the feedback. I originally went with the isUnsynced flag because I thought reading from a single data source would be simpler. But I agree, keeping the main Task schema clean is the better long-term solution. Using a separate set for the UUIDs is a cleaner approach, and I should have asked first. I'll start refactoring the code to use this new method. Thanks for the guidance, I appreciate you teaching me the trade-offs! |
Addresses code review feedback to keep the main Task schema clean and avoid coupling UI state with the data model. Key changes: - Reverted changes to `Task` type (removed `isUnsynced`). - Updated Dexie schema (v3) to add a separate `unsynced_tasks` table instead of modifying the main `tasks` table. - Implemented `db.transaction` in all write operations (Add, Edit, Delete) to ensure atomic updates to both tables. - Refactored UI state to use a `Set` for O(1) lookups of unsynced items. - Updated `syncTasksWithTwAndDb` to clear the separate table on success. - Added a notification badge to the Sync button to show the count of unsynced items. - Updated tests to mock the new sidecar table architecture.
|
I've made the requested changes! The implementation now uses a separate sidecar table instead of modifying the main Task schema. Ready for your review. Let me know if there is anything else you'd like me to adjust! |
| // this.version(1).stores({ | ||
| // tasks: 'uuid, email, status, project', | ||
| // }); | ||
| this.version(2).stores({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary to move to V2 and store these in DB? I think it is still a breaking change. Probably just storing the unique UUIDS inside a state should be enough to track un synced Tasks; considering we'll add first load as sync up later on
|
Closing as Discussed on Zulip |
Description
This PR implements a "local-first" architecture for task modifications. When a task is added, edited, completed, or deleted, the change is saved immediately to the local IndexedDB (Dexie), providing an instant UI response.
Refactor Update: Based on feedback, this implementation uses a Sidecar Table approach. This avoids modifying the core
Taskschema and strictly decouples UI state from business data.Key Changes
- Adds a separate
unsynced_taskstable to store only the UUIDs of locally modified tasks.- Ensures the main
taskstable remains unmodified to preserve data purity.handleAddTask,handleSaveClick,handleSaveTags, and deletion/completion handlers now usedb.transactionto atomically update both the task data and the unsynced UUID list.Setin React state for- Adds a red "Unsynced" badge to task rows found in the sidecar list.
- Adds a notification counter to the "Sync" button showing the exact number of pending items.
- Displays a
-in the ID column for newly created local tasks (hiding the temporary negative ID).syncTasksWithTwAndDbto clear theunsynced_taskstable upon a successful full sync.Tasks.test.tsxto mock the new sidecar table architecture.Checklist
npx prettier --write .(for formatting)gofmt -w .(for Go backend)npm test(for JS/TS testing)Additional Notes
Video: Link to see changes