[REL] v1.2.2

# v1.2.2

This is just a small bug fix release, because we need it in Odoo.

Bug fix: allow renderings for detached components. Before this release, Owl
ignored renderings in some cases if a component is detached.  We now still render
it, because it is important in some cases.

README.md

@@ -120,7 +120,7 @@ npm install @odoo/owl
 
 If you want to use a simple `<script>` tag, the last release can be downloaded here:
 
-- [owl-1.2.0](https://github.com/odoo/owl/releases/tag/v1.2.0)
+- [owl-1.2.2](https://github.com/odoo/owl/releases/tag/v1.2.2)
 
 ## License
 

doc/learning/tutorial_todoapp.md

@@ -545,7 +545,7 @@ application), since it involves extracting all task related code out of the
 components. Here is the new content of the `app.js` file:
 
 ```js
-const { Component, Store } = owl;
+const { Component, Store, mount } = owl;
 const { xml } = owl.tags;
 const { whenReady } = owl.utils;
 const { useRef, useDispatch, useStore } = owl.hooks;
@@ -808,7 +808,7 @@ For reference, here is the final code:
 
 ```js
 (function () {
-  const { Component, Store } = owl;
+  const { Component, Store, mount } = owl;
   const { xml } = owl.tags;
   const { whenReady } = owl.utils;
   const { useRef, useDispatch, useState, useStore } = owl.hooks;

doc/reference/component.md

@@ -10,13 +10,21 @@
   - [Static Properties](#static-properties)
   - [Methods](#methods)
   - [Lifecycle](#lifecycle)
+    - [`constructor(parent, props)`](#constructorparent-props)
+    - [`willStart()`](#willstart)
+    - [`mounted()`](#mounted)
+    - [`willUpdateProps(nextProps)`](#willupdatepropsnextprops)
+    - [`willPatch()`](#willpatch)
+    - [`patched(snapshot)`](#patchedsnapshot)
+    - [`willUnmount()`](#willunmount)
+    - [`catchError(error)`](#catcherrorerror)
   - [Root Component](#root-component)
   - [Composition](#composition)
   - [Form Input Bindings](#form-input-bindings)
   - [References](#references)
   - [Dynamic sub components](#dynamic-sub-components)
   - [Functional Components](#functional-components)
-  - [SVG components](#svg-components)
+  - [SVG Components](#svg-components)
 
 ## Overview
 
@@ -289,8 +297,13 @@ We explain here all the public methods of the `Component` class.
   are updated. It returns a boolean, which indicates if the component should
   ignore a props update. If it returns false, then `willUpdateProps` will not
   be called, and no rendering will occur. Its default implementation is to
-  always return true. This is an optimization, similar to React's `shouldComponentUpdate`. Most of the time, this should not be used, but it
-  can be useful if we are handling large number of components.
+  always return true. Note that this is an optimization, similar to React's `shouldComponentUpdate`. Most of the time, this should not be used, but it
+  can be useful if we are handling large number of components. Since this is an
+  optimization, Owl has the freedom to ignore the result of `shouldUpdate` in
+  some cases (for example, if a component is remounted, or if we want to force
+  a full rerender of the UI). However, if `shouldUpdate` returns true, then Owl
+  provides the guarantee that the component will be rendered at some point in
+  the future (except if the component is destroyed or if some part of the UI crashes).
 
 * **`destroy()`**. As its name suggests, this method will remove the component,
   and perform all necessary cleanup, such as unmounting the component, its children,

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@odoo/owl",
-  "version": "1.2.0",
+  "version": "1.2.2",
   "description": "Odoo Web Library (OWL)",
   "main": "dist/owl.cjs.js",
   "browser": "dist/owl.iife.js",
@@ -44,7 +44,7 @@
     "github-api": "^3.3.0",
     "jest": "^23.6.0",
     "jest-environment-jsdom": "^24.7.1",
-    "live-server": "^1.2.1",
+    "live-server": "^1.2.2",
     "npm-run-all": "^4.1.5",
     "prettier": "^2.0.4",
     "rollup": "^1.6.0",

roadmap.md

@@ -1,6 +1,6 @@
 # 🦉 OWL Roadmap 🦉
 
-- Current version: 1.2.0
+- Current version: 1.2.2
 - Status: stable
 
 This roadmap is only an attempt at predicting Owl's future.  Everything may

src/component/component.ts

@@ -322,7 +322,14 @@ export class Component<Props extends {} = any, T extends Env = Env> {
     }
     if (__owl__.currentFiber) {
       const currentFiber = __owl__.currentFiber;
-      if (currentFiber.target === target && currentFiber.position === position) {
+      if (!currentFiber.target && !currentFiber.position) {
+        // this means we have a pending rendering, but it was a render operation,
+        // not a mount operation. We can simply update the fiber with the target
+        // and the position
+        currentFiber.target = target;
+        currentFiber.position = position;
+        return scheduler.addFiber(currentFiber);
+      } else if (currentFiber.target === target && currentFiber.position === position) {
         return scheduler.addFiber(currentFiber);
       } else {
         scheduler.rejectFiber(currentFiber, "Mounting operation cancelled");
@@ -366,12 +373,6 @@ export class Component<Props extends {} = any, T extends Env = Env> {
   async render(force: boolean = false): Promise<void> {
     const __owl__ = this.__owl__;
     const currentFiber = __owl__.currentFiber;
-    if (!__owl__.isMounted && !currentFiber) {
-      // if we get here, this means that the component was either never mounted,
-      // or was unmounted and some state change  triggered a render. Either way,
-      // we do not want to actually render anything in this case.
-      return;
-    }
     if (currentFiber && !currentFiber.isRendered && !currentFiber.isCompleted) {
       return scheduler.addFiber(currentFiber.root);
     }
@@ -385,8 +386,6 @@ export class Component<Props extends {} = any, T extends Env = Env> {
         if (fiber.isCompleted) {
           return;
         }
-        // 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

src/component/fiber.ts

@@ -82,6 +82,7 @@ export class Fiber {
 
     let oldFiber = __owl__.currentFiber;
     if (oldFiber && !oldFiber.isCompleted) {
+      this.force = true;
       if (oldFiber.root === oldFiber && !parent) {
         // both oldFiber and this fiber are root fibers
         this._reuseFiber(oldFiber);
@@ -187,7 +188,8 @@ export class Fiber {
   complete() {
     let component = this.component;
     this.isCompleted = true;
-    if (!this.target && !component.__owl__.isMounted) {
+    const { isMounted, isDestroyed } = component.__owl__;
+    if (isDestroyed) {
       return;
     }
 
@@ -201,14 +203,16 @@ export class Fiber {
     const patchLen = patchQueue.length;
 
     // call willPatch hook on each fiber of patchQueue
-    for (let i = 0; i < patchLen; i++) {
-      const fiber = patchQueue[i];
-      if (fiber.shouldPatch) {
-        component = fiber.component;
-        if (component.__owl__.willPatchCB) {
-          component.__owl__.willPatchCB();
+    if (isMounted) {
+      for (let i = 0; i < patchLen; i++) {
+        const fiber = patchQueue[i];
+        if (fiber.shouldPatch) {
+          component = fiber.component;
+          if (component.__owl__.willPatchCB) {
+            component.__owl__.willPatchCB();
+          }
+          component.willPatch();
         }
-        component.willPatch();
       }
     }
 
@@ -270,16 +274,18 @@ export class Fiber {
     }
 
     // call patched/mounted hook on each fiber of (reversed) patchQueue
-    for (let i = patchLen - 1; i >= 0; i--) {
-      const fiber = patchQueue[i];
-      component = fiber.component;
-      if (fiber.shouldPatch && !this.target) {
-        component.patched();
-        if (component.__owl__.patchedCB) {
-          component.__owl__.patchedCB();
+    if (isMounted || inDOM) {
+      for (let i = patchLen - 1; i >= 0; i--) {
+        const fiber = patchQueue[i];
+        component = fiber.component;
+        if (fiber.shouldPatch && !this.target) {
+          component.patched();
+          if (component.__owl__.patchedCB) {
+            component.__owl__.patchedCB();
+          }
+        } else {
+          component.__callMounted();
         }
-      } else if (this.target ? inDOM : true) {
-        component.__callMounted();
       }
     }
   }

src/store.ts

@@ -75,6 +75,11 @@ export class Store extends Context {
     );
     return result;
   }
+
+  __notifyComponents(): Promise<void> {
+    this.trigger("before-update");
+    return super.__notifyComponents();
+  }
 }
 
 interface SelectorOptions {
@@ -105,13 +110,16 @@ export function useStore(selector, options: SelectorOptions = {}): any {
     const newRevNumber = hashFn(result);
     if ((newRevNumber > 0 && revNumber !== newRevNumber) || !isEqual(oldResult, result)) {
       revNumber = newRevNumber;
-      if (options.onUpdate) {
-        options.onUpdate(result);
-      }
       return true;
     }
     return false;
   }
+  if (options.onUpdate) {
+    store.on("before-update", component, () => {
+      const newValue = selector(store!.state, component.props!);
+      options.onUpdate(newValue);
+    });
+  }
   store.updateFunctions[componentId].push(function (): boolean {
     return selectCompareUpdate(store!.state, component.props);
   });
@@ -132,6 +140,9 @@ export function useStore(selector, options: SelectorOptions = {}): any {
   const __destroy = component.__destroy;
   component.__destroy = (parent) => {
     delete store.updateFunctions[componentId];
+    if (options.onUpdate) {
+      store.off("before-update", component);
+    }
     __destroy.call(component, parent);
   };
 

tests/component/async.test.ts

@@ -1405,4 +1405,160 @@ describe("async rendering", () => {
     expect(fixture.innerHTML).toBe("<div>2</div>");
     expect(Widget.prototype.__render).toHaveBeenCalledTimes(2);
   });
+
+  test("components with shouldUpdate=false", async () => {
+    const state = { p: 1, cc: 10 };
+
+    class ChildChild extends Component {
+      static template = xml`
+        <div>
+          child child: <t t-esc="state.cc"/>
+        </div>`;
+      state = state;
+      shouldUpdate() {
+        return false;
+      }
+    }
+
+    class Child extends Component {
+      static components = { ChildChild };
+      static template = xml`
+        <div>
+          child
+          <ChildChild/>
+        </div>`;
+
+      shouldUpdate() {
+        return false;
+      }
+    }
+
+    let parent: any;
+    class Parent extends Component {
+      static components = { Child };
+      static template = xml`
+        <div>
+          parent: <t t-esc="state.p"/>
+          <Child/>
+        </div>`;
+
+      state = state;
+      constructor(a, b) {
+        super(a, b);
+        parent = this;
+      }
+      shouldUpdate() {
+        return false;
+      }
+    }
+
+    class App extends Component {
+      static components = { Parent };
+      static template = xml`
+        <div>
+          <Parent/>
+        </div>`;
+    }
+
+    var div = document.createElement("div");
+    fixture.appendChild(div);
+
+    const app = new App();
+
+    await app.mount(fixture);
+    expect(fixture.innerHTML).toBe(
+      "<div></div><div><div> parent: 1<div> child <div> child child: 10</div></div></div></div>"
+    );
+    app.mount(div);
+
+    // wait for rendering from second mount to go through parent
+    await Promise.resolve();
+    await Promise.resolve();
+    state.cc++;
+    state.p++;
+    parent.render();
+    await nextTick();
+    expect(fixture.innerHTML).toBe(
+      "<div><div><div> parent: 2<div> child <div> child child: 11</div></div></div></div></div>"
+    );
+  });
+
+  test("components with shouldUpdate=false, part 2", async () => {
+    const state = { p: 1, cc: 10 };
+    let shouldUpdate = true;
+
+    class ChildChild extends Component {
+      static template = xml`
+        <div>
+          child child: <t t-esc="state.cc"/>
+        </div>`;
+      state = state;
+      shouldUpdate() {
+        return shouldUpdate;
+      }
+    }
+
+    class Child extends Component {
+      static components = { ChildChild };
+      static template = xml`
+        <div>
+          child
+          <ChildChild/>
+        </div>`;
+
+      shouldUpdate() {
+        return shouldUpdate;
+      }
+    }
+
+    let parent: any;
+    class Parent extends Component {
+      static components = { Child };
+      static template = xml`
+        <div>
+          parent: <t t-esc="state.p"/>
+          <Child/>
+        </div>`;
+
+      state = state;
+      constructor(a, b) {
+        super(a, b);
+        parent = this;
+      }
+      shouldUpdate() {
+        return shouldUpdate;
+      }
+    }
+
+    class App extends Component {
+      static components = { Parent };
+      static template = xml`
+        <div>
+          <Parent/>
+        </div>`;
+    }
+
+    const app = new App();
+
+    await app.mount(fixture);
+    expect(fixture.innerHTML).toBe(
+      "<div><div> parent: 1<div> child <div> child child: 10</div></div></div></div>"
+    );
+
+    state.cc++;
+    state.p++;
+    app.render();
+
+    // wait for rendering to go through child
+    await Promise.resolve();
+    await Promise.resolve();
+    await Promise.resolve();
+
+    shouldUpdate = false;
+    parent.render();
+    await nextTick();
+    expect(fixture.innerHTML).toBe(
+      "<div><div> parent: 2<div> child <div> child child: 11</div></div></div></div>"
+    );
+  });
 });

tests/component/component.test.ts

@@ -962,6 +962,69 @@ describe("lifecycle hooks", () => {
       "parent:patched",
     ]);
   });
+
+  test("willPatch/patched hook is not called if not mounted in DOM", async () => {
+    const steps: string[] = [];
+
+    class ChildWidget extends Component {
+      static template = xml`<div/>`;
+      constructor(parent, props) {
+        super(parent, props);
+        steps.push("child:constructor");
+      }
+      mounted() {
+        steps.push("child:mounted");
+      }
+      willPatch() {
+        steps.push("child:willPatch");
+      }
+      patched() {
+        steps.push("child:patched");
+      }
+    }
+    class ParentWidget extends Component {
+      static template = xml`
+        <div>
+          <t t-component="child" v="state.n"/>
+        </div>
+      `;
+      static components = { child: ChildWidget };
+      state = useState({ n: 1 });
+      constructor() {
+        super();
+        steps.push("parent:constructor");
+      }
+      mounted() {
+        steps.push("parent:mounted");
+      }
+      willPatch() {
+        steps.push("parent:willPatch");
+      }
+      patched() {
+        steps.push("parent:patched");
+      }
+    }
+
+    const div = document.createElement("div");
+    const widget = new ParentWidget();
+    await widget.mount(div);
+    expect(steps).toEqual(["parent:constructor", "child:constructor"]);
+
+    widget.state.n = 2;
+    await nextTick();
+
+    expect(steps).toEqual(["parent:constructor", "child:constructor"]);
+
+    // then we remount the component in the dom
+    await widget.mount(fixture);
+
+    expect(steps).toEqual([
+      "parent:constructor",
+      "child:constructor",
+      "child:mounted",
+      "parent:mounted",
+    ]);
+  });
 });
 
 describe("destroy method", () => {

tests/component/un_mounting.test.ts

@@ -323,6 +323,38 @@ describe("unmounting and remounting", () => {
     expect(steps).toEqual([2, 2, 3]);
   });
 
+  test("change state and render while mounted in detached dom", async () => {
+    class App extends Component {
+      static template = xml`<div><t t-esc="state.val"/></div>`;
+      state = useState({ val: 1 });
+    }
+
+    const detachedDiv = document.createElement("div");
+    const app = await mount(App, { target: detachedDiv });
+
+    expect(detachedDiv.innerHTML).toBe("<div>1</div>");
+    app.state.val = 2;
+    await nextTick();
+    expect(detachedDiv.innerHTML).toBe("<div>2</div>");
+  });
+
+  test("destroy and change state after mounted in detached dom", async () => {
+    class App extends Component {
+      static template = xml`<div><t t-esc="state.val"/></div>`;
+      state = useState({ val: 1 });
+    }
+
+    const detachedDiv = document.createElement("div");
+    const app = await mount(App, { target: detachedDiv });
+
+    expect(detachedDiv.innerHTML).toBe("<div>1</div>");
+
+    app.destroy();
+    app.state.val = 2;
+    await nextTick();
+    expect(detachedDiv.innerHTML).toBe("");
+  });
+
   test("change state while component is unmounted", async () => {
     let child;
     class Child extends Component {

tests/store_hooks.test.ts

@@ -571,12 +571,12 @@ describe("connecting a component to store", () => {
     app.state.beerId = 2;
     await nextTick();
     expect(fixture.innerHTML).toBe("<div><span>kwak</span></div>");
-    expect(counter).toBe(1);
+    expect(counter).toBe(0);
 
     store.dispatch("renameBeer", { id: 2, name: "orval" });
     await nextTick();
     expect(fixture.innerHTML).toBe("<div><span>orval</span></div>");
-    expect(counter).toBe(2);
+    expect(counter).toBe(1);
   });
 
   test("connected component is properly cleaned up on destroy", async () => {
@@ -1345,4 +1345,58 @@ describe("various scenarios", () => {
     await nextTick();
     expect(fixture.innerHTML).toBe("<div><div>testWorld<div>3</div></div></div>");
   });
+
+  test("parent/children with store, parent is remounted", async () => {
+    const store = new Store({ state: { a: 1, b: 1 } });
+
+    class Child extends Component {
+      static template = xml`<div><t t-esc="a"/></div>`;
+      a: any;
+      constructor(parent, props) {
+        super(parent, props);
+        this.a = useStore(
+          (state, props) => {
+            return state.a;
+          },
+          {
+            onUpdate: (a) => {
+              this.a = a;
+            },
+          }
+        );
+      }
+    }
+
+    class Parent extends Component {
+      static template = xml`
+        <div>
+          parent: <t t-esc="b"/>
+          <Child/>
+        </div>`;
+      static components = { Child };
+
+      b: any;
+      constructor(parent, props) {
+        super(parent, props);
+        this.b = useStore((state, props) => {
+          return state.b;
+        });
+      }
+    }
+    (env as any).store = store;
+
+    const div = document.createElement("div");
+    fixture.appendChild(div);
+
+    // initial mounting
+    const parent = await mount(Parent, { target: fixture, env });
+    expect(fixture.innerHTML).toBe("<div></div><div> parent: 1<div>1</div></div>");
+
+    // remounting component, then immediately update store.state
+    parent.mount(div);
+    store.state.a++;
+
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<div><div> parent: 1<div>2</div></div></div>");
+  });
 });

tests/tooling/debug_script_3.test.ts

@@ -8,7 +8,7 @@ import * as owl from "../../src/index";
 
 import { Component, Env } from "../../src/component/component";
 import { xml } from "../../src/tags";
-import { makeTestFixture, makeTestEnv } from "../helpers";
+import { makeTestFixture, makeTestEnv, nextTick } from "../helpers";
 
 let fixture: HTMLElement = makeTestFixture();
 let env: Env = makeTestEnv();
@@ -31,6 +31,7 @@ test("log a specific message for render method calls if component is not mounted
   parent.unmount();
   parent.state.value = 2;
 
+  await nextTick();
   expect(steps).toEqual([
     "[OWL_DEBUG] Parent<id=1> constructor, props={}",
     "[OWL_DEBUG] Parent<id=1> mount",
@@ -40,7 +41,10 @@ test("log a specific message for render method calls if component is not mounted
     "[OWL_DEBUG] Parent<id=1> mounted",
     "[OWL_DEBUG] scheduler: stop running tasks queue",
     "[OWL_DEBUG] Parent<id=1> willUnmount",
-    "[OWL_DEBUG] Parent<id=1> render (warning: component is not mounted, this render has no effect)",
+    "[OWL_DEBUG] Parent<id=1> render (warning: component is not mounted)",
+    "[OWL_DEBUG] scheduler: start running tasks queue",
+    "[OWL_DEBUG] Parent<id=1> rendering template",
+    "[OWL_DEBUG] scheduler: stop running tasks queue",
   ]);
   console.log = log;
 });

tools/debug.js

@@ -102,7 +102,7 @@
       const __owl__ = component.__owl__;
       let msg = `render`;
       if (!__owl__.isMounted && !__owl__.currentFiber) {
-        msg += ` (warning: component is not mounted, this render has no effect)`;
+        msg += ` (warning: component is not mounted)`;
       }
       log(msg);
       return render(...args);