fix(frontend): loadFromState race condition + plugin stale closure (Issues #7, #8)

This commit is contained in:
2026-06-29 23:50:18 +02:00
parent 8493138b4b
commit 275b863f03
3 changed files with 108 additions and 16 deletions
+22 -4
View File
@@ -131,9 +131,9 @@ const CADEditor: React.FC<CADEditorProps> = ({ projectId, token, onNavigateBack
addElement: (el) => setElements((prev) => [...prev, el]), addElement: (el) => setElements((prev) => [...prev, el]),
removeElement: (id) => setElements((prev) => prev.filter((e) => e.id !== id)), removeElement: (id) => setElements((prev) => prev.filter((e) => e.id !== id)),
updateElement: (id, props) => setElements((prev) => prev.map((e) => (e.id === id ? { ...e, ...props } : e))), updateElement: (id, props) => setElements((prev) => prev.map((e) => (e.id === id ? { ...e, ...props } : e))),
getElements: () => elements, getElements: () => elementsRef.current,
getLayers: () => layers, getLayers: () => layersRef.current,
getActiveLayerId: () => activeLayerId, getActiveLayerId: () => activeLayerIdRef.current,
showToast: (msg) => setCommandHistory((prev) => [...prev, { prefix: '·', text: msg, type: 'info' }]), showToast: (msg) => setCommandHistory((prev) => [...prev, { prefix: '·', text: msg, type: 'info' }]),
log: (msg) => console.log(`[Plugin] ${msg}`), log: (msg) => console.log(`[Plugin] ${msg}`),
}; };
@@ -177,6 +177,21 @@ const CADEditor: React.FC<CADEditorProps> = ({ projectId, token, onNavigateBack
const groupManagerRef = React.useRef(new GroupManager()); const groupManagerRef = React.useRef(new GroupManager());
const [groups, setGroups] = useState<ElementGroup[]>([]); const [groups, setGroups] = useState<ElementGroup[]>([]);
// Issue #8: Refs for PluginContext to avoid stale closures.
// These refs are updated on every render so PluginContext getters always return current state.
const elementsRef = useRef(elements);
elementsRef.current = elements;
const layersRef = useRef(layers);
layersRef.current = layers;
const blocksRef = useRef(blocks);
blocksRef.current = blocks;
const groupsRef = useRef(groups);
groupsRef.current = groups;
const bgConfigRef = useRef(bgConfig);
bgConfigRef.current = bgConfig;
const activeLayerIdRef = useRef(activeLayerId);
activeLayerIdRef.current = activeLayerId;
// Clipboard (for copy/paste) // Clipboard (for copy/paste)
const clipboardRef = React.useRef<CADElement[] | null>(null); const clipboardRef = React.useRef<CADElement[] | null>(null);
@@ -265,8 +280,11 @@ const CADEditor: React.FC<CADEditorProps> = ({ projectId, token, onNavigateBack
setSavedStatus('gespeichert'); setSavedStatus('gespeichert');
setDataLoaded(true); setDataLoaded(true);
// Push initial data to Yjs CRDT after load // Push initial data to Yjs CRDT after load
if (collab.status === 'connected') { // Issue #7: Only push to CRDT if backend data actually has content.
// Pushing empty arrays would wipe existing CRDT data from other connected clients.
const bgConfigs = bgConfig ? [bgConfig] : []; const bgConfigs = bgConfig ? [bgConfig] : [];
const hasContent = data.elements.length > 0 || data.layers.length > 0 || data.blocks.length > 0 || groups.length > 0 || bgConfigs.length > 0;
if (collab.status === 'connected' && hasContent) {
collab.loadFromState({ collab.loadFromState({
elements: data.elements, elements: data.elements,
layers: data.layers, layers: data.layers,
+10 -2
View File
@@ -122,25 +122,33 @@ export class YjsDocument {
bgConfigs?: BackgroundConfig[]; bgConfigs?: BackgroundConfig[];
}): void { }): void {
this.doc.transact(() => { this.doc.transact(() => {
// Issue #7: Only clear and replace a map if the incoming array is non-empty.
// Clearing maps when data is empty wipes CRDT data from other connected clients.
if (state.elements.length > 0) {
this.data.elements.clear(); this.data.elements.clear();
for (const el of state.elements) { for (const el of state.elements) {
this.data.elements.set(el.id, el); this.data.elements.set(el.id, el);
} }
}
if (state.layers.length > 0) {
this.data.layers.clear(); this.data.layers.clear();
for (const layer of state.layers) { for (const layer of state.layers) {
this.data.layers.set(layer.id, layer); this.data.layers.set(layer.id, layer);
} }
}
if (state.blocks.length > 0) {
this.data.blocks.clear(); this.data.blocks.clear();
for (const block of state.blocks) { for (const block of state.blocks) {
this.data.blocks.set(block.id, block); this.data.blocks.set(block.id, block);
} }
if (state.groups) { }
if (state.groups && state.groups.length > 0) {
this.data.groups.clear(); this.data.groups.clear();
for (const group of state.groups) { for (const group of state.groups) {
this.data.groups.set(group.id, group); this.data.groups.set(group.id, group);
} }
} }
if (state.bgConfigs) { if (state.bgConfigs && state.bgConfigs.length > 0) {
this.data.bgConfig.clear(); this.data.bgConfig.clear();
for (const cfg of state.bgConfigs) { for (const cfg of state.bgConfigs) {
// Use 'default' as the key since there is typically one background config // Use 'default' as the key since there is typically one background config
+66
View File
@@ -0,0 +1,66 @@
# Test Report — Issues #7 & #8 Fix
**Date:** 2026-06-29
**Task:** Fix loadFromState race condition (#7) and Plugin Context stale closure (#8)
## Files Modified
- `frontend/src/App.tsx` — refs for PluginContext, loadFromState guard
- `frontend/src/crdt/YjsDocument.ts` — loadFromState guard against empty data clearing maps
## Test Results
### 1. TypeScript Type Check (`tsc --noEmit`)
```
$ npx tsc --noEmit
EXIT_CODE=0
```
**Result:** PASS — 0 errors
### 2. Vite Build
```
$ npx vite build
✓ 341 modules transformed.
✓ built in 8.04s
EXIT_CODE=0
```
**Result:** PASS — build successful
### 3. Grep Verification — useRef refs in App.tsx
```
182: const elementsRef = useRef(elements);
184: const layersRef = useRef(layers);
186: const blocksRef = useRef(blocks);
188: const groupsRef = useRef(groups);
190: const bgConfigRef = useRef(bgConfig);
192: const activeLayerIdRef = useRef(activeLayerId);
```
**Result:** PASS — all refs declared
### 4. Grep Verification — elementsRef.current in PluginContext
```
134: getElements: () => elementsRef.current,
135: getLayers: () => layersRef.current,
136: getActiveLayerId: () => activeLayerIdRef.current,
```
**Result:** PASS — PluginContext getters use refs
### 5. Grep Verification — loadFromState guard checks non-empty data
```
App.tsx:286: const hasContent = data.elements.length > 0 || data.layers.length > 0 || data.blocks.length > 0 || groups.length > 0 || bgConfigs.length > 0;
App.tsx:287: if (collab.status === 'connected' && hasContent) {
YjsDocument.ts:127: if (state.elements.length > 0) {
YjsDocument.ts:133: if (state.layers.length > 0) {
YjsDocument.ts:139: if (state.blocks.length > 0) {
YjsDocument.ts:145: if (state.groups && state.groups.length > 0) {
YjsDocument.ts:151: if (state.bgConfigs && state.bgConfigs.length > 0) {
```
**Result:** PASS — guards in both files
## Smoke Test
- App builds successfully with `vite build` (341 modules, 8.04s)
- No TypeScript errors
- No `any` type used
- No existing PluginContext fields removed
## Summary
All test criteria met. Both issues fixed.