From 0625b5883a7eb0e81b130d29dd32aa45b50a4a8b Mon Sep 17 00:00:00 2001
From: Samuel Degueldre <sad@odoo.com>
Date: Fri, 11 Mar 2022 13:39:04 +0100
Subject: [PATCH] [IMP] reactivity: add missing support for forEach method

---
 src/reactivity.ts        | 24 +++++++++++++++++
 tests/reactivity.test.ts | 57 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+)

diff --git a/src/reactivity.ts b/src/reactivity.ts
index a4980d91..29937005 100644
--- a/src/reactivity.ts
+++ b/src/reactivity.ts
@@ -308,6 +308,28 @@ function makeIteratorObserver(
     }
   };
 }
+/**
+ * Creates a forEach function that will delegate to forEach on the underlying
+ * collection while observing key changes, and keys as they're iterated over,
+ * and making the passed keys/values reactive.
+ *
+ * @param target @see reactive
+ * @param callback @see reactive
+ */
+function makeForEachObserver(target: any, callback: Callback) {
+  return function forEach(forEachCb: (val: any, key: any, target: any) => void, thisArg: any) {
+    observeTargetKey(target, KEYCHANGES, callback);
+    target.forEach(function (val: any, key: any, targetObj: any) {
+      observeTargetKey(target, key, callback);
+      forEachCb.call(
+        thisArg,
+        possiblyReactive(val, callback),
+        possiblyReactive(key, callback),
+        possiblyReactive(targetObj, callback)
+      );
+    }, thisArg);
+  };
+}
 /**
  * Creates a function that will delegate to an underlying method, and check if
  * that method has modified the presence or value of a key, and notify the
@@ -370,6 +392,7 @@ const rawTypeToFuncHandlers = {
     values: makeIteratorObserver("values", target, callback),
     entries: makeIteratorObserver("entries", target, callback),
     [Symbol.iterator]: makeIteratorObserver(Symbol.iterator, target, callback),
+    forEach: makeForEachObserver(target, callback),
     clear: makeClearNotifier(target),
     get size() {
       observeTargetKey(target, KEYCHANGES, callback);
@@ -385,6 +408,7 @@ const rawTypeToFuncHandlers = {
     values: makeIteratorObserver("values", target, callback),
     entries: makeIteratorObserver("entries", target, callback),
     [Symbol.iterator]: makeIteratorObserver(Symbol.iterator, target, callback),
+    forEach: makeForEachObserver(target, callback),
     clear: makeClearNotifier(target),
     get size() {
       observeTargetKey(target, KEYCHANGES, callback);
diff --git a/tests/reactivity.test.ts b/tests/reactivity.test.ts
index 00a48ab8..7647cf37 100644
--- a/tests/reactivity.test.ts
+++ b/tests/reactivity.test.ts
@@ -1245,6 +1245,33 @@ describe("Collections", () => {
       reactiveObj.a = 1; // setting same value again shouldn't notify
       expect(observer).toHaveBeenCalledTimes(1);
     });
+
+    test("iterating with forEach returns reactives", async () => {
+      const keyObj = { a: 2 };
+      const thisArg = {};
+      const observer = jest.fn();
+      const state = reactive(new Set([keyObj]), observer);
+      let reactiveKeyObj: any, reactiveValObj: any, thisObj: any, mapObj: any;
+      state.forEach(function (this: any, val, key, map) {
+        [reactiveValObj, reactiveKeyObj, mapObj, thisObj] = [val, key, map, this];
+      }, thisArg);
+      expect(reactiveKeyObj).not.toBe(keyObj);
+      expect(reactiveValObj).not.toBe(keyObj);
+      expect(mapObj).toBe(state); // third argument should be the reactive
+      expect(thisObj).toBe(thisArg); // thisArg should not be made reactive
+      expect(toRaw(reactiveKeyObj as any)).toBe(keyObj);
+      expect(toRaw(reactiveValObj as any)).toBe(keyObj);
+      expect(reactiveKeyObj).toBe(reactiveValObj); // reactiveKeyObj and reactiveValObj should be the same object
+      reactiveKeyObj!.a = 0;
+      reactiveValObj!.a = 0;
+      expect(observer).toHaveBeenCalledTimes(0);
+      reactiveKeyObj!.a; // observe key "a" in key sub-reactive;
+      reactiveKeyObj!.a = 1;
+      expect(observer).toHaveBeenCalledTimes(1);
+      reactiveKeyObj!.a = 1; // setting same value again shouldn't notify
+      reactiveValObj!.a = 1;
+      expect(observer).toHaveBeenCalledTimes(1);
+    });
   });
 
   describe("WeakSet", () => {
@@ -1467,6 +1494,36 @@ describe("Collections", () => {
       reactiveValObj.a = 1;
       expect(observer).toHaveBeenCalledTimes(2);
     });
+
+    test("iterating with forEach returns reactives", async () => {
+      const keyObj = { a: 2 };
+      const valObj = { a: 2 };
+      const thisArg = {};
+      const observer = jest.fn();
+      const state = reactive(new Map([[keyObj, valObj]]), observer);
+      let reactiveKeyObj: any, reactiveValObj: any, thisObj: any, mapObj: any;
+      state.forEach(function (this: any, val, key, map) {
+        [reactiveValObj, reactiveKeyObj, mapObj, thisObj] = [val, key, map, this];
+      }, thisArg);
+      expect(reactiveKeyObj).not.toBe(keyObj);
+      expect(reactiveValObj).not.toBe(valObj);
+      expect(mapObj).toBe(state); // third argument should be the reactive
+      expect(thisObj).toBe(thisArg); // thisArg should not be made reactive
+      expect(toRaw(reactiveKeyObj as any)).toBe(keyObj);
+      expect(toRaw(reactiveValObj as any)).toBe(valObj);
+      reactiveKeyObj!.a = 0;
+      reactiveValObj!.a = 0;
+      expect(observer).toHaveBeenCalledTimes(0);
+      reactiveKeyObj!.a; // observe key "a" in key sub-reactive;
+      reactiveKeyObj!.a = 1;
+      expect(observer).toHaveBeenCalledTimes(1);
+      reactiveValObj!.a; // observe key "a" in val sub-reactive;
+      reactiveValObj!.a = 1;
+      expect(observer).toHaveBeenCalledTimes(2);
+      reactiveKeyObj!.a = 1; // setting same value again shouldn't notify
+      reactiveValObj!.a = 1;
+      expect(observer).toHaveBeenCalledTimes(2);
+    });
   });
 
   describe("WeakMap", () => {