[IMP] hooks/component: remove updateEnv, add useSubEnv

closes #182

doc/hooks.md

@@ -13,6 +13,7 @@
   - [`onWillPatch`](#onwillpatch)
   - [`onPatched`](#onpatched)
   - [`useRef`](#useref)
+  - [`useSubEnv`](#useSubEnv)
 
 ## Overview
 
@@ -246,3 +247,29 @@ this.ref2 = useRef("component_2");
 
 References are only guaranteed to be active while the parent component is mounted.
 If this is not the case, accessing `el` or `comp` on it will return `null`.
+
+### `useSubEnv`
+
+The environment is sometimes useful to share some common information between
+all components.  But sometimes, we want to *scope* that knowledge to a subtree.
+
+For example, if we have a form view component, maybe we would like to make some
+`model` object available to all sub component, but not to the whole application.
+This is where the `useSubEnv` hook may be useful: it let a component add some
+information to the environment in a way that only the component and its children
+can access it:
+
+```js
+class FormComponent extends Component {
+  constructor(...args) {
+    super(...args);
+    const model = makeModel();
+    useSubEnv({ model });
+  }
+}
+```
+
+The `useSubEnv` takes one argument: an object which contains some key/value that
+will be added to the parent environment. Note that it will extend, not replace
+the parent environment.  And of course, the parent environment will not be
+affected.

doc/readme.md

@@ -20,6 +20,7 @@ owl
         onPatched
         useState
         useRef
+        useSubEnv
     router
         Link
         RouteComponent

src/component/component.ts

@@ -380,26 +380,6 @@ export class Component<T extends Env, Props extends {}> {
     return true;
   }
 
-  /**
-   * This method is the correct way to update the environment of a component. Doing
-   * this will cause a full rerender of the component and its children, so this is
-   * an operation that should not be done frequently.
-   *
-   * A good usecase for updating the environment would be to update some mostly
-   * static config keys, such as a boolean to determine if we are in mobile
-   * mode or not.
-   */
-  async updateEnv(nextEnv: Partial<T>): Promise<void> {
-    const __owl__ = this.__owl__;
-    if (__owl__.parent && __owl__.parent.env === this.env) {
-      this.env = Object.create(this.env);
-    }
-    Object.assign(this.env, nextEnv);
-    if (__owl__.isMounted) {
-      await this.render(true);
-    }
-  }
-
   /**
    * Emit a custom event of type 'eventType' with the given 'payload' on the
    * component's el, if it exists. However, note that the event will only bubble

src/hooks.ts

@@ -12,9 +12,11 @@ import { Observer } from "./core/observer";
  * - useRef
  */
 
+// -----------------------------------------------------------------------------
+// useState
+// -----------------------------------------------------------------------------
+
 /**
- * useState hook
- *
  * This is the main way a component can be made reactive.  The useState hook
  * will return an observed object (or array).  Changes to that value will then
  * trigger a rerendering of the current component.
@@ -29,6 +31,9 @@ export function useState<T>(state: T): T {
   return __owl__.observer.observe(state);
 }
 
+// -----------------------------------------------------------------------------
+// Life cycle hooks
+// -----------------------------------------------------------------------------
 function makeLifecycleHook(method: string, reverse: boolean = false) {
   return function(cb) {
     const component: Component<any, any> = Component._current;
@@ -51,17 +56,16 @@ function makeLifecycleHook(method: string, reverse: boolean = false) {
   };
 }
 
-/**
- * willUnmount hook. The callback will be called when the current component is
- * willUnmounted.  Note that the component mounted method is called last.
- */
 export const onMounted = makeLifecycleHook("mountedCB", true);
 export const onWillUnmount = makeLifecycleHook("willUnmountCB");
 export const onWillPatch = makeLifecycleHook("willPatchCB");
 export const onPatched = makeLifecycleHook("patchedCB", true);
+
+// -----------------------------------------------------------------------------
+// useRef
+// -----------------------------------------------------------------------------
+
 /**
- * useRef hook
- *
  * The purpose of this hook is to allow components to get a reference to a sub
  * html node or component.
  */
@@ -83,3 +87,18 @@ export function useRef(name: string): Ref {
     }
   };
 }
+
+// -----------------------------------------------------------------------------
+// useSubEnv
+// -----------------------------------------------------------------------------
+
+
+/**
+ * This hook is a simple way to let components use a sub environment.  Note that
+ * like for all hooks, it is important that this is only called in the
+ * constructor method.
+ */
+export function useSubEnv(nextEnv) {
+  const component = Component._current;
+  component.env = Object.assign(Object.create(component.env), nextEnv);
+}

tests/component/component.test.ts

@@ -625,24 +625,6 @@ describe("lifecycle hooks", () => {
     expect(n).toBe(1);
   });
 
-  test("patched hook is called after updateEnv", async () => {
-    let n = 0;
-
-    class TestWidget extends Widget {
-      state = useState({ a: 1 });
-
-      patched() {
-        n++;
-      }
-    }
-    const widget = new TestWidget(env);
-    await widget.mount(fixture);
-    expect(n).toBe(0);
-
-    await widget.updateEnv({ isMobile: true });
-    expect(n).toBe(1);
-  });
-
   test("shouldUpdate hook prevent rerendering", async () => {
     let shouldUpdate = false;
     class TestWidget extends Widget {
@@ -2815,107 +2797,6 @@ describe("async rendering", () => {
   });
 });
 
-describe("updating environment", () => {
-  test("can update widget env", async () => {
-    const widget = new Widget(env);
-    expect(widget.env).toBe(env);
-    await widget.updateEnv(<any>{ somekey: 4 });
-    expect(widget.env).toBe(env);
-    expect((<any>widget).env.somekey).toBe(4);
-  });
-
-  test("updating widget env does not render widget (if not mounted)", async () => {
-    let n = 0;
-    class TestWidget extends Widget {
-      __render(f) {
-        n++;
-        return super.__render(f);
-      }
-    }
-
-    const widget = new TestWidget(env);
-    expect(n).toBe(0);
-    await widget.updateEnv(<any>{ somekey: 4 });
-    expect(n).toBe(0);
-    await widget.mount(fixture);
-    expect(n).toBe(1);
-    await widget.updateEnv(<any>{ somekey: 5 });
-    expect(n).toBe(2);
-    widget.unmount();
-    expect(n).toBe(2);
-    await widget.updateEnv(<any>{ somekey: 5 });
-    expect(n).toBe(2);
-  });
-
-  test("updating child env does not modify parent env", async () => {
-    env.qweb.addTemplate("ParentWidget", `<div><t t-component="child"/></div>`);
-    class ParentWidget extends Widget {
-      static components = { child: Widget };
-    }
-    const parent = new ParentWidget(env);
-    await parent.mount(fixture);
-    const child = children(parent)[0];
-    expect(child.env).toBe(parent.env);
-    await child.updateEnv(<any>{ somekey: 4 });
-    expect(child.env).not.toBe(parent.env);
-    expect((<any>parent).env.somekey).toBeUndefined();
-  });
-
-  test("updating parent env does modify child env", async () => {
-    env.qweb.addTemplate("ParentWidget", `<div><t t-component="child"/></div>`);
-    class ParentWidget extends Widget {
-      static components = { child: Widget };
-    }
-    const parent = new ParentWidget(env);
-    await parent.mount(fixture);
-    const child = children(parent)[0];
-    expect(child.env.somekey).toBeUndefined();
-    await parent.updateEnv({ somekey: 4 });
-    expect(child.env.somekey).toBe(4);
-  });
-
-  test("updating parent env does modify child env, part 2", async () => {
-    env.qweb.addTemplate("ParentWidget", `<div><Child/></div>`);
-    class ParentWidget extends Widget {
-      static components = { Child: Widget };
-    }
-    const parent = new ParentWidget(env);
-    await parent.mount(fixture);
-    const child = children(parent)[0];
-    expect(child.env.somekey).toBeUndefined();
-    await child.updateEnv({ somekey: 4 });
-    await parent.updateEnv({ someotherkey: 4 });
-    expect(child.env.someotherkey).toBe(4);
-  });
-
-  test("updating env force a rerender", async () => {
-    env.qweb.addTemplate("TestWidget", `<div><t t-esc="env.someKey"/></div>`);
-    class TestWidget extends Widget {}
-    (<any>env).someKey = "hey";
-    const widget = new TestWidget(env);
-    await widget.mount(fixture);
-    expect(fixture.innerHTML).toBe("<div>hey</div>");
-    await widget.updateEnv(<any>{ someKey: "rerendered" });
-    expect(fixture.innerHTML).toBe("<div>rerendered</div>");
-  });
-
-  test("updating env force rerendering children", async () => {
-    env.qweb.addTemplate("Parent", `<div><Child /></div>`);
-    class Child extends Widget {}
-    class Parent extends Widget {
-      static components = { Child };
-    }
-    env.qweb.addTemplate("Child", `<div><t t-esc="env.someKey"/></div>`);
-    (<any>env).someKey = "hey";
-    const widget = new Parent(env);
-    await widget.mount(fixture);
-    expect(fixture.innerHTML).toBe("<div><div>hey</div></div>");
-    await widget.updateEnv(<any>{ someKey: "rerendered" });
-    await nextTick();
-    expect(fixture.innerHTML).toBe("<div><div>rerendered</div></div>");
-  });
-});
-
 describe("widget and observable state", () => {
   test("widget is rerendered when its state is changed", async () => {
     env.qweb.addTemplate("TestWidget", `<div><t t-esc="state.drink"/></div>`);

tests/hooks.test.ts

@@ -1,6 +1,14 @@
 import { makeTestEnv, makeTestFixture, nextTick } from "./helpers";
 import { Component, Env } from "../src/component/component";
-import { useState, onMounted, onWillUnmount, useRef, onPatched, onWillPatch } from "../src/hooks";
+import {
+  useState,
+  onMounted,
+  onWillUnmount,
+  useRef,
+  onPatched,
+  onWillPatch,
+  useSubEnv
+} from "../src/hooks";
 import { xml } from "../src/tags";
 
 //------------------------------------------------------------------------------
@@ -245,12 +253,7 @@ describe("hooks", () => {
     await nextTick();
     expect(fixture.innerHTML).toBe("<div>hey2</div>");
 
-    expect(steps).toEqual([
+    expect(steps).toEqual(["hook:willPatch2", "hook:willPatch1", "hook:patched1", "hook:patched2"]);
-      "hook:willPatch2",
-      "hook:willPatch1",
-      "hook:patched1",
-      "hook:patched2"
-    ]);
   });
 
   describe("autofocus hook", () => {
@@ -316,4 +319,41 @@ describe("hooks", () => {
       expect(input2).toBe(document.activeElement);
     });
   });
+
+  test("can use sub env", async () => {
+    class TestComponent extends Component<any, any> {
+      static template = xml`<div><t t-esc="env.val"/></div>`;
+      constructor(env) {
+        super(env);
+        useSubEnv({ val: 3 });
+      }
+    }
+    const component = new TestComponent(env);
+    await component.mount(fixture);
+    expect(fixture.innerHTML).toBe("<div>3</div>");
+    expect(env).not.toHaveProperty("val");
+    expect(component.env).toHaveProperty("val");
+  });
+
+  test("parent and child env", async () => {
+    class Child extends Component<any, any> {
+      static template = xml`<div><t t-esc="env.val"/></div>`;
+      constructor(env) {
+        super(env);
+        useSubEnv({ val: 5 });
+      }
+    }
+
+    class Parent extends Component<any, any> {
+      static template = xml`<div><t t-esc="env.val"/><Child/></div>`;
+      static components = { Child}
+      constructor(env) {
+        super(env);
+        useSubEnv({ val: 3 });
+      }
+    }
+    const component = new Parent(env);
+    await component.mount(fixture);
+    expect(fixture.innerHTML).toBe( "<div>3<div>5</div></div>");
+  });
 });