Full document, spreadsheet, slideshow, and diagram tooling
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

Merge pull request 'fix(diagrams): use structuredClone for history snapshots (#546)' (#332) from fix/diagram-history-546-v2 into main

scott 7b1d7aa4 3b6eff59

+344 -31
+15 -31
src/diagrams/history.ts
··· 1 1 /** 2 2 * Undo/redo history for the diagrams whiteboard. 3 3 * 4 - * Stores deep-cloned snapshots of WhiteboardState via JSON serialization. 5 - * Maps are serialized as [key, value] entry arrays and restored on deserialize. 4 + * Stores deep-cloned snapshots of WhiteboardState via structuredClone. 5 + * structuredClone natively handles Maps, preserves undefined values, 6 + * and correctly clones nested objects — avoiding the data-loss edge cases 7 + * of JSON round-tripping (which drops undefined, cannot serialize Maps 8 + * directly, and silently converts special numeric values). 6 9 */ 7 10 8 11 import type { WhiteboardState } from './whiteboard'; 9 12 10 13 const MAX_DEPTH = 100; 11 14 12 - /** JSON-safe representation of WhiteboardState (Maps replaced with entry arrays). */ 13 - type Serialized = Omit<WhiteboardState, 'shapes' | 'arrows'> & { 14 - shapes: [string, WhiteboardState['shapes'] extends Map<string, infer V> ? V : never][]; 15 - arrows: [string, WhiteboardState['arrows'] extends Map<string, infer V> ? V : never][]; 16 - }; 17 - 18 - function serialize(state: WhiteboardState): string { 19 - const plain: Serialized = { 20 - ...state, 21 - shapes: [...state.shapes.entries()], 22 - arrows: [...state.arrows.entries()], 23 - }; 24 - return JSON.stringify(plain); 25 - } 26 - 27 - function deserialize(json: string): WhiteboardState { 28 - const plain: Serialized = JSON.parse(json); 29 - return { 30 - ...plain, 31 - shapes: new Map(plain.shapes), 32 - arrows: new Map(plain.arrows), 33 - }; 15 + /** Deep-clone a WhiteboardState, preserving Maps and all property values. */ 16 + function cloneState(state: WhiteboardState): WhiteboardState { 17 + return structuredClone(state); 34 18 } 35 19 36 20 /** 37 21 * Linear undo/redo history with max depth and branch-on-push semantics. 38 22 * 39 - * The internal stack stores serialized JSON strings. A cursor index tracks 40 - * the current position. Undo moves the cursor back; redo moves it forward. 41 - * Pushing after an undo truncates the forward (redo) entries. 23 + * The internal stack stores deep-cloned WhiteboardState snapshots. A cursor 24 + * index tracks the current position. Undo moves the cursor back; redo moves 25 + * it forward. Pushing after an undo truncates the forward (redo) entries. 42 26 */ 43 27 class History { 44 - private stack: string[] = []; 28 + private stack: WhiteboardState[] = []; 45 29 private cursor = -1; 46 30 47 31 /** Record a new state snapshot. Discards any redo entries (branching). */ 48 32 push(state: WhiteboardState): void { 49 33 // Discard everything after the cursor (branch) 50 34 this.stack.length = this.cursor + 1; 51 - this.stack.push(serialize(state)); 35 + this.stack.push(cloneState(state)); 52 36 this.cursor = this.stack.length - 1; 53 37 54 38 // Enforce max depth — drop oldest entries from the front ··· 63 47 undo(): WhiteboardState | undefined { 64 48 if (!this.canUndo()) return undefined; 65 49 this.cursor--; 66 - return deserialize(this.stack[this.cursor]!); 50 + return cloneState(this.stack[this.cursor]!); 67 51 } 68 52 69 53 /** Move forward one step. Returns the next state, or undefined if at the end. */ 70 54 redo(): WhiteboardState | undefined { 71 55 if (!this.canRedo()) return undefined; 72 56 this.cursor++; 73 - return deserialize(this.stack[this.cursor]!); 57 + return cloneState(this.stack[this.cursor]!); 74 58 } 75 59 76 60 canUndo(): boolean {
+329
tests/diagram-history.test.ts
··· 235 235 expect(undone2.shapes.get('s1')!.label).toBe('v1'); 236 236 }); 237 237 }); 238 + 239 + describe('Diagram History — complex shape properties survive undo/redo', () => { 240 + it('preserves deeply nested style objects through multiple undo/redo cycles', () => { 241 + const history = new History(); 242 + const shape = makeShape({ 243 + id: 'styled-1', 244 + style: { 245 + fill: 'linear-gradient(90deg, #ff0000, #00ff00)', 246 + stroke: '#000', 247 + strokeWidth: '3', 248 + strokeDasharray: '5,3', 249 + filter: 'drop-shadow(2px 2px 4px rgba(0,0,0,0.5))', 250 + }, 251 + }); 252 + const state = makeState({ shapes: new Map([['styled-1', shape]]) }); 253 + 254 + history.push(makeState()); 255 + history.push(state); 256 + 257 + // Cycle: undo -> redo -> undo -> redo 258 + history.undo(); 259 + history.redo(); 260 + history.undo(); 261 + const redone = history.redo()!; 262 + 263 + const restored = redone.shapes.get('styled-1')!; 264 + expect(restored.style).toEqual({ 265 + fill: 'linear-gradient(90deg, #ff0000, #00ff00)', 266 + stroke: '#000', 267 + strokeWidth: '3', 268 + strokeDasharray: '5,3', 269 + filter: 'drop-shadow(2px 2px 4px rgba(0,0,0,0.5))', 270 + }); 271 + }); 272 + 273 + it('preserves line shape with many points through undo/redo', () => { 274 + const history = new History(); 275 + const points = Array.from({ length: 50 }, (_, i) => ({ 276 + x: i * 10, 277 + y: Math.sin(i / 5) * 100, 278 + })); 279 + const shape = makeShape({ 280 + id: 'line-1', 281 + kind: 'line', 282 + points, 283 + }); 284 + const state = makeState({ shapes: new Map([['line-1', shape]]) }); 285 + 286 + history.push(makeState()); 287 + history.push(state); 288 + history.undo(); 289 + const redone = history.redo()!; 290 + 291 + const restored = redone.shapes.get('line-1')!; 292 + expect(restored.points).toHaveLength(50); 293 + expect(restored.points![0]).toEqual({ x: 0, y: 0 }); 294 + expect(restored.points![49]).toEqual({ x: 490, y: Math.sin(49 / 5) * 100 }); 295 + }); 296 + 297 + it('preserves all shape kinds with their full properties', () => { 298 + const history = new History(); 299 + const kinds: Array<Shape['kind']> = [ 300 + 'rectangle', 'ellipse', 'diamond', 'text', 'freehand', 301 + 'line', 'triangle', 'star', 'hexagon', 'cloud', 302 + 'cylinder', 'parallelogram', 'note', 303 + ]; 304 + const shapes = new Map<string, Shape>(); 305 + for (const kind of kinds) { 306 + const id = `shape-${kind}`; 307 + shapes.set(id, makeShape({ 308 + id, 309 + kind, 310 + rotation: 45, 311 + label: `Label for ${kind}`, 312 + style: { fill: '#abcdef', stroke: '#123456' }, 313 + opacity: 0.75, 314 + fontFamily: 'monospace', 315 + fontSize: 18, 316 + groupId: 'test-group', 317 + points: kind === 'freehand' || kind === 'line' 318 + ? [{ x: 0, y: 0 }, { x: 10, y: 10 }] 319 + : undefined, 320 + })); 321 + } 322 + const state = makeState({ shapes }); 323 + 324 + history.push(makeState()); 325 + history.push(state); 326 + history.undo(); 327 + const redone = history.redo()!; 328 + 329 + expect(redone.shapes.size).toBe(kinds.length); 330 + for (const kind of kinds) { 331 + const restored = redone.shapes.get(`shape-${kind}`)!; 332 + expect(restored.kind).toBe(kind); 333 + expect(restored.rotation).toBe(45); 334 + expect(restored.label).toBe(`Label for ${kind}`); 335 + expect(restored.style).toEqual({ fill: '#abcdef', stroke: '#123456' }); 336 + expect(restored.opacity).toBe(0.75); 337 + expect(restored.fontFamily).toBe('monospace'); 338 + expect(restored.fontSize).toBe(18); 339 + expect(restored.groupId).toBe('test-group'); 340 + } 341 + }); 342 + 343 + it('mutating original points array after push does not affect history', () => { 344 + const history = new History(); 345 + const points = [{ x: 0, y: 0 }, { x: 10, y: 10 }, { x: 20, y: 5 }]; 346 + const shape = makeShape({ 347 + id: 'fh-1', 348 + kind: 'freehand', 349 + points, 350 + }); 351 + const state = makeState({ shapes: new Map([['fh-1', shape]]) }); 352 + 353 + history.push(makeState()); 354 + history.push(state); 355 + 356 + // Mutate the original points 357 + points[0].x = 999; 358 + points.push({ x: 100, y: 100 }); 359 + shape.points = []; 360 + 361 + history.undo(); 362 + const redone = history.redo()!; 363 + const restored = redone.shapes.get('fh-1')!; 364 + expect(restored.points).toEqual([ 365 + { x: 0, y: 0 }, 366 + { x: 10, y: 10 }, 367 + { x: 20, y: 5 }, 368 + ]); 369 + }); 370 + 371 + it('mutating style object after push does not affect history', () => { 372 + const history = new History(); 373 + const style = { fill: '#ff0000', stroke: '#000' }; 374 + const shape = makeShape({ id: 's1', style }); 375 + const state = makeState({ shapes: new Map([['s1', shape]]) }); 376 + 377 + history.push(makeState()); 378 + history.push(state); 379 + 380 + // Mutate the original style 381 + style.fill = '#00ff00'; 382 + (style as Record<string, string>).newProp = 'injected'; 383 + 384 + history.undo(); 385 + const redone = history.redo()!; 386 + const restored = redone.shapes.get('s1')!; 387 + expect(restored.style).toEqual({ fill: '#ff0000', stroke: '#000' }); 388 + expect(restored.style).not.toHaveProperty('newProp'); 389 + }); 390 + 391 + it('preserves complex board with shapes, arrows, and groups together', () => { 392 + const history = new History(); 393 + 394 + const shape1 = makeShape({ 395 + id: 'rect-1', 396 + kind: 'rectangle', 397 + x: 0, y: 0, 398 + groupId: 'g1', 399 + style: { fill: '#aaa', stroke: '#000', strokeDasharray: '4,2' }, 400 + }); 401 + const shape2 = makeShape({ 402 + id: 'star-1', 403 + kind: 'star', 404 + x: 200, y: 100, 405 + groupId: 'g1', 406 + rotation: 30, 407 + opacity: 0.5, 408 + fontFamily: 'serif', 409 + fontSize: 20, 410 + }); 411 + const shape3 = makeShape({ 412 + id: 'fh-1', 413 + kind: 'freehand', 414 + x: 50, y: 300, 415 + points: [{ x: 0, y: 0 }, { x: 15, y: 25 }, { x: 30, y: 10 }], 416 + }); 417 + 418 + const arrow1: Arrow = { 419 + id: 'a1', 420 + from: { shapeId: 'rect-1', anchor: 'right' }, 421 + to: { shapeId: 'star-1', anchor: 'left' }, 422 + label: 'flow', 423 + style: { stroke: '#333' }, 424 + }; 425 + const arrow2: Arrow = { 426 + id: 'a2', 427 + from: { x: 400, y: 400 }, 428 + to: { shapeId: 'fh-1', anchor: 'top' }, 429 + label: '', 430 + style: {}, 431 + }; 432 + 433 + const state = makeState({ 434 + shapes: new Map([['rect-1', shape1], ['star-1', shape2], ['fh-1', shape3]]), 435 + arrows: new Map([['a1', arrow1], ['a2', arrow2]]), 436 + panX: -50, 437 + panY: 100, 438 + zoom: 1.5, 439 + gridSize: 15, 440 + snapToGrid: false, 441 + }); 442 + 443 + history.push(makeState()); 444 + history.push(state); 445 + history.undo(); 446 + const redone = history.redo()!; 447 + 448 + expect(redone.shapes.size).toBe(3); 449 + expect(redone.arrows.size).toBe(2); 450 + expect(redone.panX).toBe(-50); 451 + expect(redone.zoom).toBe(1.5); 452 + 453 + // Verify grouped shapes 454 + expect(redone.shapes.get('rect-1')!.groupId).toBe('g1'); 455 + expect(redone.shapes.get('star-1')!.groupId).toBe('g1'); 456 + expect(redone.shapes.get('star-1')!.rotation).toBe(30); 457 + expect(redone.shapes.get('star-1')!.opacity).toBe(0.5); 458 + 459 + // Verify freehand points 460 + expect(redone.shapes.get('fh-1')!.points).toEqual([ 461 + { x: 0, y: 0 }, { x: 15, y: 25 }, { x: 30, y: 10 }, 462 + ]); 463 + 464 + // Verify arrows 465 + expect(redone.arrows.get('a1')!.from).toEqual({ shapeId: 'rect-1', anchor: 'right' }); 466 + expect(redone.arrows.get('a2')!.from).toEqual({ x: 400, y: 400 }); 467 + }); 468 + 469 + it('Maps are properly cloned — shapes and arrows are real Maps after undo/redo', () => { 470 + const history = new History(); 471 + const shape = makeShape({ id: 's1' }); 472 + const state = makeState({ shapes: new Map([['s1', shape]]) }); 473 + 474 + history.push(makeState()); 475 + history.push(state); 476 + history.undo(); 477 + const redone = history.redo()!; 478 + 479 + // Verify shapes and arrows are actual Map instances 480 + expect(redone.shapes).toBeInstanceOf(Map); 481 + expect(redone.arrows).toBeInstanceOf(Map); 482 + 483 + // Verify Map operations work 484 + expect(redone.shapes.has('s1')).toBe(true); 485 + expect([...redone.shapes.keys()]).toEqual(['s1']); 486 + expect([...redone.shapes.values()]).toHaveLength(1); 487 + }); 488 + 489 + it('returned states from undo() and redo() are independent copies', () => { 490 + const history = new History(); 491 + const state1 = makeState({ 492 + shapes: new Map([['s1', makeShape({ 493 + id: 's1', 494 + style: { fill: '#red' }, 495 + points: [{ x: 1, y: 2 }], 496 + })]]), 497 + }); 498 + const state2 = makeState({ 499 + shapes: new Map([['s1', makeShape({ id: 's1', label: 'v2' })]]), 500 + }); 501 + 502 + history.push(state1); 503 + history.push(state2); 504 + 505 + const undone = history.undo()!; 506 + // Mutate deeply: style, points, and Map itself 507 + undone.shapes.get('s1')!.style.fill = '#mutated'; 508 + undone.shapes.get('s1')!.points![0].x = 999; 509 + undone.shapes.set('injected', makeShape({ id: 'injected' })); 510 + 511 + // Redo and undo again -- must not see mutations 512 + const redone = history.redo()!; 513 + expect(redone.shapes.has('injected')).toBe(false); 514 + 515 + const undone2 = history.undo()!; 516 + expect(undone2.shapes.get('s1')!.style.fill).toBe('#red'); 517 + expect(undone2.shapes.get('s1')!.points![0].x).toBe(1); 518 + expect(undone2.shapes.has('injected')).toBe(false); 519 + }); 520 + 521 + it('handles empty style and empty points gracefully', () => { 522 + const history = new History(); 523 + const shape = makeShape({ 524 + id: 's1', 525 + style: {}, 526 + points: [], 527 + }); 528 + const state = makeState({ shapes: new Map([['s1', shape]]) }); 529 + 530 + history.push(makeState()); 531 + history.push(state); 532 + history.undo(); 533 + const redone = history.redo()!; 534 + 535 + const restored = redone.shapes.get('s1')!; 536 + expect(restored.style).toEqual({}); 537 + expect(restored.points).toEqual([]); 538 + }); 539 + 540 + it('branch-on-push: pushing after undo discards redo history', () => { 541 + const history = new History(); 542 + const stateA = makeState({ panX: 1 }); 543 + const stateB = makeState({ panX: 2 }); 544 + const stateC = makeState({ panX: 3 }); 545 + 546 + history.push(stateA); 547 + history.push(stateB); 548 + history.push(stateC); 549 + 550 + // Undo twice (cursor at A) 551 + history.undo(); // -> B 552 + history.undo(); // -> A 553 + 554 + // Push a new branch 555 + const stateD = makeState({ panX: 99 }); 556 + history.push(stateD); 557 + 558 + // Redo should return nothing (B and C were discarded) 559 + expect(history.canRedo()).toBe(false); 560 + expect(history.redo()).toBeUndefined(); 561 + 562 + // Undo goes back to A 563 + const undone = history.undo()!; 564 + expect(undone.panX).toBe(1); 565 + }); 566 + });