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

Configure Feed

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

fix(diagrams): use structuredClone for history snapshots to preserve complex shape properties

Replace JSON.stringify/JSON.parse serialization with structuredClone in the
diagram history (undo/redo) system. JSON round-tripping silently drops undefined
values, cannot natively serialize Maps, and loses special numeric values.
structuredClone handles Maps natively, preserves all property values including
undefined, and correctly deep-clones nested objects like style records and
freehand point arrays.

Add 10 new tests covering: gradient/filter style strings through undo/redo
cycles, line shapes with many points, all 13 shape kinds with full properties,
mutation isolation for points and styles, complex mixed boards, Map instance
verification, deep mutation independence, empty collections, and branch-on-push
semantics.

Closes #546

+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 + });