diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index df87517..fe071d5 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -131,9 +131,9 @@ const CADEditor: React.FC = ({ projectId, token, onNavigateBack addElement: (el) => setElements((prev) => [...prev, el]), removeElement: (id) => setElements((prev) => prev.filter((e) => e.id !== id)), updateElement: (id, props) => setElements((prev) => prev.map((e) => (e.id === id ? { ...e, ...props } : e))), - getElements: () => elements, - getLayers: () => layers, - getActiveLayerId: () => activeLayerId, + getElements: () => elementsRef.current, + getLayers: () => layersRef.current, + getActiveLayerId: () => activeLayerIdRef.current, showToast: (msg) => setCommandHistory((prev) => [...prev, { prefix: '·', text: msg, type: 'info' }]), log: (msg) => console.log(`[Plugin] ${msg}`), }; @@ -177,6 +177,21 @@ const CADEditor: React.FC = ({ projectId, token, onNavigateBack const groupManagerRef = React.useRef(new GroupManager()); const [groups, setGroups] = useState([]); + // 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) const clipboardRef = React.useRef(null); @@ -265,8 +280,11 @@ const CADEditor: React.FC = ({ projectId, token, onNavigateBack setSavedStatus('gespeichert'); setDataLoaded(true); // Push initial data to Yjs CRDT after load - if (collab.status === 'connected') { - const bgConfigs = bgConfig ? [bgConfig] : []; + // 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 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({ elements: data.elements, layers: data.layers, diff --git a/frontend/src/crdt/YjsDocument.ts b/frontend/src/crdt/YjsDocument.ts index 4ffbbdd..fc726cd 100644 --- a/frontend/src/crdt/YjsDocument.ts +++ b/frontend/src/crdt/YjsDocument.ts @@ -122,25 +122,33 @@ export class YjsDocument { bgConfigs?: BackgroundConfig[]; }): void { this.doc.transact(() => { - this.data.elements.clear(); - for (const el of state.elements) { - this.data.elements.set(el.id, el); + // 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(); + for (const el of state.elements) { + this.data.elements.set(el.id, el); + } } - this.data.layers.clear(); - for (const layer of state.layers) { - this.data.layers.set(layer.id, layer); + if (state.layers.length > 0) { + this.data.layers.clear(); + for (const layer of state.layers) { + this.data.layers.set(layer.id, layer); + } } - this.data.blocks.clear(); - for (const block of state.blocks) { - this.data.blocks.set(block.id, block); + if (state.blocks.length > 0) { + this.data.blocks.clear(); + for (const block of state.blocks) { + this.data.blocks.set(block.id, block); + } } - if (state.groups) { + if (state.groups && state.groups.length > 0) { this.data.groups.clear(); for (const group of state.groups) { this.data.groups.set(group.id, group); } } - if (state.bgConfigs) { + if (state.bgConfigs && state.bgConfigs.length > 0) { this.data.bgConfig.clear(); for (const cfg of state.bgConfigs) { // Use 'default' as the key since there is typically one background config diff --git a/frontend/test_report.md b/frontend/test_report.md new file mode 100644 index 0000000..ad1a6cc --- /dev/null +++ b/frontend/test_report.md @@ -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.