fix: toDelta should ignore entries with null value (#875)

* fix: toDelta should ignore entries with null value

* fix: TextDelta convert

* chore: changeset

Author: zxch3n

.changeset/dull-pans-exercise.md

@@ -0,0 +1,6 @@
+---
+"loro-crdt": patch
+"loro-crdt-map": patch
+---
+
+fix: toDelta should ignore null style entries #875

crates/loro-wasm/src/convert.rs

@@ -722,9 +722,7 @@ pub(crate) fn loro_json_schema_to_js_json_schema(
     Ok(value.into())
 }
 
-pub(crate) fn text_delta_to_js_value(
-    delta: Vec<TextDelta>,
-) -> Result<JsValue, JsValue> {
+pub(crate) fn text_delta_to_js_value(delta: Vec<TextDelta>) -> Result<JsValue, JsValue> {
     let arr = Array::new();
     let mut iter = delta.into_iter();
     let mut current = iter.next();
@@ -734,26 +732,35 @@ pub(crate) fn text_delta_to_js_value(
 
         // Try to merge with next items
         while let Some(next) = next_item.take() {
-             match (&mut item, next) {
+            match (&mut item, next) {
                 (
-                    TextDelta::Insert { insert: i1, attributes: a1 },
-                    TextDelta::Insert { insert: i2, attributes: a2 }
+                    TextDelta::Insert {
+                        insert: i1,
+                        attributes: a1,
+                    },
+                    TextDelta::Insert {
+                        insert: i2,
+                        attributes: a2,
+                    },
                 ) if a1 == &a2 => {
                     i1.push_str(&i2);
                     // next_item is consumed, try next
                     next_item = iter.next();
                 }
                 (
-                    TextDelta::Retain { retain: r1, attributes: a1 },
-                    TextDelta::Retain { retain: r2, attributes: a2 }
+                    TextDelta::Retain {
+                        retain: r1,
+                        attributes: a1,
+                    },
+                    TextDelta::Retain {
+                        retain: r2,
+                        attributes: a2,
+                    },
                 ) if a1 == &a2 => {
                     *r1 += r2;
                     next_item = iter.next();
                 }
-                (
-                    TextDelta::Delete { delete: d1 },
-                    TextDelta::Delete { delete: d2 }
-                ) => {
+                (TextDelta::Delete { delete: d1 }, TextDelta::Delete { delete: d2 }) => {
                     *d1 += d2;
                     next_item = iter.next();
                 }
@@ -762,7 +769,7 @@ pub(crate) fn text_delta_to_js_value(
                     next_item = Some(next);
                     break;
                 }
-             }
+            }
         }
 
         // Convert merged item to JS
@@ -771,25 +778,13 @@ pub(crate) fn text_delta_to_js_value(
             TextDelta::Insert { insert, attributes } => {
                 Reflect::set(&obj, &"insert".into(), &insert.into())?;
                 if let Some(attributes) = attributes {
-                    if !attributes.is_empty() {
-                        let attrs = Object::new();
-                        for (k, v) in attributes {
-                            Reflect::set(&attrs, &k.into(), &convert(v)?)?;
-                        }
-                        Reflect::set(&obj, &"attributes".into(), &attrs)?;
-                    }
+                    set_style_attributes(&obj, attributes)?;
                 }
             }
             TextDelta::Retain { retain, attributes } => {
                 Reflect::set(&obj, &"retain".into(), &retain.into())?;
                 if let Some(attributes) = attributes {
-                    if !attributes.is_empty() {
-                        let attrs = Object::new();
-                        for (k, v) in attributes {
-                            Reflect::set(&attrs, &k.into(), &convert(v)?)?;
-                        }
-                        Reflect::set(&obj, &"attributes".into(), &attrs)?;
-                    }
+                    set_style_attributes(&obj, attributes)?;
                 }
             }
             TextDelta::Delete { delete } => {
@@ -802,3 +797,27 @@ pub(crate) fn text_delta_to_js_value(
     }
     Ok(arr.into())
 }
+
+fn set_style_attributes(
+    obj: &Object,
+    attributes: FxHashMap<String, LoroValue>,
+) -> Result<(), JsValue> {
+    if !attributes.is_empty() {
+        let attrs = Object::new();
+        let mut is_empty = true;
+        for (k, v) in attributes {
+            if v.is_null() {
+                // We should ignore attribute with null value, since it should be treated as deleted
+                continue;
+            }
+
+            is_empty = false;
+            Reflect::set(&attrs, &k.into(), &convert(v)?)?;
+        }
+
+        if !is_empty {
+            Reflect::set(obj, &"attributes".into(), &attrs)?;
+        }
+    }
+    Ok(())
+}

crates/loro-wasm/src/lib.rs

@@ -5946,7 +5946,7 @@ interface LoroDoc {
 export type Delta<T> =
   | {
     insert: T;
-    attributes?: { [key in string]: {} };
+    attributes?: { [key in string]: Value };
     retain?: undefined;
     delete?: undefined;
   }
@@ -5958,7 +5958,7 @@ export type Delta<T> =
   }
   | {
     retain: number;
-    attributes?: { [key in string]: {} };
+    attributes?: { [key in string]: Value };
     delete?: undefined;
     insert?: undefined;
   };

crates/loro-wasm/tests/richtext.test.ts

@@ -406,10 +406,10 @@ describe("richtext", () => {
 
     // Out of bounds
     try {
-        text.sliceDelta(0, 100);
-        expect.fail("Should throw error");
+      text.sliceDelta(0, 100);
+      expect.fail("Should throw error");
     } catch (e) {
-        // Expected
+      // Expected
     }
   });
 
@@ -420,9 +420,9 @@ describe("richtext", () => {
       italic: { expand: "before" },
     });
     const text = doc.getText("text");
-    text.insert(0, "你好 World!"); 
-    text.mark({ start: 0, end: 2 }, "bold", true); 
-    text.mark({ start: 3, end: 8 }, "italic", true); 
+    text.insert(0, "你好 World!");
+    text.mark({ start: 0, end: 2 }, "bold", true);
+    text.mark({ start: 3, end: 8 }, "italic", true);
 
     const deltaUtf8 = text.sliceDeltaUtf8(0, 13);
     expect(deltaUtf8).toStrictEqual([
@@ -446,10 +446,10 @@ describe("richtext", () => {
 
     // Out of bounds by UTF8
     try {
-        text.sliceDeltaUtf8(0, 100);
-        expect.fail("Should throw error");
+      text.sliceDeltaUtf8(0, 100);
+      expect.fail("Should throw error");
     } catch (e) {
-        // Expected
+      // Expected
     }
   });
 
@@ -466,9 +466,9 @@ describe("richtext", () => {
     // ✨: 7-8
     // World: 8-13
     // 🌍: 13-15
-    text.mark({ start: 0, end: 2 }, "emoji", "rocket"); 
-    text.mark({ start: 7, end: 8 }, "emoji", "sparkles"); 
-    text.mark({ start: 13, end: 15 }, "emoji", "earth"); 
+    text.mark({ start: 0, end: 2 }, "emoji", "rocket");
+    text.mark({ start: 7, end: 8 }, "emoji", "sparkles");
+    text.mark({ start: 13, end: 15 }, "emoji", "earth");
 
     const delta = text.sliceDelta(0, 15);
     expect(delta).toStrictEqual([
@@ -486,4 +486,27 @@ describe("richtext", () => {
       { insert: "World" },
     ] as Delta<string>[]);
   });
-});
+
+  it("should remove the style entry when applyDelta with style that contains null value", () => {
+    const doc = new LoroDoc();
+    doc
+      .getText("text")
+      .applyDelta([{ insert: "hello", attributes: { bold: true } }]);
+    doc
+      .getText("text")
+      .applyDelta([{ retain: 2 }, { retain: 2, attributes: { bold: null } }]);
+    expect(doc.getText("text").toDelta()).toStrictEqual([
+      {
+        insert: "he",
+        attributes: { bold: true },
+      },
+      {
+        insert: "ll",
+      },
+      {
+        insert: "o",
+        attributes: { bold: true },
+      },
+    ]);
+  });
+});