[REM] component: remove t-keepalive directive

We don't see any usecase for it, it makes the code more complex,
and there were still potential unresolved concurrency issues with
it.

Part of task #295

doc/reference/component.md

@@ -391,8 +391,7 @@ scrollbar.
 
 Note that modifying the state is not allowed here. This method is called just
 before an actual DOM patch, and is only intended to be used to save some local
-DOM state. Also, it will not be called if the component is not in the DOM (this can
+DOM state. Also, it will not be called if the component is not in the DOM.
-happen with components with `t-keepalive`).
 
 #### `patched(snapshot)`
 
@@ -402,7 +401,7 @@ likely via a change in its state/props or environment).
 This method is not called on the initial render. It is useful to interact
 with the DOM (for example, through an external library) whenever the
 component was patched. Note that this hook will not be called if the compoent is
-not in the DOM (this can happen with components with `t-keepalive`).
+not in the DOM.
 
 Updating the component state in this hook is possible, but not encouraged.
 One needs to be careful, because updates here will create an additional rendering, which in

doc/reference/qweb.md

@@ -68,8 +68,8 @@ The component system in Owl requires additional directives, to express various
 needs. Here is a list of all Owl specific directives:
 
 | Name                     | Description                                                                         |
-| ------------------------------------------------------ | ----------------------------------------------------------------------------------- |
+| ------------------------ | ----------------------------------------------------------------------------------- |
-| `t-component`, `t-props`, `t-keepalive`, `t-asyncroot` | [Defining a sub component](component.md#composition)                                |
+| `t-component`, `t-props` | [Defining a sub component](component.md#composition)                                |
 | `t-ref`                  | [Setting a reference to a dom node or a sub component](component.md#references)     |
 | `t-key`                  | [Defining a key (to help virtual dom reconciliation)](component.md#t-key-directive) |
 | `t-on-*`                 | [Event handling](component.md#event-handling)                                       |

src/component/directive.ts

@@ -186,7 +186,7 @@ QWeb.utils.defineProxy = function defineProxy(target, source) {
 
 QWeb.addDirective({
   name: "component",
-  extraNames: ["props", "keepalive"],
+  extraNames: ["props"],
   priority: 100,
   atNodeEncounter({ ctx, value, node, qweb }): boolean {
     ctx.addLine("//COMPONENT");
@@ -194,7 +194,6 @@ QWeb.addDirective({
     ctx.rootContext.shouldDefineQWeb = true;
     ctx.rootContext.shouldDefineParent = true;
     ctx.rootContext.shouldDefineUtils = true;
-    let keepAlive = node.getAttribute("t-keepalive") ? true : false;
     let hasDynamicProps = node.getAttribute("t-props") ? true : false;
 
     // t-on- events and t-transition
@@ -256,8 +255,8 @@ QWeb.addDirective({
     if (transition) {
       transitionsInsertCode = `utils.transitionInsert(vn, '${transition}');`;
     }
-    let finalizeComponentCode = `w${componentID}.${keepAlive ? "unmount" : "destroy"}();`;
+    let finalizeComponentCode = `w${componentID}.destroy();`;
-    if (ref && !keepAlive) {
+    if (ref) {
       finalizeComponentCode += `delete context.__owl__.refs[${refKey}];`;
     }
     if (transition) {
@@ -338,11 +337,6 @@ QWeb.addDirective({
       `let w${componentID} = ${templateId} in parent.__owl__.cmap ? parent.__owl__.children[parent.__owl__.cmap[${templateId}]] : false;`
     );
     let shouldProxy = !ctx.parentNode;
-    if (keepAlive) {
-      ctx.addLine(
-        `const fiber${componentID} = Object.assign(Object.create(extra.fiber), {patchQueue: []});`
-      );
-    }
     if (shouldProxy) {
       let id = ctx.generateID();
       ctx.rootContext.rootNode = id;
@@ -393,28 +387,15 @@ QWeb.addDirective({
     ctx.addIf(`w${componentID}`);
 
     // need to update component
-    let patchQueueCode = keepAlive ? `fiber${componentID}` : "extra.fiber";
-    if (keepAlive) {
-      // if we have t-keepalive="1", the component could be unmounted, but then
-      // we __updateProps is called.  This is ok, but we do not want to call
-      // the willPatch/patched hooks of the component in this case, so we
-      // disable the patch queue
-      patchQueueCode = `w${componentID}.__owl__.isMounted ? extra.fiber : fiber${componentID}`;
-    }
     let styleCode = "";
     if (tattStyle) {
       styleCode = `.then(()=>{if (w${componentID}.__owl__.isDestroyed) {return};w${componentID}.el.style=${tattStyle};});`;
     }
     ctx.addLine(
-      `w${componentID}.__updateProps(props${componentID}, ${patchQueueCode}${scopeVars &&
+      `w${componentID}.__updateProps(props${componentID}, extra.fiber${scopeVars &&
         ", " + scopeVars}, sibling)${styleCode};`
     );
     ctx.addLine(`let pvnode = w${componentID}.__owl__.pvnode;`);
-    let keepAliveCode = "";
-    if (keepAlive) {
-      keepAliveCode = `pvnode.data.hook.insert = vn => {vn.elm.parentNode.replaceChild(w${componentID}.el,vn.elm);vn.elm=w${componentID}.el;w${componentID}.__remount();};`;
-      ctx.addLine(keepAliveCode);
-    }
     if (registerCode) {
       ctx.addLine(registerCode);
     }

src/store.ts

@@ -116,7 +116,6 @@ export function useStore(selector, options: SelectorOptions = {}): any {
     }
   });
   onWillUpdateProps(props => {
-    // FIXME: only do that if not keepalive + do it in destroy in that case
     delete store.updateFunctions[component.__owl__.id];
     result = selector(store.state, props);
   });

tests/component/__snapshots__/component.test.ts.snap

@@ -234,51 +234,6 @@ exports[`class and style attributes with t-component t-att-class is properly add
 }"
 `;
 
-exports[`composition sub components dom state with t-keepalive is preserved 1`] = `
-"function anonymous(context,extra
-) {
-    let utils = this.constructor.utils;
-    let QWeb = this.constructor;
-    let parent = context;
-    let owner = context;
-    let sibling = null;
-    var h = this.h;
-    let c1 = [], p1 = {key:1};
-    var vn1 = h('div', p1, c1);
-    if (context['state'].ok) {
-        //COMPONENT
-        let templateId3 = \`__4__\`;
-        let w3 = templateId3 in parent.__owl__.cmap ? parent.__owl__.children[parent.__owl__.cmap[templateId3]] : false;
-        const fiber3 = Object.assign(Object.create(extra.fiber), {patchQueue: []});
-        let props3 = {};
-        if (w3 && w3.__owl__.currentFiber && !w3.__owl__.vnode) {
-            w3.destroy();
-            w3 = false;
-        }
-        if (w3) {
-            w3.__updateProps(props3, w3.__owl__.isMounted ? extra.fiber : fiber3, undefined, undefined, sibling);
-            let pvnode = w3.__owl__.pvnode;
-            pvnode.data.hook.insert = vn => {vn.elm.parentNode.replaceChild(w3.el,vn.elm);vn.elm=w3.el;w3.__remount();};
-            c1.push(pvnode);
-        } else {
-            let componentKey3 = \`InputWidget\`;
-            let W3 = context.constructor.components[componentKey3] || QWeb.components[componentKey3]|| context['InputWidget'];
-            if (!W3) {throw new Error('Cannot find the definition of component \\"' + componentKey3 + '\\"')}
-            w3 = new W3(parent, props3);
-            parent.__owl__.cmap[templateId3] = w3.__owl__.id;
-            let def2 = w3.__prepare(extra.fiber, undefined, undefined, sibling);
-            let pvnode = h('dummy', {key: templateId3, hook: {insert(vn) { let nvn=w3.__mount(fiber, pvnode.elm);pvnode.elm=nvn.elm;},remove() {},destroy(vn) {w3.unmount();}}});
-            const fiber = w3.__owl__.currentFiber;
-            def2.then(function () {if (w3.__owl__.isDestroyed) {return;} const vnode = fiber.vnode; pvnode.sel = vnode.sel; });
-            c1.push(pvnode);
-            w3.__owl__.pvnode = pvnode;
-        }
-        sibling = w3.__owl__.currentFiber || sibling;
-    }
-    return vn1;
-}"
-`;
-
 exports[`composition sub components with some state rendered in a loop 1`] = `
 "function anonymous(context,extra
 ) {
@@ -460,51 +415,6 @@ exports[`dynamic t-props basic use 1`] = `
 }"
 `;
 
-exports[`lifecycle hooks willPatch/patched hook with t-keepalive 1`] = `
-"function anonymous(context,extra
-) {
-    let utils = this.constructor.utils;
-    let QWeb = this.constructor;
-    let parent = context;
-    let owner = context;
-    let sibling = null;
-    var h = this.h;
-    let c1 = [], p1 = {key:1};
-    var vn1 = h('div', p1, c1);
-    if (context['state'].flag) {
-        //COMPONENT
-        let templateId3 = \`__4__\`;
-        let w3 = templateId3 in parent.__owl__.cmap ? parent.__owl__.children[parent.__owl__.cmap[templateId3]] : false;
-        const fiber3 = Object.assign(Object.create(extra.fiber), {patchQueue: []});
-        let props3 = {v:context['state'].n};
-        if (w3 && w3.__owl__.currentFiber && !w3.__owl__.vnode) {
-            w3.destroy();
-            w3 = false;
-        }
-        if (w3) {
-            w3.__updateProps(props3, w3.__owl__.isMounted ? extra.fiber : fiber3, undefined, undefined, sibling);
-            let pvnode = w3.__owl__.pvnode;
-            pvnode.data.hook.insert = vn => {vn.elm.parentNode.replaceChild(w3.el,vn.elm);vn.elm=w3.el;w3.__remount();};
-            c1.push(pvnode);
-        } else {
-            let componentKey3 = \`ChildWidget\`;
-            let W3 = context.constructor.components[componentKey3] || QWeb.components[componentKey3]|| context['ChildWidget'];
-            if (!W3) {throw new Error('Cannot find the definition of component \\"' + componentKey3 + '\\"')}
-            w3 = new W3(parent, props3);
-            parent.__owl__.cmap[templateId3] = w3.__owl__.id;
-            let def2 = w3.__prepare(extra.fiber, undefined, undefined, sibling);
-            let pvnode = h('dummy', {key: templateId3, hook: {insert(vn) { let nvn=w3.__mount(fiber, pvnode.elm);pvnode.elm=nvn.elm;},remove() {},destroy(vn) {w3.unmount();}}});
-            const fiber = w3.__owl__.currentFiber;
-            def2.then(function () {if (w3.__owl__.isDestroyed) {return;} const vnode = fiber.vnode; pvnode.sel = vnode.sel; });
-            c1.push(pvnode);
-            w3.__owl__.pvnode = pvnode;
-        }
-        sibling = w3.__owl__.currentFiber || sibling;
-    }
-    return vn1;
-}"
-`;
-
 exports[`other directives with t-component t-on with getter as handler 1`] = `
 "function anonymous(context,extra
 ) {

tests/component/component.test.ts

@@ -836,48 +836,6 @@ describe("lifecycle hooks", () => {
       "parent:patched"
     ]);
   });
-
-  test("willPatch/patched hook with t-keepalive", async () => {
-    // we make sure here that willPatch/patched is only called if widget is in
-    // dom, mounted
-    const steps: string[] = [];
-
-    class ChildWidget extends Widget {
-      willPatch() {
-        steps.push("child:willPatch");
-      }
-      patched() {
-        steps.push("child:patched");
-      }
-      willUnmount() {
-        steps.push("child:willUnmount");
-      }
-      mounted() {
-        steps.push("child:mounted");
-      }
-    }
-    class ParentWidget extends Widget {
-      static template = xml`
-        <div>
-          <ChildWidget t-if="state.flag" v="state.n" t-keepalive="1"/>
-        </div>
-      `;
-      static components = { ChildWidget };
-      state = useState({ n: 1, flag: true });
-    }
-
-    const widget = new ParentWidget();
-    await widget.mount(fixture);
-
-    expect(env.qweb.templates[ParentWidget.template].fn.toString()).toMatchSnapshot();
-    expect(steps).toEqual(["child:mounted"]);
-    widget.state.flag = false;
-    await nextTick();
-    expect(steps).toEqual(["child:mounted", "child:willUnmount"]);
-    widget.state.flag = true;
-    await nextTick();
-    expect(steps).toEqual(["child:mounted", "child:willUnmount", "child:mounted"]);
-  });
 });
 
 describe("destroy method", () => {
@@ -1287,97 +1245,6 @@ describe("composition", () => {
     expect(fixture.innerHTML).toBe("<div><div>0<button>Inc</button></div></div>");
   });
 
-  test("sub components with t-keepalive are not destroyed if no longer in dom", async () => {
-    env.qweb.addTemplate(
-      "ParentWidget",
-      `<div><t t-if="state.ok"><Counter t-keepalive="1"/></t></div>`
-    );
-    class ParentWidget extends Widget {
-      state = useState({ ok: true });
-      static components = { Counter };
-    }
-    const widget = new ParentWidget();
-    await widget.mount(fixture);
-    const button = fixture.getElementsByTagName("button")[0];
-    await button.click();
-    await nextTick();
-    expect(fixture.innerHTML).toBe("<div><div>1<button>Inc</button></div></div>");
-    const counter = children(widget)[0];
-    expect(counter.__owl__.isMounted).toBe(true);
-
-    widget.state.ok = false;
-    await nextTick();
-    expect(fixture.innerHTML).toBe("<div></div>");
-    expect(counter.__owl__.isMounted).toBe(false);
-
-    widget.state.ok = true;
-    await nextTick();
-    expect(counter.__owl__.isMounted).toBe(true);
-    expect(fixture.innerHTML).toBe("<div><div>1<button>Inc</button></div></div>");
-  });
-
-  test("sub components dom state with t-keepalive is preserved", async () => {
-    env.qweb.addTemplate(
-      "ParentWidget",
-      `<div><t t-if="state.ok"><InputWidget t-keepalive="1"/></t></div>`
-    );
-    class InputWidget extends Widget {}
-    class ParentWidget extends Widget {
-      state = useState({ ok: true });
-      static components = { InputWidget };
-    }
-    env.qweb.addTemplate("InputWidget", "<input/>");
-    const widget = new ParentWidget();
-    await widget.mount(fixture);
-    const input = fixture.getElementsByTagName("input")[0];
-    input.value = "test";
-    widget.state.ok = false;
-    await nextTick();
-    expect(fixture.innerHTML).toBe("<div></div>");
-
-    widget.state.ok = true;
-    await nextTick();
-    expect(fixture.innerHTML).toBe("<div><input></div>");
-    const input2 = fixture.getElementsByTagName("input")[0];
-    expect(input).toBe(input2);
-    expect(input2.value).toBe("test");
-    expect(env.qweb.templates.ParentWidget.fn.toString()).toMatchSnapshot();
-  });
-
-  test("sub widget with t-ref and t-keepalive", async () => {
-    class ChildWidget extends Widget {
-      static template = xml`<span>Hello</span>`;
-    }
-    class ParentWidget extends Widget {
-      static template = xml`
-            <div>
-                <t t-if="state.ok"><ChildWidget t-ref="child" t-keepalive="1"/></t>
-            </div>
-        `;
-      static components = { ChildWidget };
-      state = useState({ ok: true });
-      child = useRef("child");
-    }
-    const widget = new ParentWidget();
-    await widget.mount(fixture);
-    let child = children(widget)[0];
-
-    expect(fixture.innerHTML).toBe("<div><span>Hello</span></div>");
-    expect(widget.child.comp).toEqual(child);
-
-    widget.state.ok = false;
-    await nextTick();
-
-    expect(fixture.innerHTML).toBe("<div></div>");
-    expect(widget.child.comp).toEqual(child);
-
-    widget.state.ok = true;
-    await nextTick();
-
-    expect(fixture.innerHTML).toBe("<div><span>Hello</span></div>");
-    expect(widget.child.comp).toEqual(child);
-  });
-
   test("sub components rendered in a loop", async () => {
     env.qweb.addTemplate("ChildWidget", `<span><t t-esc="props.n"/></span>`);
     class ChildWidget extends Widget {}

tools/playground/app.js

@@ -443,7 +443,6 @@ async function start() {
     owl.utils.whenReady()
   ]);
   const qweb = new owl.QWeb({ templates });
-  debugger;
   owl.config.env = { qweb };
   const app = new App();
   app.mount(document.body);

tools/playground/templates.xml

@@ -31,7 +31,7 @@
             t-att-style="topEditorStyle"/>
         <t t-if="state.splitLayout">
           <div class="separator horizontal"/>
-          <TabbedEditor t-keepalive="1"
+          <TabbedEditor
              js="false"
              css="state.css"
              xml="state.xml"