fix issue with vnodes being patched twice

This commit is contained in:
Géry Debongnie
2019-01-30 14:56:47 +01:00
parent d832fd395a
commit 6012d76388
4 changed files with 49 additions and 15 deletions
+3 -3
View File
@@ -55,7 +55,6 @@ export class Widget<T extends WEnv, Props> {
constructor(parent: Widget<T, {}> | T, props?: Props) { constructor(parent: Widget<T, {}> | T, props?: Props) {
wl.push(this); wl.push(this);
// is this a good idea? // is this a good idea?
// Pro: if props is empty, we can create easily a widget // Pro: if props is empty, we can create easily a widget
// Con: this is not really safe // Con: this is not really safe
@@ -211,14 +210,15 @@ export class Widget<T extends WEnv, Props> {
/** /**
* Only called by qweb t-widget directive * Only called by qweb t-widget directive
*/ */
_mount(vnode: VNode) { _mount(vnode: VNode, elm: HTMLElement): VNode {
this.__widget__.vnode = vnode; this.__widget__.vnode = patch(elm, vnode);
if (this.__widget__.parent) { if (this.__widget__.parent) {
if (this.__widget__.parent.__widget__.isMounted) { if (this.__widget__.parent.__widget__.isMounted) {
this.__widget__.isMounted = true; this.__widget__.isMounted = true;
this.mounted(); this.mounted();
} }
} }
return this.__widget__.vnode;
} }
private visitSubTree(callback: (w: Widget<T, any>) => void) { private visitSubTree(callback: (w: Widget<T, any>) => void) {
+7 -9
View File
@@ -210,14 +210,14 @@ export class QWeb {
const mainNode = doc.firstChild!; const mainNode = doc.firstChild!;
this._compileNode(mainNode, ctx); this._compileNode(mainNode, ctx);
if (ctx.shouldProtectContext) {
ctx.code.unshift(" context = Object.create(context);");
}
if (ctx.shouldDefineOwner) { if (ctx.shouldDefineOwner) {
// this is necessary to prevent some directives (t-forach for ex) to // this is necessary to prevent some directives (t-forach for ex) to
// pollute the rendering context by adding some keys in it. // pollute the rendering context by adding some keys in it.
ctx.code.unshift(" let owner = context;"); ctx.code.unshift(" let owner = context;");
} }
if (ctx.shouldProtectContext) {
ctx.code.unshift(" context = Object.create(context);");
}
if (!ctx.rootNode) { if (!ctx.rootNode) {
throw new Error("A template should have one root node"); throw new Error("A template should have one root node");
@@ -728,7 +728,6 @@ const widgetDirective: Directive = {
let dummyID = ctx.generateID(); let dummyID = ctx.generateID();
let defID = ctx.generateID(); let defID = ctx.generateID();
let widgetID = ctx.generateID(); let widgetID = ctx.generateID();
ctx.addLine(`let _${dummyID} = {}; // DUMMY`);
let keyID = key && ctx.generateID(); let keyID = key && ctx.generateID();
if (key) { if (key) {
// we bind a variable to the key (could be a complex expression, so we // we bind a variable to the key (could be a complex expression, so we
@@ -736,7 +735,7 @@ const widgetDirective: Directive = {
ctx.addLine(`let key${keyID} = ${key};`); ctx.addLine(`let key${keyID} = ${key};`);
} }
ctx.addLine(`let _${dummyID}_index = c${ctx.parentNode}.length;`); ctx.addLine(`let _${dummyID}_index = c${ctx.parentNode}.length;`);
ctx.addLine(`c${ctx.parentNode}.push(_${dummyID});`); ctx.addLine(`c${ctx.parentNode}.push(null);`);
ctx.addLine(`let def${defID};`); ctx.addLine(`let def${defID};`);
let templateID = key let templateID = key
? `key${keyID}` ? `key${keyID}`
@@ -746,11 +745,10 @@ const widgetDirective: Directive = {
ctx.addLine( ctx.addLine(
`let w${widgetID} = ${templateID} in context.__widget__.cmap ? context.__widget__.children[context.__widget__.cmap[${templateID}]] : false;` `let w${widgetID} = ${templateID} in context.__widget__.cmap ? context.__widget__.children[context.__widget__.cmap[${templateID}]] : false;`
); );
ctx.addLine(`if (w${widgetID}) {`); ctx.addLine(`if (w${widgetID}) {`);
ctx.indent(); ctx.indent();
ctx.addLine( ctx.addLine(
`def${defID} = w${widgetID}.updateProps(${props}).then(()=>{vnode=w${widgetID}.__widget__.vnode;c${ `def${defID} = w${widgetID}.updateProps(${props}).then(()=>{let vnode=h(w${widgetID}.__widget__.vnode.sel, {key: ${templateID}});c${
ctx.parentNode ctx.parentNode
}[_${dummyID}_index]=vnode;vnode.data.hook = {remove(){w${widgetID}.destroy()}}});` }[_${dummyID}_index]=vnode;vnode.data.hook = {remove(){w${widgetID}.destroy()}}});`
); );
@@ -765,9 +763,9 @@ const widgetDirective: Directive = {
`context.__widget__.cmap[${templateID}] = _${widgetID}.__widget__.id;` `context.__widget__.cmap[${templateID}] = _${widgetID}.__widget__.id;`
); );
ctx.addLine( ctx.addLine(
`def${defID} = _${widgetID}._start().then(() => _${widgetID}._render()).then(vnode=>{c${ `def${defID} = _${widgetID}._start().then(() => _${widgetID}._render()).then(vnode=>{let pvnode=h(vnode.sel, {key: ${templateID}});c${
ctx.parentNode ctx.parentNode
}[_${dummyID}_index]=vnode;vnode.data.hook = {create(_,vn){_${widgetID}._mount(vn)},remove(){_${widgetID}.destroy()}}});` }[_${dummyID}_index]=pvnode;pvnode.data.hook = {insert(vn){let nvn=_${widgetID}._mount(vnode, vn.elm);pvnode.elm=nvn.elm},remove(){_${widgetID}.destroy()}}});`
); );
let ref = node.getAttribute("t-ref"); let ref = node.getAttribute("t-ref");
+15 -3
View File
@@ -22,16 +22,28 @@ const template = `
<t t-widget="ColorWidget" t-props="{color: state.color}"/> <t t-widget="ColorWidget" t-props="{color: state.color}"/>
<button t-on-click="addNotif(false)">Add notif</button> <button t-on-click="addNotif(false)">Add notif</button>
<button t-on-click="addNotif(true)">Add sticky notif</button> <button t-on-click="addNotif(true)">Add sticky notif</button>
<t t-foreach="state.test" t-as="blip"> <t t-foreach="state.test" t-as="number">
<t t-widget="Counter" t-key="blip" t-props="{initialState: 100}"/> <t t-widget="ChildWidget" t-key="number"/>
</t> </t>
</div> </div>
`; `;
let n = 1;
class ChildWidget extends Widget<Env, never> {
name = "c";
template = `<span><t t-esc="state.n"/></span>`;
constructor(parent) {
super(parent);
this.state = { n };
n++;
}
}
export class Discuss extends Widget<Env, {}> { export class Discuss extends Widget<Env, {}> {
name = "discuss"; name = "discuss";
template = template; template = template;
widgets = { Clock, Counter, ColorWidget }; widgets = { Clock, Counter, ColorWidget, ChildWidget };
state = { validcounter: true, color: "red", test: [1, 2, 3] }; state = { validcounter: true, color: "red", test: [1, 2, 3] };
mounted() {} mounted() {}
+24
View File
@@ -714,3 +714,27 @@ describe("random stuff", () => {
expect(fixture.innerHTML).toBe("<div>txttxt<div></div></div>"); expect(fixture.innerHTML).toBe("<div>txttxt<div></div></div>");
}); });
}); });
describe("miscellaneous", () => {
test("updating widget immediately", async () => {
// in this situation, we protect against a bug that occurred: because of the
// interplay between widgets and vnodes, a sub widget vnode was patched
// twice.
class Parent extends Widget<WEnv, {}> {
name = "a";
template = `<div><t t-widget="child" t-props="state"/></div>`;
widgets = { child: Child };
state = { flag: false };
}
class Child extends Widget<WEnv, {}> {
template = `<span>abc<t t-if="props.flag">def</t></span>`;
}
const widget = new Parent(env);
await widget.mount(fixture);
expect(fixture.innerHTML).toBe("<div><span>abc</span></div>");
await widget.updateState({ flag: true });
expect(fixture.innerHTML).toBe("<div><span>abcdef</span></div>");
});
});