[REF] observer: notify subscribers directly

This rev. moves the logic of batching the notifications of changes
occurring in the same microtask tick, from the observer, to the
listeners (the context, and the render function of components).
By doing so in render, we ensure that similar renderings are
batched in a single one, wherever they come from (state change,
store change, direct call...).

src/component/component.ts

@@ -328,8 +328,23 @@ export class Component<T extends Env, Props extends {}> {
     if (__owl__.currentFiber && !__owl__.currentFiber.isRendered) {
       return scheduler.addFiber(__owl__.currentFiber.root);
     }
+    // if we aren't mounted at this point, it implies that there is a
+    // currentFiber that is already rendered (isRendered is true), so we are
+    // about to be mounted
+    const isMounted = __owl__.isMounted;
     const fiber = new Fiber(null, this, undefined, undefined, force, null);
-    this.__render(fiber);
+    Promise.resolve().then(() => {
+      if (__owl__.isMounted || !isMounted) {
+        // we are mounted (__owl__.isMounted), or if we are currently being
+        // mounted (!isMounted), so we call __render
+        this.__render(fiber);
+      } else {
+        // we were mounted when render was called, but we aren't anymore, so we
+        // were actually about to be unmounted ; we can thus forget about this
+        // fiber
+        __owl__.currentFiber = null;
+      }
+    });
     return scheduler.addFiber(fiber);
   }
 

src/component/fiber.ts

@@ -173,10 +173,8 @@ export class Fiber {
       if (document.body.contains(this.target)) {
         component.__callMounted();
       }
-    } else {
-      if (component.__owl__.isMounted && this === this.root) {
-        this.patchComponents();
-      }
+    } else if (component.__owl__.isMounted && this === this.root) {
+      this.patchComponents();
     }
     this.isCompleted = true;
   }

src/component/scheduler.ts

@@ -34,8 +34,7 @@ export class Scheduler {
         fiber,
         callback: () => {
           if (fiber.error) {
-            reject(fiber.error);
-            return;
+            return reject(fiber.error);
           }
           resolve();
         }

src/context.ts

@@ -45,7 +45,16 @@ export class Context extends EventBus {
   constructor(state: Object = {}) {
     super();
     this.observer = new Observer();
-    this.observer.notifyCB = this.__notifyComponents.bind(this);
+    this.observer.notifyCB = () => {
+      // notify components in the next microtask tick to ensure that subscribers
+      // are notified only once for all changes that occur in the same micro tick
+      let rev = this.rev;
+      return Promise.resolve().then(() => {
+        if (rev === this.rev) {
+          this.__notifyComponents();
+        }
+      });
+    };
     this.state = this.observer.observe(state);
     this.subscriptions.update = [];
   }

src/core/observer.ts

@@ -24,14 +24,6 @@ export class Observer {
   weakMap: WeakMap<any, any> = new WeakMap();
 
   notifyCB() {}
-  async notifyChange() {
-    this.dirty = true;
-    await Promise.resolve();
-    if (this.dirty) {
-      this.dirty = false;
-      this.notifyCB();
-    }
-  }
 
   observe<T>(value: T, parent?: any): T {
     if (value === null || typeof value !== "object" || value instanceof Date) {
@@ -65,7 +57,7 @@ export class Observer {
           }
           self._updateRevNumber(target);
           target[key] = newVal;
-          self.notifyChange();
+          self.notifyCB();
         }
         return true;
       },
@@ -73,7 +65,7 @@ export class Observer {
         if (key in target) {
           delete target[key];
           self._updateRevNumber(target);
-          self.notifyChange();
+          self.notifyCB();
         }
         return true;
       }

src/hooks.ts

@@ -24,19 +24,9 @@ import { Observer } from "./core/observer";
 export function useState<T>(state: T): T {
   const component: Component<any, any> = Component.current!;
   const __owl__ = component.__owl__;
-  const renderFn = __owl__.renderFn;
-  let lastRenderRevNumber;
-  __owl__.renderFn = function(comp, params) {
-    lastRenderRevNumber = __owl__.observer!.rev;
-    return renderFn(comp, params);
-  };
   if (!__owl__.observer) {
     __owl__.observer = new Observer();
-    __owl__.observer.notifyCB = () => {
-      if (lastRenderRevNumber < __owl__.observer!.rev) {
-        component.render();
-      }
-    };
+    __owl__.observer.notifyCB = component.render.bind(component);
   }
   return __owl__.observer.observe(state);
 }

tests/component/component.test.ts

@@ -3400,10 +3400,6 @@ describe("async rendering", () => {
   });
 
   test("change state and call manually render: no unnecessary rendering", async () => {
-    // when the state is changed, the component isn't notified directly (we wait
-    // for a microtask tick before calling 'render'), so it may happen that
-    // another rendering is done meanwhile, already using the new value of the
-    // state
     class Widget extends Component<any, any> {
       static template = xml`<div><t t-esc="state.val"/></div>`;
       state = useState({ val: 1 });

tests/core/observer.test.ts

@@ -408,7 +408,7 @@ describe("observer", () => {
     obj.a = 111;
     obj.f = 222;
     await nextMicroTick();
-    expect(observer.notifyCB).toBeCalledTimes(4);
+    expect(observer.notifyCB).toBeCalledTimes(5);
   });
 
   test("throw error when state is mutated in object if allowMutation=false", async () => {