feat: avoid no-op draft writes

packages/root-cms/ui/hooks/useDraftDoc.test.ts

@@ -0,0 +1,50 @@
+import {describe, expect, it} from 'vitest';
+import {testDraftUpdateChangesValue} from './useDraftDoc.js';
+
+describe('testDraftUpdateChangesValue', () => {
+  it('returns false for deeply equal values', () => {
+    expect(
+      testDraftUpdateChangesValue(
+        {items: [{title: 'One'}, {title: 'Two'}]},
+        {items: [{title: 'One'}, {title: 'Two'}]}
+      )
+    ).toBe(false);
+  });
+
+  it('returns true when values differ', () => {
+    expect(testDraftUpdateChangesValue('Published title', 'Draft title')).toBe(
+      true
+    );
+  });
+
+  it('treats same-reference objects as changed', () => {
+    const value = {title: 'Maybe mutated in place'};
+
+    expect(testDraftUpdateChangesValue(value, value)).toBe(true);
+  });
+
+  it('returns false when deleting an absent value', () => {
+    expect(testDraftUpdateChangesValue(undefined, undefined)).toBe(false);
+  });
+
+  it('returns true when deleting a present null value', () => {
+    expect(testDraftUpdateChangesValue(null, undefined)).toBe(true);
+  });
+
+  it('can treat null as a delete operation for batch updates', () => {
+    expect(
+      testDraftUpdateChangesValue(undefined, null, {deleteNull: true})
+    ).toBe(false);
+    expect(testDraftUpdateChangesValue(null, null, {deleteNull: true})).toBe(
+      true
+    );
+  });
+
+  it('allows changed metadata array updates', () => {
+    expect(testDraftUpdateChangesValue(['en'], ['en', 'es'])).toBe(true);
+  });
+
+  it('skips unchanged metadata array updates', () => {
+    expect(testDraftUpdateChangesValue(['en', 'es'], ['en', 'es'])).toBe(false);
+  });
+});

packages/root-cms/ui/hooks/useDraftDoc.tsx

@@ -22,6 +22,7 @@ import {
   UnsubscribeCallback,
 } from '../utils/json-trie-store.js';
 import {errorMessage} from '../utils/notifications.js';
+import {deepEqual} from '../utils/objects.js';
 import {TIME_UNITS} from '../utils/time.js';
 
 const SAVE_DELAY = 3 * TIME_UNITS.second;
@@ -200,6 +201,10 @@ export class DraftDocController extends EventListener {
     if (this.readOnly) {
       return;
     }
+    if (!testDraftUpdateChangesValue(this.store.get(key), value)) {
+      // Avoid metadata-only writes that make published docs look like drafts.
+      return;
+    }
     if (value === undefined) {
       // Firestore doesn't support `undefined`, so use deleteField() instead.
       this.pendingUpdates.set(key, deleteField());
@@ -220,8 +225,16 @@ export class DraftDocController extends EventListener {
     if (this.readOnly) {
       return;
     }
+    const changedUpdates: Record<string, any> = {};
     for (const key in updates) {
       const val = updates[key];
+      if (
+        !testDraftUpdateChangesValue(this.store.get(key), val, {
+          deleteNull: true,
+        })
+      ) {
+        continue;
+      }
       if (val === null || val === undefined) {
         // Firestore doesn't support `undefined`, so use deleteField() instead.
         // NOTE(stevenle): this doesn't currently handle nested `undefined`
@@ -230,8 +243,13 @@ export class DraftDocController extends EventListener {
       } else {
         this.pendingUpdates.set(key, val);
       }
+      changedUpdates[key] = val;
+    }
+    if (Object.keys(changedUpdates).length === 0) {
+      // Avoid metadata-only writes when every batch update is a no-op.
+      return;
     }
-    this.store.update(updates);
+    this.store.update(changedUpdates);
     this.setSaveState(SaveState.UPDATES_PENDING);
     if (this.autoSave) {
       this.queueChanges();
@@ -245,6 +263,10 @@ export class DraftDocController extends EventListener {
     if (this.readOnly) {
       return;
     }
+    if (!testDraftUpdateChangesValue(this.store.get(key), undefined)) {
+      // Avoid metadata-only writes when deleting a value that is already absent.
+      return;
+    }
     this.pendingUpdates.set(key, deleteField());
     this.store.set(key, undefined);
     this.setSaveState(SaveState.UPDATES_PENDING);
@@ -386,6 +408,28 @@ export class DraftDocController extends EventListener {
   }
 }
 
+/**
+ * Returns whether a requested draft update would change the current value.
+ * This prevents no-op field edits from being saved as metadata-only writes.
+ */
+export function testDraftUpdateChangesValue(
+  currentValue: any,
+  newValue: any,
+  options?: {deleteNull?: boolean}
+): boolean {
+  if (newValue === undefined || (options?.deleteNull && newValue === null)) {
+    return currentValue !== undefined;
+  }
+  if (currentValue === newValue && isMutableDraftValue(currentValue)) {
+    return true;
+  }
+  return !deepEqual(currentValue, newValue);
+}
+
+function isMutableDraftValue(value: any): boolean {
+  return typeof value === 'object' && value !== null;
+}
+
 function applyUpdates(data: any, updates: any) {
   for (const key in updates) {
     const val = updates[key];