[FIX] parser, code_generator: wrapped children recognition

Several directives (t-key, t-log, t-translation, …) are represented by wrapper ASTs that contain another AST or null. Because these wrappers don't share the same type as their children, various AST type checks were broken. This commit addresses those issues by: - Commuting Translation / TranslationContext and Multi ASTs so that Multi children are spread as expected (see parseChildren). - Parsing the t-key directive before t-esc / t-out (in line with https://github.com/odoo/owl/pull/1685). - Ensuring wrappers around TSet ASTs are recognized as having no representation, so compileMulti properly discards children equivalent to TSet.
2025-10-06 19:59:41 +07:00 · 2025-09-10 15:17:25 +02:00
parent c2728c9daf
commit 521111644c
9 changed files with 300 additions and 45 deletions
@@ -995,7 +995,7 @@ export class CodeGenerator {
    const isNewBlock = !block || forceNewBlock;
    let codeIdx = this.target.code.length;
    if (isNewBlock) {
-      const n = ast.content.filter((c) => c.type !== ASTType.TSet).length;
+      const n = ast.content.filter((c) => !c.hasNoRepresentation).length;
      let result: string | null = null;
      if (n <= 1) {
        for (let child of ast.content) {
@@ -1009,15 +1009,15 @@ export class CodeGenerator {
    let index = 0;
    for (let i = 0, l = ast.content.length; i < l; i++) {
      const child = ast.content[i];
-      const isTSet = child.type === ASTType.TSet;
+      const forceNewBlock = !child.hasNoRepresentation;
      const subCtx = createContext(ctx, {
        block,
        index,
-        forceNewBlock: !isTSet,
+        forceNewBlock,
        isLast: ctx.isLast && i === l - 1,
      });
      this.compileAST(child, subCtx);
-      if (!isTSet) {
+      if (forceNewBlock) {
        index++;
      }
    }
@@ -31,12 +31,17 @@ export const enum ASTType {
  TPortal,
 }

-export interface ASTText {
+export interface BaseAST {
+  type: ASTType;
+  hasNoRepresentation?: true;
+}
+
+export interface ASTText extends BaseAST {
  type: ASTType.Text;
  value: string;
 }

-export interface ASTComment {
+export interface ASTComment extends BaseAST {
  type: ASTType.Comment;
  value: string;
 }
@@ -52,7 +57,7 @@ interface TModelInfo {
  specialInitTargetAttr: string | null;
 }

-export interface ASTDomNode {
+export interface ASTDomNode extends BaseAST {
  type: ASTType.DomNode;
  tag: string;
  content: AST[];
@@ -65,24 +70,24 @@ export interface ASTDomNode {
  ns: string | null;
 }

-export interface ASTMulti {
+export interface ASTMulti extends BaseAST {
  type: ASTType.Multi;
  content: AST[];
 }

-export interface ASTTEsc {
+export interface ASTTEsc extends BaseAST {
  type: ASTType.TEsc;
  expr: string;
  defaultValue: string;
 }

-export interface ASTTOut {
+export interface ASTTOut extends BaseAST {
  type: ASTType.TOut;
  expr: string;
  body: AST[] | null;
 }

-export interface ASTTif {
+export interface ASTTif extends BaseAST {
  type: ASTType.TIf;
  condition: string;
  content: AST;
@@ -90,15 +95,16 @@ export interface ASTTif {
  tElse: AST | null;
 }

-export interface ASTTSet {
+export interface ASTTSet extends BaseAST {
  type: ASTType.TSet;
  name: string;
  value: string | null; // value defined in attribute
  defaultValue: string | null; // value defined in body, if text
  body: AST[] | null; // content of body if not text
+  hasNoRepresentation: true;
 }

-export interface ASTTForEach {
+export interface ASTTForEach extends BaseAST {
  type: ASTType.TForEach;
  collection: string;
  elem: string;
@@ -111,13 +117,13 @@ export interface ASTTForEach {
  key: string | null;
 }

-export interface ASTTKey {
+export interface ASTTKey extends BaseAST {
  type: ASTType.TKey;
  expr: string;
  content: AST;
 }

-export interface ASTTCall {
+export interface ASTTCall extends BaseAST {
  type: ASTType.TCall;
  name: string;
  body: AST[] | null;
@@ -132,7 +138,7 @@ interface SlotDefinition {
  attrsTranslationCtx: Attrs | null;
 }

-export interface ASTComponent {
+export interface ASTComponent extends BaseAST {
  type: ASTType.TComponent;
  name: string;
  isDynamic: boolean;
@@ -143,7 +149,7 @@ export interface ASTComponent {
  slots: { [name: string]: SlotDefinition } | null;
 }

-export interface ASTSlot {
+export interface ASTSlot extends BaseAST {
  type: ASTType.TSlot;
  name: string;
  attrs: Attrs | null;
@@ -152,34 +158,34 @@ export interface ASTSlot {
  defaultContent: AST | null;
 }

-export interface ASTTCallBlock {
+export interface ASTTCallBlock extends BaseAST {
  type: ASTType.TCallBlock;
  name: string;
 }

-export interface ASTDebug {
+export interface ASTDebug extends BaseAST {
  type: ASTType.TDebug;
  content: AST | null;
 }

-export interface ASTLog {
+export interface ASTLog extends BaseAST {
  type: ASTType.TLog;
  expr: string;
  content: AST | null;
 }

-export interface ASTTranslation {
+export interface ASTTranslation extends BaseAST {
  type: ASTType.TTranslation;
  content: AST | null;
 }

-export interface ASTTranslationContext {
+export interface ASTTranslationContext extends BaseAST {
  type: ASTType.TTranslationContext;
  content: AST | null;
  translationCtx: string;
 }

-export interface ASTTPortal {
+export interface ASTTPortal extends BaseAST {
  type: ASTType.TPortal;
  target: string;
  content: AST;
@@ -255,9 +261,9 @@ function parseNode(node: Node, ctx: ParsingContext): AST | null {
    parseTCallBlock(node, ctx) ||
    parseTTranslation(node, ctx) ||
    parseTTranslationContext(node, ctx) ||
+    parseTKey(node, ctx) ||
    parseTEscNode(node, ctx) ||
    parseTOutNode(node, ctx) ||
-    parseTKey(node, ctx) ||
    parseTSlot(node, ctx) ||
    parseComponent(node, ctx) ||
    parseDOMNode(node, ctx) ||
@@ -334,20 +340,30 @@ function parseTCustom(node: Element, ctx: ParsingContext): AST | null {
 function parseTDebugLog(node: Element, ctx: ParsingContext): AST | null {
  if (node.hasAttribute("t-debug")) {
    node.removeAttribute("t-debug");
-    return {
+    const content = parseNode(node, ctx);
+    const ast: ASTDebug = {
      type: ASTType.TDebug,
-      content: parseNode(node, ctx),
+      content,
    };
+    if (content?.hasNoRepresentation) {
+      ast.hasNoRepresentation = true;
+    }
+    return ast;
  }

  if (node.hasAttribute("t-log")) {
    const expr = node.getAttribute("t-log")!;
    node.removeAttribute("t-log");
-    return {
+    const content = parseNode(node, ctx);
+    const ast: ASTLog = {
      type: ASTType.TLog,
      expr,
-      content: parseNode(node, ctx),
+      content,
    };
+    if (content?.hasNoRepresentation) {
+      ast.hasNoRepresentation = true;
+    }
+    return ast;
  }
  return null;
 }
@@ -598,11 +614,19 @@ function parseTKey(node: Element, ctx: ParsingContext): AST | null {
  }
  const key = node.getAttribute("t-key")!;
  node.removeAttribute("t-key");
-  const body = parseNode(node, ctx);
-  if (!body) {
+  const content = parseNode(node, ctx);
+  if (!content) {
    return null;
  }
-  return { type: ASTType.TKey, expr: key, content: body };
+  const ast: ASTTKey = {
+    type: ASTType.TKey,
+    expr: key,
+    content,
+  };
+  if (content.hasNoRepresentation) {
+    ast.hasNoRepresentation = true;
+  }
+  return ast;
 }

 // -----------------------------------------------------------------------------
@@ -724,7 +748,7 @@ function parseTSetNode(node: Element, ctx: ParsingContext): AST | null {
  if (node.textContent !== node.innerHTML) {
    body = parseChildren(node, ctx);
  }
-  return { type: ASTType.TSet, name, value, defaultValue, body };
+  return { type: ASTType.TSet, name, value, defaultValue, body, hasNoRepresentation: true };
 }

 // -----------------------------------------------------------------------------
@@ -916,32 +940,55 @@ function parseTSlot(node: Element, ctx: ParsingContext): AST | null {
 // Translation
 // -----------------------------------------------------------------------------

+function wrapInTTranslationAST(r: AST | null) {
+  const ast: ASTTranslation = { type: ASTType.TTranslation, content: r };
+  if (r?.hasNoRepresentation) {
+    ast.hasNoRepresentation = true;
+  }
+  return ast;
+}
+
 function parseTTranslation(node: Element, ctx: ParsingContext): AST | null {
  if (node.getAttribute("t-translation") !== "off") {
    return null;
  }
  node.removeAttribute("t-translation");
-  return {
-    type: ASTType.TTranslation,
-    content: parseNode(node, ctx),
-  };
+  const result = parseNode(node, ctx);
+  if (result?.type === ASTType.Multi) {
+    const children = result.content.map(wrapInTTranslationAST);
+    return makeASTMulti(children);
+  }
+  return wrapInTTranslationAST(result);
 }

 // -----------------------------------------------------------------------------
 // Translation Context
 // -----------------------------------------------------------------------------

+function wrapInTTranslationContextAST(r: AST | null, translationCtx: string) {
+  const ast: ASTTranslationContext = {
+    type: ASTType.TTranslationContext,
+    content: r,
+    translationCtx,
+  };
+  if (r?.hasNoRepresentation) {
+    ast.hasNoRepresentation = true;
+  }
+  return ast;
+}
+
 function parseTTranslationContext(node: Element, ctx: ParsingContext): AST | null {
  const translationCtx = node.getAttribute("t-translation-context");
  if (!translationCtx) {
    return null;
  }
  node.removeAttribute("t-translation-context");
-  return {
-    type: ASTType.TTranslationContext,
-    content: parseNode(node, ctx),
-    translationCtx,
-  };
+  const result = parseNode(node, ctx);
+  if (result?.type === ASTType.Multi) {
+    const children = result.content.map((c) => wrapInTTranslationContextAST(c, translationCtx));
+    return makeASTMulti(children);
+  }
+  return wrapInTTranslationContextAST(result, translationCtx);
 }

 // -----------------------------------------------------------------------------
@@ -990,6 +1037,14 @@ function parseChildren(node: Element, ctx: ParsingContext): AST[] {
  return children;
 }

+function makeASTMulti(children: AST[]) {
+  const ast: ASTMulti = { type: ASTType.Multi, content: children };
+  if (children.every((c) => c.hasNoRepresentation)) {
+    ast.hasNoRepresentation = true;
+  }
+  return ast;
+}
+
 /**
 * Parse all the child nodes of a given node and return an ast if possible.
 * In the case there are multiple children, they are wrapped in a astmulti.
@@ -1002,7 +1057,7 @@ function parseChildNodes(node: Element, ctx: ParsingContext): AST | null {
    case 1:
      return children[0];
    default:
-      return { type: ASTType.Multi, content: children };
+      return makeASTMulti(children);
  }
 }

@@ -49,6 +49,27 @@ exports[`debugging t-debug on sub template 2`] = `
 }"
 `;

+exports[`debugging t-debug: interaction with t-set 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  let { isBoundary, withDefault, setContextValue } = helpers;
+  
+  let block1 = createBlock(\`<span><block-text-0/></span>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    ctx = Object.create(ctx);
+    ctx[isBoundary] = 1
+    debugger;
+    setContextValue(ctx, \\"foo\\", 42);
+    debugger;
+    setContextValue(ctx, \\"bar\\", 49);
+    let txt1 = ctx['foo']+ctx['bar'];
+    return block1([txt1]);
+  }
+}"
+`;
+
 exports[`debugging t-log 1`] = `
 "function anonymous(app, bdom, helpers
 ) {
@@ -66,3 +87,24 @@ exports[`debugging t-log 1`] = `
  }
 }"
 `;
+
+exports[`debugging t-log: interaction with t-set 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  let { isBoundary, withDefault, setContextValue } = helpers;
+  
+  let block1 = createBlock(\`<span><block-text-0/></span>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    ctx = Object.create(ctx);
+    ctx[isBoundary] = 1
+    console.log(ctx['foo']);
+    setContextValue(ctx, \\"foo\\", 42);
+    console.log(ctx['bar']);
+    setContextValue(ctx, \\"bar\\", 49);
+    let txt1 = ctx['foo']+ctx['bar'];
+    return block1([txt1]);
+  }
+}"
+`;
@@ -103,3 +103,18 @@ exports[`t-key t-key on sub dom node pushes a child block in its parent 2`] = `
  }
 }"
 `;
+
+exports[`t-key t-key: interaction with t-esc 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  let block1 = createBlock(\`<p><block-text-0/></p>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    const tKey_1 = ctx['key'];
+    let txt1 = ctx['text'];
+    return toggler(tKey_1, block1([txt1]));
+  }
+}"
+`;
@@ -10,8 +10,7 @@ exports[`translation context body of t-sets are translated in context 1`] = `
    ctx = Object.create(ctx);
    ctx[isBoundary] = 1
    setContextValue(ctx, \\"label\\", \`traduit\`);
-    const b2 = text(ctx['label']);
-    return multi([b2]);
+    return text(ctx['label']);
  }
 }"
 `;
@@ -94,6 +93,23 @@ exports[`translation context slot attrs and text contents are translated in cont
 }"
 `;

+exports[`translation context t-translation-context with several children 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  let block1 = createBlock(\`<div><div/><div/><block-child-0/></div>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let b2;
+    if (true) {
+      b2 = text(\`\`);
+    }
+    return block1([], [b2]);
+  }
+}"
+`;
+
 exports[`translation context translation of attributes in context 1`] = `
 "function anonymous(app, bdom, helpers
 ) {
@@ -153,6 +169,21 @@ exports[`translation support body of t-sets inside translation=off are not trans
 }"
 `;

+exports[`translation support body of t-sets inside translation=off are not translated 2 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  let { isBoundary, withDefault, setContextValue } = helpers;
+  
+  return function template(ctx, node, key = \\"\\") {
+    ctx = Object.create(ctx);
+    ctx[isBoundary] = 1
+    setContextValue(ctx, \\"label\\", \`untranslated\`);
+    return text(ctx['label']);
+  }
+}"
+`;
+
 exports[`translation support body of t-sets with html content are translated 1`] = `
 "function anonymous(app, bdom, helpers
 ) {
@@ -264,6 +295,23 @@ exports[`translation support t-set and falsy t-value: t-body are translated 1`]
 }"
 `;

+exports[`translation support t-translation with several children 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  let block1 = createBlock(\`<div><div/><div/><block-child-0/></div>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let b2;
+    if (true) {
+      b2 = text(\`\`);
+    }
+    return block1([], [b2]);
+  }
+}"
+`;
+
 exports[`translation support translation is done on the trimmed text, with extra spaces readded after 1`] = `
 "function anonymous(app, bdom, helpers
 ) {
@@ -692,6 +692,7 @@ describe("qweb parser", () => {
      value: "value",
      defaultValue: null,
      body: null,
+      hasNoRepresentation: true,
    });
  });

@@ -702,6 +703,7 @@ describe("qweb parser", () => {
      defaultValue: "ok",
      value: null,
      body: null,
+      hasNoRepresentation: true,
    });

    expect(parse(`<t t-set="v"><div>ok</div></t>`)).toEqual({
@@ -723,6 +725,7 @@ describe("qweb parser", () => {
          content: [{ type: ASTType.Text, value: "ok" }],
        },
      ],
+      hasNoRepresentation: true,
    });

    expect(parse(`<t t-set="v"><div>ok</div>abc</t>`)).toEqual({
@@ -745,6 +748,7 @@ describe("qweb parser", () => {
        },
        { type: ASTType.Text, value: "abc" },
      ],
+      hasNoRepresentation: true,
    });
  });

@@ -758,6 +762,7 @@ describe("qweb parser", () => {
        defaultValue: "ok",
        value: null,
        body: null,
+        hasNoRepresentation: true,
      },
      tElif: null,
      tElse: null,
@@ -783,7 +788,14 @@ describe("qweb parser", () => {
          condition: "flag",
          content: { type: ASTType.Text, value: "1" },
          tElif: null,
-          tElse: { type: ASTType.TSet, name: "ourvar", value: "0", defaultValue: null, body: null },
+          tElse: {
+            type: ASTType.TSet,
+            name: "ourvar",
+            value: "0",
+            defaultValue: null,
+            body: null,
+            hasNoRepresentation: true,
+          },
        },
      ],
    });
@@ -38,4 +38,34 @@ describe("debugging", () => {
    expect(console.log).toHaveBeenCalledWith(45);
    console.log = consoleLog;
  });
+
+  test("t-log: interaction with t-set", () => {
+    const consoleLog = console.log;
+    console.log = jest.fn();
+
+    const template = `
+      <t>
+        <t t-log="foo" t-set="foo" t-value="42"/>
+        <t t-log="bar" t-set="bar" t-value="49"/>
+        <span t-esc="foo + bar"/>
+      </t>
+    `;
+    snapshotTemplate(template);
+    renderToString(template);
+    expect(console.log).toHaveBeenCalledWith(undefined);
+    expect(console.log).toHaveBeenCalledWith(undefined);
+    console.log = consoleLog;
+  });
+
+  test("t-debug: interaction with t-set", () => {
+    const template = `
+      <t>
+        <t t-debug="" t-set="foo" t-value="42"/>
+        <t t-debug="" t-set="bar" t-value="49"/>
+        <span t-esc="foo + bar"/>
+      </t>
+    `;
+    snapshotTemplate(template);
+    renderToString(template);
+  });
 });
@@ -63,4 +63,10 @@ describe("t-key", () => {

    expect(renderToString(template2, { key: "1" })).toBe("<div><h1></h1></div>");
  });
+
+  test("t-key: interaction with t-esc", async () => {
+    const template = `<p t-key="key" t-esc="text"/>`;
+
+    expect(renderToString(template, { key: "1", text: "abc" })).toBe("<p>abc</p>");
+  });
 });
@@ -129,6 +129,21 @@ describe("translation support", () => {
    expect(fixture.innerHTML).toBe("untranslated");
  });

+  test("body of t-sets inside translation=off are not translated 2", async () => {
+    class SomeComponent extends Component {
+      static template = xml`
+        <t>
+          <t t-translation="off" t-set="label">untranslated</t>
+          <t t-esc="label"/>
+        </t>`;
+    }
+
+    const translateFn = () => "translated";
+
+    await mount(SomeComponent, fixture, { translateFn });
+    expect(fixture.innerHTML).toBe("untranslated");
+  });
+
  test("body of t-sets with html content are translated", async () => {
    class SomeComponent extends Component {
      static template = xml`
@@ -170,6 +185,22 @@ describe("translation support", () => {
    await mount(SomeComponent, fixture, { translateFn });
    expect(fixture.innerHTML).toBe("translated");
  });
+
+  test("t-translation with several children", async () => {
+    class SomeComponent extends Component {
+      static template = xml`
+          <div>
+            <t t-translation="off">
+                <div/>
+                <div/>
+            </t>
+            <t t-if="true"/>
+        </div>
+      `;
+    }
+    await mount(SomeComponent, fixture);
+    expect(fixture.outerHTML).toBe("<div><div><div></div><div></div></div></div>");
+  });
 });

 describe("translation context", () => {
@@ -293,4 +324,20 @@ describe("translation context", () => {
    expect(translateFn).toHaveBeenCalledWith("param", "fr");
    expect(translateFn).toHaveBeenCalledWith("title", "pt");
  });
+
+  test("t-translation-context with several children", async () => {
+    class SomeComponent extends Component {
+      static template = xml`
+          <div>
+            <t t-translation-context="ctx">
+                <div/>
+                <div/>
+            </t>
+            <t t-if="true"/>
+        </div>
+      `;
+    }
+    await mount(SomeComponent, fixture);
+    expect(fixture.outerHTML).toBe("<div><div><div></div><div></div></div></div>");
+  });
 });