refactor!: reduce footguns when using for_each(fn) on rust (#540)

crates/loro-ffi/src/container/list.rs

@@ -90,7 +90,7 @@ impl LoroList {
     // TODO: wrap it in ffi side
     pub fn for_each<I>(&self, f: I)
     where
-        I: FnMut((usize, loro::ValueOrContainer)),
+        I: FnMut(loro::ValueOrContainer),
     {
         self.list.for_each(f)
     }

crates/loro-internal/src/handler.rs

@@ -2574,29 +2574,36 @@ impl ListHandler {
 
     pub fn for_each<I>(&self, mut f: I)
     where
-        I: FnMut((usize, ValueOrHandler)),
+        I: FnMut(ValueOrHandler),
     {
         match &self.inner {
             MaybeDetached::Detached(l) => {
                 let l = l.try_lock().unwrap();
-                for (i, v) in l.value.iter().enumerate() {
-                    f((i, v.clone()))
+                for v in l.value.iter() {
+                    f(v.clone())
                 }
             }
             MaybeDetached::Attached(inner) => {
+                let mut temp = vec![];
                 inner.with_state(|state| {
                     let a = state.as_list_state().unwrap();
-                    for (i, v) in a.iter().enumerate() {
+                    for v in a.iter() {
                         match v {
                             LoroValue::Container(c) => {
-                                f((i, ValueOrHandler::Handler(create_handler(inner, c.clone()))));
+                                temp.push(ValueOrHandler::Handler(create_handler(
+                                    inner,
+                                    c.clone(),
+                                )));
                             }
                             value => {
-                                f((i, ValueOrHandler::Value(value.clone())));
+                                temp.push(ValueOrHandler::Value(value.clone()));
                             }
                         }
                     }
                 });
+                for v in temp.into_iter() {
+                    f(v);
+                }
             }
         }
     }
@@ -3257,19 +3264,26 @@ impl MovableListHandler {
                     f(v.clone());
                 }
             }
-            MaybeDetached::Attached(m) => m.with_state(|state| {
-                let a = state.as_movable_list_state().unwrap();
-                for v in a.iter() {
-                    match v {
-                        LoroValue::Container(c) => {
-                            f(ValueOrHandler::Handler(create_handler(m, c.clone())));
-                        }
-                        value => {
-                            f(ValueOrHandler::Value(value.clone()));
+            MaybeDetached::Attached(m) => {
+                let mut temp = vec![];
+                m.with_state(|state| {
+                    let a = state.as_movable_list_state().unwrap();
+                    for v in a.iter() {
+                        match v {
+                            LoroValue::Container(c) => {
+                                temp.push(ValueOrHandler::Handler(create_handler(m, c.clone())));
+                            }
+                            value => {
+                                temp.push(ValueOrHandler::Value(value.clone()));
+                            }
                         }
                     }
+                });
+
+                for v in temp.into_iter() {
+                    f(v);
                 }
-            }),
+            }
         }
     }
 
@@ -3547,19 +3561,29 @@ impl MapHandler {
                 }
             }
             MaybeDetached::Attached(inner) => {
+                let mut temp = vec![];
                 inner.with_state(|state| {
                     let a = state.as_map_state().unwrap();
                     for (k, v) in a.iter() {
                         if let Some(v) = &v.value {
                             match v {
                                 LoroValue::Container(c) => {
-                                    f(k, ValueOrHandler::Handler(create_handler(inner, c.clone())))
+                                    temp.push((
+                                        k.to_string(),
+                                        ValueOrHandler::Handler(create_handler(inner, c.clone())),
+                                    ));
+                                }
+                                value => {
+                                    temp.push((k.to_string(), ValueOrHandler::Value(value.clone())))
                                 }
-                                value => f(k, ValueOrHandler::Value(value.clone())),
                             }
                         }
                     }
                 });
+
+                for (k, v) in temp.into_iter() {
+                    f(&k, v.clone());
+                }
             }
         }
     }

crates/loro-internal/src/jsonpath.rs

@@ -482,15 +482,16 @@ impl PathValue for MapHandler {
     }
 
     fn for_each_for_path(&self, f: &mut dyn FnMut(ValueOrHandler) -> ControlFlow<()>) {
-        let mut all = vec![];
+        let mut done = false;
         self.for_each(|_, v| {
-            all.push(v);
-        });
-        for v in all {
-            if let ControlFlow::Break(_) = f(v) {
-                break;
+            if done {
+                return;
             }
-        }
+
+            if let ControlFlow::Break(_) = f(v) {
+                done = true;
+            }
+        });
     }
 
     fn length_for_path(&self) -> usize {
@@ -520,15 +521,16 @@ impl PathValue for ListHandler {
     }
 
     fn for_each_for_path(&self, f: &mut dyn FnMut(ValueOrHandler) -> ControlFlow<()>) {
-        let mut all = vec![];
+        let mut done = false;
         self.for_each(|v| {
-            all.push(v.1);
-        });
-        for v in all {
-            if let ControlFlow::Break(_) = f(v) {
-                break;
+            if done {
+                return;
             }
-        }
+
+            if let ControlFlow::Break(_) = f(v) {
+                done = true;
+            }
+        });
     }
 
     fn length_for_path(&self) -> usize {
@@ -562,15 +564,16 @@ impl PathValue for MovableListHandler {
     }
 
     fn for_each_for_path(&self, f: &mut dyn FnMut(ValueOrHandler) -> ControlFlow<()>) {
-        let mut all = vec![];
+        let mut done = false;
         self.for_each(|v| {
-            all.push(v);
-        });
-        for v in all {
-            if let ControlFlow::Break(_) = f(v) {
-                break;
+            if done {
+                return;
             }
-        }
+
+            if let ControlFlow::Break(_) = f(v) {
+                done = true;
+            }
+        })
     }
 
     fn length_for_path(&self) -> usize {

crates/loro-internal/src/state/richtext_state.rs

@@ -109,9 +109,14 @@ impl RichtextState {
     }
 
     pub(crate) fn iter(&mut self, mut callback: impl FnMut(&str) -> bool) {
-        for span in self.state.get_mut().iter() {
-            if !callback(span.text.as_str()) {
-                return;
+        for span in self.state.get_mut().iter_chunk() {
+            match span {
+                RichtextStateChunk::Text(text_chunk) => {
+                    if !callback(text_chunk.as_str()) {
+                        return;
+                    }
+                }
+                RichtextStateChunk::Style { .. } => {}
             }
         }
     }

crates/loro-wasm/src/lib.rs

@@ -1781,11 +1781,14 @@ impl LoroText {
         JsValue::from_str("Text").into()
     }
 
-    /// Iterate each span(internal storage unit) of the text.
+    /// Iterate each text span(internal storage unit)
     ///
     /// The callback function will be called for each span in the text.
     /// If the callback returns `false`, the iteration will stop.
     ///
+    /// Limitation: you cannot access or alter the doc state when iterating (this is for performance consideration).
+    /// If you need to access or alter the doc state, please use `toString` instead.
+    ///
     /// @example
     /// ```ts
     /// import { LoroDoc } from "loro-crdt";
@@ -2632,7 +2635,7 @@ impl LoroList {
     #[wasm_bindgen(js_name = "toArray", method, skip_typescript)]
     pub fn to_array(&mut self) -> Vec<JsValueOrContainer> {
         let mut arr: Vec<JsValueOrContainer> = Vec::with_capacity(self.length());
-        self.handler.for_each(|(_, x)| {
+        self.handler.for_each(|x| {
             arr.push(match x {
                 ValueOrHandler::Value(v) => {
                     let v: JsValue = v.into();

crates/loro/src/lib.rs

@@ -1010,10 +1010,10 @@ impl LoroList {
     /// Iterate over the elements of the list.
     pub fn for_each<I>(&self, mut f: I)
     where
-        I: FnMut((usize, ValueOrContainer)),
+        I: FnMut(ValueOrContainer),
     {
-        self.handler.for_each(&mut |(index, v)| {
-            f((index, ValueOrContainer::from(v)));
+        self.handler.for_each(&mut |v| {
+            f(ValueOrContainer::from(v));
         })
     }
 
@@ -1379,6 +1379,9 @@ impl LoroText {
     ///
     /// The callback function will be called for each character in the text.
     /// If the callback returns `false`, the iteration will stop.
+    ///
+    /// Limitation: you cannot access or alter the doc state when iterating.
+    /// If you need to access or alter the doc state, please use `to_string` instead.
     pub fn iter(&self, callback: impl FnMut(&str) -> bool) {
         self.handler.iter(callback);
     }
@@ -2217,6 +2220,16 @@ impl LoroMovableList {
     pub fn clear(&self) -> LoroResult<()> {
         self.handler.clear()
     }
+
+    /// Iterate over the elements of the list.
+    pub fn for_each<I>(&self, mut f: I)
+    where
+        I: FnMut(ValueOrContainer),
+    {
+        self.handler.for_each(&mut |v| {
+            f(ValueOrContainer::from(v));
+        })
+    }
 }
 
 impl Default for LoroMovableList {

crates/loro/tests/loro_rust_test.rs

@@ -1994,6 +1994,48 @@ fn test_fork_when_detached() {
     assert_eq!(doc.get_text("text").to_string(), "Hello, world! Alice!");
 }
 
+#[test]
+fn test_for_each_movable_list() {
+    let doc = LoroDoc::new();
+    let list = doc.get_movable_list("list");
+    list.insert(0, 1).unwrap();
+    list.insert(1, "hello").unwrap();
+    list.insert(2, true).unwrap();
+    let mut vec = vec![];
+    list.for_each(|v| {
+        vec.push(v.into_value().unwrap());
+    });
+    assert_eq!(vec, vec![1.into(), "hello".into(), true.into()]);
+}
+
+#[test]
+fn test_for_each_map() {
+    let doc = LoroDoc::new();
+    let map = doc.get_map("map");
+    map.insert("0", 0).unwrap();
+    map.insert("1", 1).unwrap();
+    map.insert("2", 2).unwrap();
+    let mut vec = vec![];
+    map.for_each(|_, v| {
+        vec.push(v.into_value().unwrap());
+    });
+    assert_eq!(vec, vec![0.into(), 1.into(), 2.into()]);
+}
+
+#[test]
+fn test_for_each_list() {
+    let doc = LoroDoc::new();
+    let list = doc.get_list("list");
+    list.insert(0, 0).unwrap();
+    list.insert(1, 1).unwrap();
+    list.insert(2, 2).unwrap();
+    let mut vec = vec![];
+    list.for_each(|v| {
+        vec.push(v.into_value().unwrap());
+    });
+    assert_eq!(vec, vec![0.into(), 1.into(), 2.into()]);
+}
+
 #[test]
 #[should_panic]
 fn should_avoid_initialize_new_container_accidentally() {