From 971b7988035c524d58a8b22748492495690eb4aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Fri, 8 Feb 2019 09:17:04 +0100 Subject: [PATCH] protect against widget destruction before being started --- web/static/src/ts/core/component.ts | 3 +- web/static/src/ts/core/qweb_vdom.ts | 4 +- web/static/tests/core/component.test.ts | 54 +++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/web/static/src/ts/core/component.ts b/web/static/src/ts/core/component.ts index c56d7bf1..f9c70804 100644 --- a/web/static/src/ts/core/component.ts +++ b/web/static/src/ts/core/component.ts @@ -1,3 +1,4 @@ +import h from "../../../libs/snabbdom/src/h"; import sdAttrs from "../../../libs/snabbdom/src/modules/attributes"; import sdListeners from "../../../libs/snabbdom/src/modules/eventlisteners"; import { init } from "../../../libs/snabbdom/src/snabbdom"; @@ -231,7 +232,7 @@ export class Component< this.__widget__.renderProps = this.props; this.__widget__.renderPromise = this.willStart().then(() => { if (this.__widget__.isDestroyed) { - return Promise.resolve(this.env.qweb.render("default")); + return Promise.resolve(h("div")); } this.__widget__.isStarted = true; if (this.inlineTemplate) { diff --git a/web/static/src/ts/core/qweb_vdom.ts b/web/static/src/ts/core/qweb_vdom.ts index d74b229c..45a1ae07 100644 --- a/web/static/src/ts/core/qweb_vdom.ts +++ b/web/static/src/ts/core/qweb_vdom.ts @@ -821,12 +821,12 @@ const widgetDirective: Directive = { ctx.dedent(); ctx.addLine(`} else {`); // not started ctx.indent(); + ctx.addLine(`isNew${widgetID} = true`); ctx.addLine( `if (props${widgetID} === w${widgetID}.__widget__.renderProps) {` ); ctx.indent(); ctx.addLine(`def${defID} = w${widgetID}.__widget__.renderPromise;`); - ctx.addLine(`isNew${widgetID} = true`); ctx.dedent(); ctx.addLine(`} else {`); ctx.indent(); @@ -880,7 +880,7 @@ const widgetDirective: Directive = { ctx.addLine(`} else {`); ctx.indent(); ctx.addLine( - `def${defID} = def${defID}.then(()=>{let vnode=h(w${widgetID}.__widget__.vnode.sel, {key: ${templateID}});vnode.elm=w${widgetID}.el;c${ + `def${defID} = def${defID}.then(()=>{if (!w${widgetID}.__widget__.vnode) {return};let vnode=h(w${widgetID}.__widget__.vnode.sel, {key: ${templateID}});vnode.elm=w${widgetID}.el;c${ ctx.parentNode }[_${dummyID}_index]=vnode;vnode.data.hook = {insert(a){a.elm.parentNode.replaceChild(w${widgetID}.el,a.elm);a.elm=w${widgetID}.el;w${widgetID}.__mount();},remove(){w${widgetID}.${ keepAlive ? "detach" : "destroy" diff --git a/web/static/tests/core/component.test.ts b/web/static/tests/core/component.test.ts index 51f6fc14..b764054c 100644 --- a/web/static/tests/core/component.test.ts +++ b/web/static/tests/core/component.test.ts @@ -796,6 +796,60 @@ describe("random stuff/miscellaneous", () => { }); describe("async rendering", () => { + test("destroying a widget before start is over", async () => { + let def = makeDeferred(); + class W extends Widget { + inlineTemplate = "invalid><"; + willStart(): Promise { + return def; + } + } + const w = new W(env); + w.mount(fixture); + expect(w.__widget__.isDestroyed).toBe(false); + expect(w.__widget__.isMounted).toBe(false); + expect(w.__widget__.isStarted).toBe(false); + w.destroy(); + def.resolve(); + await nextTick(); + expect(w.__widget__.isDestroyed).toBe(true); + expect(w.__widget__.isMounted).toBe(false); + expect(w.__widget__.isStarted).toBe(false); + }); + + test("destroying/recreating a subwidget with different props (if start is not over)", async () => { + let def = makeDeferred(); + let n = 0; + class W extends Widget { + inlineTemplate = `
`; + widgets = { Child }; + state = { val: 1 }; + } + + class Child extends Widget { + inlineTemplate = `child:`; + constructor(parent, props) { + super(parent, props); + n++; + } + willStart(): Promise { + return def; + } + } + const w = new W(env); + await w.mount(fixture); + expect(n).toBe(0); + w.updateState({ val: 2 }); + expect(n).toBe(1); + await nextTick(); + w.updateState({ val: 3 }); + expect(n).toBe(2); + def.resolve(); + await nextTick(); + expect(children(w).length).toBe(1); + expect(fixture.innerHTML).toBe("
child:3
"); + }); + test("creating two async widgets, scenario 1", async () => { let defA = makeDeferred(); let defB = makeDeferred();