[FIX] component: scope issue with slots

Slot templates need to be able to access variables from the parent
scope.

closes #228

src/component.ts

@@ -274,7 +274,7 @@ export class Component<T extends Env, Props extends {}, State extends {}> {
     }
   }
 
-  async render(force: boolean = false, patchQueue?: any[]): Promise<void> {
+  async render(force: boolean = false, patchQueue?: any[], scope?: any, vars?: any): Promise<void> {
     const __owl__ = this.__owl__;
     if (!__owl__.isMounted) {
       return;
@@ -283,7 +283,7 @@ export class Component<T extends Env, Props extends {}, State extends {}> {
     if (shouldPatch) {
       patchQueue = [];
     }
-    const renderVDom = this.__render(force, patchQueue);
+    const renderVDom = this.__render(force, patchQueue, scope, vars);
     const renderId = __owl__.renderId;
     await renderVDom;
 
@@ -417,7 +417,9 @@ export class Component<T extends Env, Props extends {}, State extends {}> {
   async __updateProps(
     nextProps: Props,
     forceUpdate: boolean = false,
-    patchQueue?: any[]
+    patchQueue?: any[],
+    scope?: any,
+    vars?: any,
   ): Promise<void> {
     const shouldUpdate = forceUpdate || this.shouldUpdate(nextProps);
     if (shouldUpdate) {
@@ -427,7 +429,7 @@ export class Component<T extends Env, Props extends {}, State extends {}> {
       }
       await this.willUpdateProps(nextProps);
       this.props = nextProps;
-      await this.render(forceUpdate, patchQueue);
+      await this.render(forceUpdate, patchQueue, scope, vars);
     }
   }
 
@@ -441,14 +443,14 @@ export class Component<T extends Env, Props extends {}, State extends {}> {
     __owl__.vnode = patch(target, vnode);
   }
 
-  __prepare(): Promise<VNode> {
+  __prepare(scope?: Object, vars?: any): Promise<VNode> {
     const __owl__ = this.__owl__;
     __owl__.renderProps = this.props;
-    __owl__.renderPromise = this.__prepareAndRender();
+    __owl__.renderPromise = this.__prepareAndRender(scope, vars);
     return __owl__.renderPromise;
   }
 
-  async __prepareAndRender(): Promise<VNode> {
+  async __prepareAndRender(scope?: Object, vars?: any): Promise<VNode> {
     await this.willStart();
     const __owl__ = this.__owl__;
     if (__owl__.isDestroyed) {
@@ -480,10 +482,10 @@ export class Component<T extends Env, Props extends {}, State extends {}> {
     }
     __owl__.render = qweb.render.bind(qweb, this.template);
     this.__observeState();
-    return this.__render();
+    return this.__render(false, [], scope, vars);
   }
 
-  async __render(force: boolean = false, patchQueue: any[] = []): Promise<VNode> {
+  async __render(force: boolean = false, patchQueue: any[] = [], scope?: Object, vars?: any): Promise<VNode> {
     const __owl__ = this.__owl__;
     __owl__.renderId++;
     const promises: Promise<void>[] = [];
@@ -499,7 +501,9 @@ export class Component<T extends Env, Props extends {}, State extends {}> {
       handlers: __owl__.boundHandlers,
       mountedHandlers: __owl__.mountedHandlers,
       forceUpdate: force,
-      patchQueue
+      patchQueue,
+      scope,
+      vars
     });
     patch.push(vnode);
     if (__owl__.observer) {

src/qweb_core.ts

@@ -288,45 +288,44 @@ export class QWeb extends EventBus {
     return template.fn.call(this, context, extra);
   }
 
-  _compile(name: string, elem: Element, parentNode?: number): CompiledTemplate {
+  _compile(name: string, elem: Element, parentContext?: Context): CompiledTemplate {
     const isDebug = elem.attributes.hasOwnProperty("t-debug");
     const ctx = new Context(name);
-    if (parentNode) {
+    if (parentContext) {
-      ctx.nextID = parentNode + 1;
+      ctx.variables = Object.create(parentContext.variables);
-      ctx.parentNode = parentNode;
+      ctx.nextID = parentContext.parentNode! + 1;
+      ctx.parentNode = parentContext.parentNode!;
       ctx.allowMultipleRoots = true;
-      ctx.addLine(`let c${parentNode} = extra.parentNode;`);
+      ctx.addLine(`let c${ctx.parentNode} = extra.parentNode;`);
+
+      for (let v in parentContext.variables) {
+        let variable = <any>parentContext.variables[v];
+        if (variable.id) {
+          ctx.addLine(`let ${variable.id} = extra.vars.${variable.id}`);
+        }
+      }
+    }
+    if (parentContext) {
+      ctx.addLine("    Object.assign(context, extra.scope);");
     }
     this._compileNode(elem, ctx);
 
-    if (ctx.shouldProtectContext) {
+    if (!parentContext) {
-      ctx.code.unshift("    context = Object.create(context);");
-    }
-    if (ctx.shouldDefineOwner) {
-      // this is necessary to prevent some directives (t-forach for ex) to
-      // pollute the rendering context by adding some keys in it.
-      ctx.code.unshift("    let owner = context;");
-    }
-    if (ctx.shouldDefineQWeb) {
-      ctx.code.unshift("    let QWeb = this.constructor;");
-    }
-    if (ctx.shouldDefineUtils) {
-      ctx.code.unshift("    let utils = this.utils;");
-    }
-
-    if (!parentNode) {
       if (!ctx.rootNode) {
         throw new Error("A template should have one root node");
       }
       ctx.addLine(`return vn${ctx.rootNode};`);
     }
+
+    let code = ctx.generateCode();
+
     let template;
     try {
-      template = new Function("context", "extra", ctx.code.join("\n")) as CompiledTemplate;
+      template = new Function("context", "extra", code.join("\n")) as CompiledTemplate;
     } catch (e) {
       const templateName = ctx.templateName.replace(/`/g, "'");
       console.groupCollapsed(`Invalid Code generated by ${templateName}`);
-      console.warn(ctx.code.join("\n"));
+      console.warn(code.join("\n"));
       console.groupEnd();
       throw new Error(
         `Invalid generated code while compiling template '${templateName}': ${e.message}`
@@ -672,10 +671,12 @@ export class Context {
   shouldDefineQWeb: boolean = false;
   shouldDefineUtils: boolean = false;
   shouldProtectContext: boolean = false;
+  shouldTrackScope: boolean = false;
   inLoop: boolean = false;
   inPreTag: boolean = false;
   templateName: string;
   allowMultipleRoots: boolean = false;
+  scopeVars: any[] = [];
 
   constructor(name?: string) {
     this.rootContext = this;
@@ -688,6 +689,34 @@ export class Context {
     return id;
   }
 
+  generateCode(): string[] {
+    const shouldTrackScope = this.shouldTrackScope && this.scopeVars.length;
+    if (shouldTrackScope) {
+      // add some vars to scope if needed
+      for (let scopeVar of this.scopeVars.reverse()) {
+        let { index, key, indent } = scopeVar;
+        const prefix = new Array(indent + 2).join("    ");
+        this.code.splice(index + 1, 0, prefix + `scope.${key} = context.${key};`);
+      }
+      this.code.unshift("    const scope = Object.create(null);");
+    }
+    if (this.shouldProtectContext) {
+      this.code.unshift("    context = Object.create(context);");
+    }
+    if (this.shouldDefineOwner) {
+      // this is necessary to prevent some directives (t-forach for ex) to
+      // pollute the rendering context by adding some keys in it.
+      this.code.unshift("    let owner = context;");
+    }
+    if (this.shouldDefineQWeb) {
+      this.code.unshift("    let QWeb = this.constructor;");
+    }
+    if (this.shouldDefineUtils) {
+      this.code.unshift("    let utils = this.utils;");
+    }
+    return this.code;
+  }
+
   withParent(node: number): Context {
     if (
       !this.allowMultipleRoots &&
@@ -716,9 +745,15 @@ export class Context {
     this.indentLevel--;
   }
 
-  addLine(line: string) {
+  addLine(line: string): number {
     const prefix = new Array(this.indentLevel + 2).join("    ");
     this.code.push(prefix + line);
+    return this.code.length - 1;
+  }
+
+  addToScope(key: string, expr: string) {
+    const index = this.addLine(`context.${key} = ${expr};`);
+    this.rootContext.scopeVars.push({ index, key, indent: this.indentLevel });
   }
 
   addIf(condition: string) {

src/qweb_directives.ts

@@ -255,11 +255,11 @@ QWeb.addDirective({
     ctx.addLine(`var _length${keysID} = _${keysID}.length;`);
     ctx.addLine(`for (let i = 0; i < _length${keysID}; i++) {`);
     ctx.indent();
-    ctx.addLine(`context.${name}_first = i === 0;`);
+    ctx.addToScope(name + '_first', 'i === 0');
-    ctx.addLine(`context.${name}_last = i === _length${keysID} - 1;`);
+    ctx.addToScope(name + '_last', `i === _length${keysID} - 1`);
-    ctx.addLine(`context.${name}_index = i;`);
+    ctx.addToScope(name + '_index', 'i');
-    ctx.addLine(`context.${name} = _${keysID}[i];`);
+    ctx.addToScope(name, `_${keysID}[i]`);
-    ctx.addLine(`context.${name}_value = _${valuesID}[i];`);
+    ctx.addToScope(name + '_value', `_${valuesID}[i]`);
     const nodeCopy = <Element>node.cloneNode(true);
     let shouldWarn = nodeCopy.tagName !== "t" && !nodeCopy.hasAttribute("t-key");
     if (!shouldWarn && node.tagName === "t") {

src/qweb_extensions.ts

@@ -556,7 +556,16 @@ QWeb.addDirective({
     ctx.addLine(`context.__owl__.cmap[${templateID}] = w${componentID}.__owl__.id;`);
 
     // SLOTS
-    if (node.childNodes.length) {
+    const varDefs: string[] = [];
+    const hasSlots = node.childNodes.length;
+    if (hasSlots) {
+      ctx.rootContext.shouldTrackScope = true;
+      for (let v of Object.values(ctx.variables)) {
+        if (v["id"]) {
+          varDefs.push(v["id"]);
+        }
+      }
+
       const clone = <Element>node.cloneNode(true);
       const slotNodes = clone.querySelectorAll("[t-set]");
       const slotId = qweb.nextSlotId++;
@@ -567,7 +576,7 @@ QWeb.addDirective({
           slotNode.parentElement!.removeChild(slotNode);
           const key = slotNode.getAttribute("t-set")!;
           slotNode.removeAttribute("t-set");
-          const slotFn = qweb._compile(`slot_${key}_template`, slotNode, ctx.parentNode!);
+          const slotFn = qweb._compile(`slot_${key}_template`, slotNode, ctx);
           qweb.slots[`${slotId}_${key}`] = slotFn.bind(qweb);
         }
       }
@@ -576,12 +585,14 @@ QWeb.addDirective({
         for (let child of Object.values(clone.childNodes)) {
           t.appendChild(child);
         }
-        const slotFn = qweb._compile(`slot_default_template`, t, ctx.parentNode!);
+        const slotFn = qweb._compile(`slot_default_template`, t, ctx);
         qweb.slots[`${slotId}_default`] = slotFn.bind(qweb);
       }
     }
 
-    ctx.addLine(`def${defID} = w${componentID}.__prepare();`);
+    const scopeVars =
+      hasSlots && ctx.scopeVars.length ? `Object.assign({}, scope), {${varDefs.join(",")}}` : "";
+    ctx.addLine(`def${defID} = w${componentID}.__prepare(${scopeVars});`);
     // hack: specify empty remove hook to prevent the node from being removed from the DOM
     ctx.addLine(
       `def${defID} = def${defID}.then(vnode=>{${createHook}let pvnode=h(vnode.sel, {key: ${templateID}, hook: {insert(vn) {let nvn=w${componentID}.__mount(vnode, pvnode.elm);pvnode.elm=nvn.elm;${refExpr}${transitionsInsertCode}},remove() {},destroy(vn) {${finalizeComponentCode}}}});c${
@@ -596,7 +607,8 @@ QWeb.addDirective({
       ctx.addLine(`utils.validateProps(w${componentID}.constructor, props${componentID})`);
     }
     ctx.addLine(
-      `def${defID} = def${defID} || w${componentID}.__updateProps(props${componentID}, extra.forceUpdate, ${patchQueueCode});`
+      `def${defID} = def${defID} || w${componentID}.__updateProps(props${componentID}, extra.forceUpdate, ${patchQueueCode}${scopeVars &&
+        ", " + scopeVars});`
     );
     let keepAliveCode = "";
     if (keepAlive) {
@@ -774,7 +786,7 @@ QWeb.addDirective({
     ctx.addLine(
       `slot${slotKey}(context.__owl__.parent, Object.assign({}, extra, {parentNode: c${
         ctx.parentNode
-      }}));`
+      }, vars: extra.vars}));`
     );
     ctx.closeIf();
     return true;

tests/__snapshots__/component.test.ts.snap

@@ -1193,14 +1193,183 @@ exports[`t-slot directive can define and call slots 2`] = `
     c1.push(vn2);
     const slot3 = this.slots[context.__owl__.slotId + '_' + 'header'];
     if (slot3) {
-        slot3(context.__owl__.parent, Object.assign({}, extra, {parentNode: c2}));
+        slot3(context.__owl__.parent, Object.assign({}, extra, {parentNode: c2, vars: extra.vars}));
     }
     let c4 = [], p4 = {key:4};
     var vn4 = h('div', p4, c4);
     c1.push(vn4);
     const slot5 = this.slots[context.__owl__.slotId + '_' + 'footer'];
     if (slot5) {
-        slot5(context.__owl__.parent, Object.assign({}, extra, {parentNode: c4}));
+        slot5(context.__owl__.parent, Object.assign({}, extra, {parentNode: c4, vars: extra.vars}));
+    }
+    return vn1;
+}"
+`;
+
+exports[`t-slot directive slots are rendered with proper context, part 2 1`] = `
+"function anonymous(context,extra
+) {
+    var h = this.utils.h;
+    var _1 = context['props'].to;
+    let c2 = [], p2 = {key:2,attrs:{href: _1}};
+    var vn2 = h('a', p2, c2);
+    const slot3 = this.slots[context.__owl__.slotId + '_' + 'default'];
+    if (slot3) {
+        slot3(context.__owl__.parent, Object.assign({}, extra, {parentNode: c2, vars: extra.vars}));
+    }
+    return vn2;
+}"
+`;
+
+exports[`t-slot directive slots are rendered with proper context, part 2 2`] = `
+"function anonymous(context,extra
+) {
+    let utils = this.utils;
+    let QWeb = this.constructor;
+    let owner = context;
+    context = Object.create(context);
+    const scope = Object.create(null);
+    var h = this.utils.h;
+    let c1 = [], p1 = {key:1};
+    var vn1 = h('div', p1, c1);
+    let c2 = [], p2 = {key:2};
+    var vn2 = h('u', p2, c2);
+    c1.push(vn2);
+    var _3 = context['state'].users;
+    if (!_3) { throw new Error('QWeb error: Invalid loop expression')}
+    var _4 = _5 = _3;
+    if (!(_3 instanceof Array)) {
+        _4 = Object.keys(_3);
+        _5 = Object.values(_3);
+    }
+    var _length4 = _4.length;
+    for (let i = 0; i < _length4; i++) {
+        context.user_first = i === 0;
+        scope.user_first = context.user_first;
+        context.user_last = i === _length4 - 1;
+        scope.user_last = context.user_last;
+        context.user_index = i;
+        scope.user_index = context.user_index;
+        context.user = _4[i];
+        scope.user = context.user;
+        context.user_value = _5[i];
+        scope.user_value = context.user_value;
+        let c6 = [], p6 = {key:context['user'].id};
+        var vn6 = h('li', p6, c6);
+        c2.push(vn6);
+        //COMPONENT
+        let def8;
+        let w9 = String(-9 - i) in context.__owl__.cmap ? context.__owl__.children[context.__owl__.cmap[String(-9 - i)]] : false;
+        let _7_index = c6.length;
+        c6.push(null);
+        let props9 = {to:'/user/'+context['user'].id};
+        if (w9 && w9.__owl__.renderPromise && !w9.__owl__.vnode) {
+            if (utils.shallowEqual(props9, w9.__owl__.renderProps)) {
+                def8 = w9.__owl__.renderPromise;
+            } else {
+                w9.destroy();
+                w9 = false;
+            }
+        }
+        if (!w9) {
+            let componentKey9 = \`Link\`;
+            let W9 = context.components && context.components[componentKey9] || QWeb.components[componentKey9];
+            if (!W9) {throw new Error('Cannot find the definition of component \\"' + componentKey9 + '\\"')}
+            w9 = new W9(owner, props9);
+            context.__owl__.cmap[String(-9 - i)] = w9.__owl__.id;
+            w9.__owl__.slotId = 1;
+            def8 = w9.__prepare(Object.assign({}, scope), {});
+            def8 = def8.then(vnode=>{let pvnode=h(vnode.sel, {key: String(-9 - i), hook: {insert(vn) {let nvn=w9.__mount(vnode, pvnode.elm);pvnode.elm=nvn.elm;},remove() {},destroy(vn) {w9.destroy();}}});c6[_7_index]=pvnode;w9.__owl__.pvnode = pvnode;});
+        } else {
+            def8 = def8 || w9.__updateProps(props9, extra.forceUpdate, extra.patchQueue, Object.assign({}, scope), {});
+            def8 = def8.then(()=>{if (w9.__owl__.isDestroyed) {return};let pvnode=w9.__owl__.pvnode;c6[_7_index]=pvnode;});
+        }
+        extra.promises.push(def8);
+    }
+    return vn1;
+}"
+`;
+
+exports[`t-slot directive slots are rendered with proper context, part 3 1`] = `
+"function anonymous(context,extra
+) {
+    var h = this.utils.h;
+    var _1 = context['props'].to;
+    let c2 = [], p2 = {key:2,attrs:{href: _1}};
+    var vn2 = h('a', p2, c2);
+    const slot3 = this.slots[context.__owl__.slotId + '_' + 'default'];
+    if (slot3) {
+        slot3(context.__owl__.parent, Object.assign({}, extra, {parentNode: c2, vars: extra.vars}));
+    }
+    return vn2;
+}"
+`;
+
+exports[`t-slot directive slots are rendered with proper context, part 3 2`] = `
+"function anonymous(context,extra
+) {
+    let utils = this.utils;
+    let QWeb = this.constructor;
+    let owner = context;
+    context = Object.create(context);
+    const scope = Object.create(null);
+    var h = this.utils.h;
+    let c1 = [], p1 = {key:1};
+    var vn1 = h('div', p1, c1);
+    let c2 = [], p2 = {key:2};
+    var vn2 = h('u', p2, c2);
+    c1.push(vn2);
+    var _3 = context['state'].users;
+    if (!_3) { throw new Error('QWeb error: Invalid loop expression')}
+    var _4 = _5 = _3;
+    if (!(_3 instanceof Array)) {
+        _4 = Object.keys(_3);
+        _5 = Object.values(_3);
+    }
+    var _length4 = _4.length;
+    for (let i = 0; i < _length4; i++) {
+        context.user_first = i === 0;
+        scope.user_first = context.user_first;
+        context.user_last = i === _length4 - 1;
+        scope.user_last = context.user_last;
+        context.user_index = i;
+        scope.user_index = context.user_index;
+        context.user = _4[i];
+        scope.user = context.user;
+        context.user_value = _5[i];
+        scope.user_value = context.user_value;
+        let c6 = [], p6 = {key:context['user'].id};
+        var vn6 = h('li', p6, c6);
+        c2.push(vn6);
+        var _7 = 'User '+context['user'].name;
+        //COMPONENT
+        let def9;
+        let w10 = String(-10 - i) in context.__owl__.cmap ? context.__owl__.children[context.__owl__.cmap[String(-10 - i)]] : false;
+        let _8_index = c6.length;
+        c6.push(null);
+        let props10 = {to:'/user/'+context['user'].id};
+        if (w10 && w10.__owl__.renderPromise && !w10.__owl__.vnode) {
+            if (utils.shallowEqual(props10, w10.__owl__.renderProps)) {
+                def9 = w10.__owl__.renderPromise;
+            } else {
+                w10.destroy();
+                w10 = false;
+            }
+        }
+        if (!w10) {
+            let componentKey10 = \`Link\`;
+            let W10 = context.components && context.components[componentKey10] || QWeb.components[componentKey10];
+            if (!W10) {throw new Error('Cannot find the definition of component \\"' + componentKey10 + '\\"')}
+            w10 = new W10(owner, props10);
+            context.__owl__.cmap[String(-10 - i)] = w10.__owl__.id;
+            w10.__owl__.slotId = 1;
+            def9 = w10.__prepare(Object.assign({}, scope), {_7});
+            def9 = def9.then(vnode=>{let pvnode=h(vnode.sel, {key: String(-10 - i), hook: {insert(vn) {let nvn=w10.__mount(vnode, pvnode.elm);pvnode.elm=nvn.elm;},remove() {},destroy(vn) {w10.destroy();}}});c6[_8_index]=pvnode;w10.__owl__.pvnode = pvnode;});
+        } else {
+            def9 = def9 || w10.__updateProps(props10, extra.forceUpdate, extra.patchQueue, Object.assign({}, scope), {_7});
+            def9 = def9.then(()=>{if (w10.__owl__.isDestroyed) {return};let pvnode=w10.__owl__.pvnode;c6[_8_index]=pvnode;});
+        }
+        extra.promises.push(def9);
     }
     return vn1;
 }"

tests/component.test.ts

@@ -2920,6 +2920,82 @@ describe("t-slot directive", () => {
     );
   });
 
+  test("slots are rendered with proper context, part 2", async () => {
+    env.qweb.addTemplates(`
+        <templates>
+            <a t-name="Link" t-att-href="props.to">
+                <t t-slot="default"/>
+            </a>
+            <div t-name="App">
+                <u><li t-foreach="state.users" t-as="user" t-key="user.id">
+                    <Link to="'/user/' + user.id">User <t t-esc="user.name"/></Link>
+                </li></u>
+            </div>
+        </templates>
+    `);
+    class Link extends Widget {}
+
+    class App extends Widget {
+        state = {users: [{id: 1, name: 'Aaron'}, {id: 2, name: 'David'}]};
+        components = { Link };
+    }
+
+    const app = new App(env);
+    await app.mount(fixture);
+
+    expect(fixture.innerHTML).toBe(
+      "<div><u><li><a href=\"/user/1\">User Aaron</a></li><li><a href=\"/user/2\">User David</a></li></u></div>"
+    );
+    expect(env.qweb.templates.Link.fn.toString()).toMatchSnapshot();
+    expect(env.qweb.templates.App.fn.toString()).toMatchSnapshot();
+
+    // test updateprops here
+    app.state.users[1].name = "Mathieu";
+    await nextTick();
+    expect(fixture.innerHTML).toBe(
+      "<div><u><li><a href=\"/user/1\">User Aaron</a></li><li><a href=\"/user/2\">User Mathieu</a></li></u></div>"
+    );
+  });
+
+  test("slots are rendered with proper context, part 3", async () => {
+    env.qweb.addTemplates(`
+        <templates>
+            <a t-name="Link" t-att-href="props.to">
+                <t t-slot="default"/>
+            </a>
+            <div t-name="App">
+                <u><li t-foreach="state.users" t-as="user" t-key="user.id" >
+                    <t t-set="userdescr" t-value="'User ' + user.name"/>
+                    <Link to="'/user/' + user.id"><t t-esc="userdescr"/></Link>
+                </li></u>
+            </div>
+        </templates>
+    `);
+    class Link extends Widget {}
+
+    class App extends Widget {
+        state = {users: [{id: 1, name: 'Aaron'}, {id: 2, name: 'David'}]};
+        components = { Link };
+    }
+
+    const app = new App(env);
+    await app.mount(fixture);
+
+    expect(fixture.innerHTML).toBe(
+      "<div><u><li><a href=\"/user/1\">User Aaron</a></li><li><a href=\"/user/2\">User David</a></li></u></div>"
+    );
+    expect(env.qweb.templates.Link.fn.toString()).toMatchSnapshot();
+    expect(env.qweb.templates.App.fn.toString()).toMatchSnapshot();
+
+    // test updateprops here
+    app.state.users[1].name = "Mathieu";
+    await nextTick();
+    expect(fixture.innerHTML).toBe(
+      "<div><u><li><a href=\"/user/1\">User Aaron</a></li><li><a href=\"/user/2\">User Mathieu</a></li></u></div>"
+    );
+
+  });
+
   test("refs are properly bound in slots", async () => {
     env.qweb.addTemplates(`
         <templates>