From 69047135a1a9ace158e9aa090c62ced59c7af478 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Sat, 9 Feb 2019 13:03:44 +0100 Subject: [PATCH] small qweb optimisation: reuse bound handlers if possible --- web/static/src/ts/core/component.ts | 9 +++- web/static/src/ts/core/qweb_vdom.ts | 5 +- .../core/__snapshots__/qweb_vdom.test.ts.snap | 12 +++-- web/static/tests/core/qweb_vdom.test.ts | 52 ++++++++++++------- 4 files changed, 52 insertions(+), 26 deletions(-) diff --git a/web/static/src/ts/core/component.ts b/web/static/src/ts/core/component.ts index f9c70804..1f54dc94 100644 --- a/web/static/src/ts/core/component.ts +++ b/web/static/src/ts/core/component.ts @@ -33,6 +33,7 @@ interface Meta { renderId: number; renderProps: any; renderPromise: Promise | null; + boundHandlers: { [key: number]: any }; } const patch = init([sdListeners, sdAttrs]); @@ -100,7 +101,8 @@ export class Component< cmap: {}, renderId: 1, renderPromise: null, - renderProps: props + renderProps: props, + boundHandlers: {} }; } @@ -251,7 +253,10 @@ export class Component< this.__widget__.renderId++; const promises: Promise[] = []; const template = this.inlineTemplate || this.template; - let vnode = this.env.qweb.render(template, this, { promises }); + let vnode = this.env.qweb.render(template, this, { + promises, + handlers: this.__widget__.boundHandlers + }); // this part is critical for the patching process to be done correctly. The // tricky part is that a child widget can be rerendered on its own, which diff --git a/web/static/src/ts/core/qweb_vdom.ts b/web/static/src/ts/core/qweb_vdom.ts index 45a1ae07..3a8061e9 100644 --- a/web/static/src/ts/core/qweb_vdom.ts +++ b/web/static/src/ts/core/qweb_vdom.ts @@ -722,10 +722,11 @@ const onDirective: Directive = { return ""; }); ctx.addLine( - `p${nodeID}.on = {${eventName}: context['${handler}'].bind(owner${ + `extra.handlers[${nodeID}] = extra.handlers[${nodeID}] || context['${handler}'].bind(owner${ extraArgs ? ", " + qweb._formatExpression(extraArgs) : "" - })};` + });` ); + ctx.addLine(`p${nodeID}.on = {${eventName}: extra.handlers[${nodeID}]};`); } }; diff --git a/web/static/tests/core/__snapshots__/qweb_vdom.test.ts.snap b/web/static/tests/core/__snapshots__/qweb_vdom.test.ts.snap index d8140294..0d307c56 100644 --- a/web/static/tests/core/__snapshots__/qweb_vdom.test.ts.snap +++ b/web/static/tests/core/__snapshots__/qweb_vdom.test.ts.snap @@ -890,7 +890,8 @@ exports[`t-on can bind event handler 1`] = ` let h = this.h; let c1 = [], p1 = {}; let vn1 = h('button', p1, c1); - p1.on = {click: context['add'].bind(owner)}; + extra.handlers[1] = extra.handlers[1] || context['add'].bind(owner); + p1.on = {click: extra.handlers[1]}; c1.push({text: \`Click\`}); return vn1; }" @@ -903,7 +904,8 @@ exports[`t-on can bind handlers with arguments 1`] = ` let h = this.h; let c1 = [], p1 = {}; let vn1 = h('button', p1, c1); - p1.on = {click: context['add'].bind(owner, 5)}; + extra.handlers[1] = extra.handlers[1] || context['add'].bind(owner, 5); + p1.on = {click: extra.handlers[1]}; c1.push({text: \`Click\`}); return vn1; }" @@ -937,7 +939,8 @@ exports[`t-on can bind handlers with loop variable as argument 1`] = ` let c6 = [], p6 = {}; let vn6 = h('a', p6, c6); c5.push(vn6); - p6.on = {click: context['activate'].bind(owner, context['action'])}; + extra.handlers[6] = extra.handlers[6] || context['activate'].bind(owner, context['action']); + p6.on = {click: extra.handlers[6]}; c6.push({text: \`link\`}); } c1.push({text: \` @@ -953,7 +956,8 @@ exports[`t-on handler is bound to proper owner 1`] = ` let h = this.h; let c1 = [], p1 = {}; let vn1 = h('button', p1, c1); - p1.on = {click: context['add'].bind(owner)}; + extra.handlers[1] = extra.handlers[1] || context['add'].bind(owner); + p1.on = {click: extra.handlers[1]}; c1.push({text: \`Click\`}); return vn1; }" diff --git a/web/static/tests/core/qweb_vdom.test.ts b/web/static/tests/core/qweb_vdom.test.ts index 02aa23c8..7853c8de 100644 --- a/web/static/tests/core/qweb_vdom.test.ts +++ b/web/static/tests/core/qweb_vdom.test.ts @@ -25,9 +25,10 @@ function trim(str: string): string { function renderToDOM( qweb: QWeb, template: string, - context: EvalContext = {} + context: EvalContext = {}, + extra?: any ): HTMLElement | Text { - const vnode = qweb.render(template, context); + const vnode = qweb.render(template, context, extra); // we snapshot here the compiled code. This is useful to prevent unwanted code // change. @@ -658,11 +659,16 @@ describe("t-on", () => { test("can bind event handler", () => { qweb.addTemplate("test", ``); let a = 1; - const node = renderToDOM(qweb, "test", { - add() { - a = 3; - } - }); + const node = renderToDOM( + qweb, + "test", + { + add() { + a = 3; + } + }, + { handlers: [] } + ); (node).click(); expect(a).toBe(3); }); @@ -670,11 +676,16 @@ describe("t-on", () => { test("can bind handlers with arguments", () => { qweb.addTemplate("test", ``); let a = 1; - const node = renderToDOM(qweb, "test", { - add(n) { - a = a + n; - } - }); + const node = renderToDOM( + qweb, + "test", + { + add(n) { + a = a + n; + } + }, + { handlers: [] } + ); (node).click(); expect(a).toBe(6); }); @@ -688,11 +699,16 @@ describe("t-on", () => {
  • link
    • ` ); - const node = renderToDOM(qweb, "test", { - activate(action) { - expect(action).toBe("someval"); - } - }); + const node = renderToDOM( + qweb, + "test", + { + activate(action) { + expect(action).toBe("someval"); + } + }, + { handlers: [] } + ); (node).getElementsByTagName("a")[0].click(); }); @@ -704,7 +720,7 @@ describe("t-on", () => { expect(this).toBe(owner); } }; - const node = renderToDOM(qweb, "test", owner); + const node = renderToDOM(qweb, "test", owner, { handlers: [] }); (node).click(); }); });