drawdb-io / drawdb-io/drawdb

[BUG] Fix impure state updates in DiagramContext and refactor Workspace

Open
#737 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
39.5k
Forks
3.2k
Avg merge
10h 46m
Merged PRs (30d)
16

Description

**Describe the bug**
[DiagramContext.jsx](cci:7://file:///Users/mohitkourav/Desktop/Ap%20merge%20project%20/drawdb/drawdb/src/context/DiagramContext.jsx:0:0-0:0) contained impure state updates where side effects (like updating the undo/redo stack) were occurring directly inside state setter functions. This can lead to unpredictable behavior, double-invocation in React Strict Mode, and state inconsistencies. Additionally, the context value was not memoized, causing unnecessary re-renders for all consuming components. [Workspace.jsx](cci:7://file:///Users/mohitkourav/Desktop/Ap%20merge%20project%20/drawdb/drawdb/src/components/Workspace.jsx:0:0-0:0) also contained mixed concerns, handling both UI layout and complex data persistence logic.

**To Reproduce**
Steps to reproduce the behavior:
1. Open [src/context/DiagramContext.jsx](cci:7://file:///Users/mohitkourav/Desktop/Ap%20merge%20project%20/drawdb/drawdb/src/context/DiagramContext.jsx:0:0-0:0).
2. Observe [addTable](cci:1://file:///Users/mohitkourav/Desktop/Ap%20merge%20project%20/drawdb/drawdb/src/context/DiagramContext.jsx:18:2-65:4), [deleteTable](cci:1://file:///Users/mohitkourav/Desktop/Ap%20merge%20project%20/drawdb/drawdb/src/context/DiagramContext.jsx:67:2-105:4), etc., where [setUndoStack](cci:1://file:///Users/mohitkourav/Desktop/Ap%20merge%20project%20/drawdb/drawdb/src/context/UndoRedoContext.jsx:4:2-4:24) is called inside `setTables` or `setRelationships` callbacks.
3. Observe the `value` object passed to `DiagramContext.Provider` is created on every render without `useMemo`.

**Expected behavior**
- State updates should be pure. Side effects like updating history (undo/redo) should happen outside of state setters.
- Context values should be memoized to prevent performance degradation.
- Data persistence logic should be separated from UI components for better maintainability.

**Additional context**
I have implemented the following fixes:
- Refactored [DiagramContext.jsx](cci:7://file:///Users/mohitkourav/Desktop/Ap%20merge%20project%20/drawdb/drawdb/src/context/DiagramContext.jsx:0:0-0:0) to move side effects out of state updaters and added `useMemo` for the context value.
- Extracted the diagram load/save/autosave logic from [Workspace.jsx](cci:7://file:///Users/mohitkourav/Desktop/Ap%20merge%20project%20/drawdb/drawdb/src/components/Workspace.jsx:0:0-0:0) into a new custom hook [src/hooks/useDiagramIO.js](cci:7://file:///Users/mohitkourav/Desktop/Ap%20merge%20project%20/drawdb/drawdb/src/hooks/useDiagramIO.js:0:0-0:0).
- Simplified [Workspace.jsx](cci:7://file:///Users/mohitkourav/Desktop/Ap%20merge%20project%20/drawdb/drawdb/src/components/Workspace.jsx:0:0-0:0) to focus on layout and component composition.

Contributor guide

Open the contributing guide

Research direction

Start by reading src/context/DiagramContext.jsx and src/components/Workspace.jsx, focusing on the state updates, provider value, and persistence responsibilities described in the issue. Review src/hooks/useDiagramIO.js to understand the proposed separation. Done means state updates are pure, the context value is memoized, and diagram load/save/autosave logic is separated from Workspace.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.