[FIX] component: reuse widget if possible

Previous code always destroy and recreate widgets.
This commit is contained in:
Géry Debongnie
2019-06-01 21:44:43 +02:00
committed by VincentSchippefilt
parent 8dc3ec94bf
commit 466c12a0e6
5 changed files with 155 additions and 51 deletions
+9
View File
@@ -99,6 +99,7 @@ const NODE_HOOKS_PARAMS = {
interface Utils {
h: typeof h;
objectToAttrString(obj: Object): string;
shallowEqual(p1: Object, p2: Object): boolean;
[key: string]: any;
}
@@ -112,6 +113,14 @@ export const UTILS: Utils = {
}
}
return classes.join(" ");
},
shallowEqual(p1, p2) {
for (let k in p1) {
if (p1[k] !== p2[k]) {
return false;
}
}
return true;
}
};
+29 -15
View File
@@ -199,6 +199,10 @@ QWeb.addDirective({
* ```
*
* ```js
* // we assign utils on top of the function because it will be useful for
* // each widgets
* let utils = this.utils;
*
* // this is the virtual node representing the parent div
* let c1 = [], p1 = { key: 1 };
* var vn1 = h("div", p1, c1);
@@ -232,17 +236,19 @@ QWeb.addDirective({
* // computation, so it is certainly better to do it only once
* let props4 = { flag: context["state"].flag };
*
* // If we have a widget, currently rendering, but not ready yet, and which was
* // rendered with different props, we do not want to wait for it to be ready,
* // then update it. We simply destroy it, and start anew.
* if (
* w4 &&
* w4.__owl__.renderPromise &&
* !w4.__owl__.isStarted &&
* props4 !== w4.__owl__.renderProps
* ) {
* w4.destroy();
* w4 = false;
* // If we have a widget, currently rendering, but not ready yet, we do not want
* // to wait for it to be ready if we can avoid it
* if (w4 && w4.__owl__.renderPromise && !w4.__owl__.vnode) {
* // we check if the props are the same. In that case, we can simply reuse
* // the previous rendering and skip all useless work
* if (utils.shallowEqual(props4, w4.__owl__.renderProps)) {
* def3 = w4.__owl__.renderPromise;
* } else {
* // if the props are not the same, we destroy the widget and starts anew.
* // this will be faster than waiting for its rendering, then updating it
* w4.destroy();
* w4 = false;
* }
* }
*
* if (!w4) {
@@ -312,7 +318,9 @@ QWeb.addDirective({
* } else {
* // this is the 'update' path of the directive.
* // the call to _updateProps is the actual widget update
* def3 = w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
* // Note that we only update the props if we cannot reuse the previous
* // rendering work (in the case it was rendered with the same props)
* def3 = def3 || w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
* def3 = def3.then(() => {
* // if widget was destroyed in the meantime, we do nothing (so, this
* // means that the parent's element children list will have a null in
@@ -450,10 +458,16 @@ QWeb.addDirective({
);
ctx.addLine(`let props${widgetID} = ${props || "{}"};`);
ctx.addIf(
`w${widgetID} && w${widgetID}.__owl__.renderPromise && !w${widgetID}.__owl__.vnode && props${widgetID} !== w${widgetID}.__owl__.renderProps`
`w${widgetID} && w${widgetID}.__owl__.renderPromise && !w${widgetID}.__owl__.vnode`
);
ctx.addIf(
`utils.shallowEqual(props${widgetID}, w${widgetID}.__owl__.renderProps)`
);
ctx.addLine(`def${defID} = w${widgetID}.__owl__.renderPromise;`);
ctx.addElse();
ctx.addLine(`w${widgetID}.destroy();`);
ctx.addLine(`w${widgetID} = false`);
ctx.addLine(`w${widgetID} = false;`);
ctx.closeIf();
ctx.closeIf();
ctx.addIf(`!w${widgetID}`);
@@ -486,7 +500,7 @@ QWeb.addDirective({
ctx.addElse();
// need to update widget
ctx.addLine(
`def${defID} = w${widgetID}._updateProps(props${widgetID}, extra.forceUpdate, extra.patchQueue);`
`def${defID} = def${defID} || w${widgetID}._updateProps(props${widgetID}, extra.forceUpdate, extra.patchQueue);`
);
let keepAliveCode = "";
if (keepAlive) {
+16 -8
View File
@@ -15,9 +15,13 @@ exports[`animations t-transition combined with t-widget 1`] = `
let def3;
let w4 = 4 in context.__owl__.cmap ? context.__owl__.children[context.__owl__.cmap[4]] : false;
let props4 = {};
if (w4 && w4.__owl__.renderPromise && !w4.__owl__.vnode && props4 !== w4.__owl__.renderProps) {
w4.destroy();
w4 = false
if (w4 && w4.__owl__.renderPromise && !w4.__owl__.vnode) {
if (utils.shallowEqual(props4, w4.__owl__.renderProps)) {
def3 = w4.__owl__.renderPromise;
} else {
w4.destroy();
w4 = false;
}
}
if (!w4) {
let widgetKey4 = \`Child\`;
@@ -31,7 +35,7 @@ exports[`animations t-transition combined with t-widget 1`] = `
};
utils.transitionRemove(vn.elm, 'chimay', finalize);}}});c1[_2_index]=pvnode;w4.__owl__.pvnode = pvnode;});
} else {
def3 = w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
def3 = def3 || w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
def3 = def3.then(()=>{if (w4.__owl__.isDestroyed) {return};let pvnode=w4.__owl__.pvnode;c1[_2_index]=pvnode;});
}
extra.promises.push(def3);
@@ -55,9 +59,13 @@ exports[`animations t-transition combined with t-widget and t-if 1`] = `
let def3;
let w4 = 4 in context.__owl__.cmap ? context.__owl__.children[context.__owl__.cmap[4]] : false;
let props4 = {};
if (w4 && w4.__owl__.renderPromise && !w4.__owl__.vnode && props4 !== w4.__owl__.renderProps) {
w4.destroy();
w4 = false
if (w4 && w4.__owl__.renderPromise && !w4.__owl__.vnode) {
if (utils.shallowEqual(props4, w4.__owl__.renderProps)) {
def3 = w4.__owl__.renderPromise;
} else {
w4.destroy();
w4 = false;
}
}
if (!w4) {
let widgetKey4 = \`Child\`;
@@ -71,7 +79,7 @@ exports[`animations t-transition combined with t-widget and t-if 1`] = `
};
utils.transitionRemove(vn.elm, 'chimay', finalize);}}});c1[_2_index]=pvnode;w4.__owl__.pvnode = pvnode;});
} else {
def3 = w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
def3 = def3 || w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
def3 = def3.then(()=>{if (w4.__owl__.isDestroyed) {return};let pvnode=w4.__owl__.pvnode;c1[_2_index]=pvnode;});
}
extra.promises.push(def3);
+56 -28
View File
@@ -16,9 +16,13 @@ exports[`class and style attributes with t-widget dynamic t-att-style is properl
const _5 = context['state'].style;
let w4 = 4 in context.__owl__.cmap ? context.__owl__.children[context.__owl__.cmap[4]] : false;
let props4 = {};
if (w4 && w4.__owl__.renderPromise && !w4.__owl__.vnode && props4 !== w4.__owl__.renderProps) {
w4.destroy();
w4 = false
if (w4 && w4.__owl__.renderPromise && !w4.__owl__.vnode) {
if (utils.shallowEqual(props4, w4.__owl__.renderProps)) {
def3 = w4.__owl__.renderPromise;
} else {
w4.destroy();
w4 = false;
}
}
if (!w4) {
let widgetKey4 = \`child\`;
@@ -29,7 +33,7 @@ exports[`class and style attributes with t-widget dynamic t-att-style is properl
def3 = w4._prepare();
def3 = def3.then(vnode=>{vnode.data.hook = {create(_, vn){vn.elm.style = _5}};let pvnode=h(vnode.sel, {key: 4, hook: {insert(vn) {let nvn=w4._mount(vnode, pvnode.elm);pvnode.elm=nvn.elm;},remove() {},destroy(vn) {w4.destroy();}}});c1[_2_index]=pvnode;w4.__owl__.pvnode = pvnode;});
} else {
def3 = w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
def3 = def3 || w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
def3 = def3.then(()=>{if (w4.__owl__.isDestroyed) {return};w4.el.style=_5;let pvnode=w4.__owl__.pvnode;c1[_2_index]=pvnode;});
}
extra.promises.push(def3);
@@ -53,9 +57,13 @@ exports[`class and style attributes with t-widget t-att-class is properly added/
const _5 = {a: context['state'].a,b: context['state'].b};
let w4 = 4 in context.__owl__.cmap ? context.__owl__.children[context.__owl__.cmap[4]] : false;
let props4 = {};
if (w4 && w4.__owl__.renderPromise && !w4.__owl__.vnode && props4 !== w4.__owl__.renderProps) {
w4.destroy();
w4 = false
if (w4 && w4.__owl__.renderPromise && !w4.__owl__.vnode) {
if (utils.shallowEqual(props4, w4.__owl__.renderProps)) {
def3 = w4.__owl__.renderPromise;
} else {
w4.destroy();
w4 = false;
}
}
if (!w4) {
let widgetKey4 = \`child\`;
@@ -70,7 +78,7 @@ exports[`class and style attributes with t-widget t-att-class is properly added/
}
}}};let pvnode=h(vnode.sel, {key: 4, hook: {insert(vn) {let nvn=w4._mount(vnode, pvnode.elm);pvnode.elm=nvn.elm;},remove() {},destroy(vn) {w4.destroy();}}});c1[_2_index]=pvnode;w4.__owl__.pvnode = pvnode;});
} else {
def3 = w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
def3 = def3 || w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
def3 = def3.then(()=>{if (w4.__owl__.isDestroyed) {return};let cl=w4.el.classList;for (let k in _5) {if (_5[k]) {cl.add(k)} else {cl.remove(k)}}let pvnode=w4.__owl__.pvnode;c1[_2_index]=pvnode;});
}
extra.promises.push(def3);
@@ -108,9 +116,13 @@ exports[`composition sub widgets with some state rendered in a loop 1`] = `
let def6;
let w7 = key8 in context.__owl__.cmap ? context.__owl__.children[context.__owl__.cmap[key8]] : false;
let props7 = {};
if (w7 && w7.__owl__.renderPromise && !w7.__owl__.vnode && props7 !== w7.__owl__.renderProps) {
w7.destroy();
w7 = false
if (w7 && w7.__owl__.renderPromise && !w7.__owl__.vnode) {
if (utils.shallowEqual(props7, w7.__owl__.renderProps)) {
def6 = w7.__owl__.renderPromise;
} else {
w7.destroy();
w7 = false;
}
}
if (!w7) {
let widgetKey7 = \`ChildWidget\`;
@@ -121,7 +133,7 @@ exports[`composition sub widgets with some state rendered in a loop 1`] = `
def6 = w7._prepare();
def6 = def6.then(vnode=>{let pvnode=h(vnode.sel, {key: key8, hook: {insert(vn) {let nvn=w7._mount(vnode, pvnode.elm);pvnode.elm=nvn.elm;},remove() {},destroy(vn) {w7.destroy();}}});c1[_5_index]=pvnode;w7.__owl__.pvnode = pvnode;});
} else {
def6 = w7._updateProps(props7, extra.forceUpdate, extra.patchQueue);
def6 = def6 || w7._updateProps(props7, extra.forceUpdate, extra.patchQueue);
def6 = def6.then(()=>{if (w7.__owl__.isDestroyed) {return};let pvnode=w7.__owl__.pvnode;c1[_5_index]=pvnode;});
}
extra.promises.push(def6);
@@ -145,9 +157,13 @@ exports[`composition t-widget with dynamic value 1`] = `
let def3;
let w4 = 4 in context.__owl__.cmap ? context.__owl__.children[context.__owl__.cmap[4]] : false;
let props4 = {};
if (w4 && w4.__owl__.renderPromise && !w4.__owl__.vnode && props4 !== w4.__owl__.renderProps) {
w4.destroy();
w4 = false
if (w4 && w4.__owl__.renderPromise && !w4.__owl__.vnode) {
if (utils.shallowEqual(props4, w4.__owl__.renderProps)) {
def3 = w4.__owl__.renderPromise;
} else {
w4.destroy();
w4 = false;
}
}
if (!w4) {
let widgetKey4 = (context['state'].widget);
@@ -158,7 +174,7 @@ exports[`composition t-widget with dynamic value 1`] = `
def3 = w4._prepare();
def3 = def3.then(vnode=>{let pvnode=h(vnode.sel, {key: 4, hook: {insert(vn) {let nvn=w4._mount(vnode, pvnode.elm);pvnode.elm=nvn.elm;},remove() {},destroy(vn) {w4.destroy();}}});c1[_2_index]=pvnode;w4.__owl__.pvnode = pvnode;});
} else {
def3 = w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
def3 = def3 || w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
def3 = def3.then(()=>{if (w4.__owl__.isDestroyed) {return};let pvnode=w4.__owl__.pvnode;c1[_2_index]=pvnode;});
}
extra.promises.push(def3);
@@ -181,9 +197,13 @@ exports[`composition t-widget with dynamic value 2 1`] = `
let def3;
let w4 = 4 in context.__owl__.cmap ? context.__owl__.children[context.__owl__.cmap[4]] : false;
let props4 = {};
if (w4 && w4.__owl__.renderPromise && !w4.__owl__.vnode && props4 !== w4.__owl__.renderProps) {
w4.destroy();
w4 = false
if (w4 && w4.__owl__.renderPromise && !w4.__owl__.vnode) {
if (utils.shallowEqual(props4, w4.__owl__.renderProps)) {
def3 = w4.__owl__.renderPromise;
} else {
w4.destroy();
w4 = false;
}
}
if (!w4) {
let widgetKey4 = \`Widget\${context['state'].widget}\`;
@@ -194,7 +214,7 @@ exports[`composition t-widget with dynamic value 2 1`] = `
def3 = w4._prepare();
def3 = def3.then(vnode=>{let pvnode=h(vnode.sel, {key: 4, hook: {insert(vn) {let nvn=w4._mount(vnode, pvnode.elm);pvnode.elm=nvn.elm;},remove() {},destroy(vn) {w4.destroy();}}});c1[_2_index]=pvnode;w4.__owl__.pvnode = pvnode;});
} else {
def3 = w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
def3 = def3 || w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
def3 = def3.then(()=>{if (w4.__owl__.isDestroyed) {return};let pvnode=w4.__owl__.pvnode;c1[_2_index]=pvnode;});
}
extra.promises.push(def3);
@@ -218,9 +238,13 @@ exports[`random stuff/miscellaneous snapshotting compiled code 1`] = `
let def3;
let w4 = key5 in context.__owl__.cmap ? context.__owl__.children[context.__owl__.cmap[key5]] : false;
let props4 = {flag: context['state'].flag};
if (w4 && w4.__owl__.renderPromise && !w4.__owl__.vnode && props4 !== w4.__owl__.renderProps) {
w4.destroy();
w4 = false
if (w4 && w4.__owl__.renderPromise && !w4.__owl__.vnode) {
if (utils.shallowEqual(props4, w4.__owl__.renderProps)) {
def3 = w4.__owl__.renderPromise;
} else {
w4.destroy();
w4 = false;
}
}
if (!w4) {
let widgetKey4 = \`child\`;
@@ -231,7 +255,7 @@ exports[`random stuff/miscellaneous snapshotting compiled code 1`] = `
def3 = w4._prepare();
def3 = def3.then(vnode=>{let pvnode=h(vnode.sel, {key: key5, hook: {insert(vn) {let nvn=w4._mount(vnode, pvnode.elm);pvnode.elm=nvn.elm;},remove() {},destroy(vn) {w4.destroy();}}});c1[_2_index]=pvnode;w4.__owl__.pvnode = pvnode;});
} else {
def3 = w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
def3 = def3 || w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
def3 = def3.then(()=>{if (w4.__owl__.isDestroyed) {return};let pvnode=w4.__owl__.pvnode;c1[_2_index]=pvnode;});
}
extra.promises.push(def3);
@@ -254,9 +278,13 @@ exports[`random stuff/miscellaneous t-props should not be undefined (snapshottin
let def3;
let w4 = 4 in context.__owl__.cmap ? context.__owl__.children[context.__owl__.cmap[4]] : false;
let props4 = {};
if (w4 && w4.__owl__.renderPromise && !w4.__owl__.vnode && props4 !== w4.__owl__.renderProps) {
w4.destroy();
w4 = false
if (w4 && w4.__owl__.renderPromise && !w4.__owl__.vnode) {
if (utils.shallowEqual(props4, w4.__owl__.renderProps)) {
def3 = w4.__owl__.renderPromise;
} else {
w4.destroy();
w4 = false;
}
}
if (!w4) {
let widgetKey4 = \`child\`;
@@ -267,7 +295,7 @@ exports[`random stuff/miscellaneous t-props should not be undefined (snapshottin
def3 = w4._prepare();
def3 = def3.then(vnode=>{let pvnode=h(vnode.sel, {key: 4, hook: {insert(vn) {let nvn=w4._mount(vnode, pvnode.elm);pvnode.elm=nvn.elm;},remove() {},destroy(vn) {w4.destroy();}}});c1[_2_index]=pvnode;w4.__owl__.pvnode = pvnode;});
} else {
def3 = w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
def3 = def3 || w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
def3 = def3.then(()=>{if (w4.__owl__.isDestroyed) {return};let pvnode=w4.__owl__.pvnode;c1[_2_index]=pvnode;});
}
extra.promises.push(def3);
+45
View File
@@ -2084,6 +2084,51 @@ describe("async rendering", () => {
await nextTick();
expect(fixture.innerHTML).toBe("<div></div>");
});
test("reuse widget if possible, in some async situation", async () => {
env.qweb.addTemplates(`
<templates>
<span t-name="ChildA">a<t t-esc="props.val"/></span>
<span t-name="ChildB">b<t t-esc="props.val"/></span>
<span t-name="Parent">
<t t-if="state.flag">
<t t-widget="ChildA" t-props="{val:state.valA}"/>
<t t-widget="ChildB" t-props="{val:state.valB}"/>
</t>
</span>
</templates>
`);
let destroyCount = 0;
class ChildA extends Widget {
destroy() {
destroyCount++;
super.destroy();
}
}
class ChildB extends Widget {
willStart(): any {
return new Promise(function () {});
}
}
class Parent extends Widget {
widgets = { ChildA, ChildB };
state = { valA: 1, valB: 2, flag: false };
}
const parent = new Parent(env);
await parent.mount(fixture);
expect(destroyCount).toBe(0);
parent.state.flag = true;
await nextTick();
expect(destroyCount).toBe(0);
parent.state.valB = 3;
await nextTick();
expect(destroyCount).toBe(0);
});
});
describe("updating environment", () => {