[BUG] Fix impure state updates in DiagramContext and refactor Workspace
- 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
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