[FIX] compiler: properly handle t-set in t-if with no content

Before this commit, whenever Owl encounter a t-if, it generates an
anchor (a "hole") in the current block being compiled. However, in some
cases, the content of the t-if may not have any content at all, and the
anchor is then useless. Worse, the code generating the anchor generates
an index based on the number of sub blocks, but if there is no content,
the next anchor being created will have the same index, which then may
cause weird bugs.

A possible way to fix this could be to make sure we increment properly
the anchor index, but we could even do better: not having an anchor at
all.
This commit is contained in:
Géry Debongnie
2022-06-29 10:18:14 +02:00
committed by Sam Degueldre
parent 76c389a7a8
commit 382e3e4010
4 changed files with 66 additions and 29 deletions
+23 -25
View File
@@ -143,7 +143,7 @@ interface Context {
ctxVar?: string;
}
function createContext(parentCtx: Context, params?: Partial<Context>) {
function createContext(parentCtx: Context, params?: Partial<Context>): Context {
return Object.assign(
{
block: null,
@@ -334,8 +334,8 @@ export class CodeGenerator {
this.addLine(`const ${varName} = ${expr};`);
}
insertAnchor(block: BlockDescription) {
const tag = `block-child-${block.children.length}`;
insertAnchor(block: BlockDescription, index: number = block.children.length) {
const tag = `block-child-${index}`;
const anchor = xmlDoc.createElement(tag);
block.insert(anchor);
}
@@ -692,7 +692,7 @@ export class CodeGenerator {
const children = ast.content;
for (let i = 0; i < children.length; i++) {
const child = ast.content[i];
const subCtx: Context = createContext(ctx, {
const subCtx = createContext(ctx, {
block,
index: block!.childNumber,
forceNewBlock: false,
@@ -761,7 +761,7 @@ export class CodeGenerator {
} else if (ast.body) {
let bodyValue = null;
bodyValue = BlockDescription.nextBlockId;
const subCtx: Context = createContext(ctx);
const subCtx = createContext(ctx);
this.compileAST({ type: ASTType.Multi, content: ast.body }, subCtx);
this.helpers.add("safeOutput");
blockStr = `safeOutput(${compileExpr(ast.expr)}, b${bodyValue})`;
@@ -772,9 +772,19 @@ export class CodeGenerator {
this.insertBlock(blockStr, block, ctx);
}
compileTIfBranch(content: AST, block: BlockDescription, ctx: Context) {
this.target.indentLevel++;
let childN = block.children.length;
this.compileAST(content, createContext(ctx, { block, index: ctx.index }));
if (block.children.length > childN) {
// we have some content => need to insert an anchor at correct index
this.insertAnchor(block!, childN);
}
this.target.indentLevel--;
}
compileTIf(ast: ASTTif, ctx: Context, nextNode?: ASTDomNode) {
let { block, forceNewBlock, index } = ctx;
let currentIndex = index;
let { block, forceNewBlock } = ctx;
const codeIdx = this.target.code.length;
const isNewBlock = !block || (block.type !== "multi" && forceNewBlock);
if (block) {
@@ -784,28 +794,16 @@ export class CodeGenerator {
block = this.createBlock(block, "multi", ctx);
}
this.addLine(`if (${compileExpr(ast.condition)}) {`);
this.target.indentLevel++;
this.insertAnchor(block!);
const subCtx: Context = createContext(ctx, { block, index: currentIndex });
this.compileAST(ast.content, subCtx);
this.target.indentLevel--;
this.compileTIfBranch(ast.content, block, ctx);
if (ast.tElif) {
for (let clause of ast.tElif) {
this.addLine(`} else if (${compileExpr(clause.condition)}) {`);
this.target.indentLevel++;
this.insertAnchor(block);
const subCtx: Context = createContext(ctx, { block, index: currentIndex });
this.compileAST(clause.content, subCtx);
this.target.indentLevel--;
this.compileTIfBranch(clause.content, block, ctx);
}
}
if (ast.tElse) {
this.addLine(`} else {`);
this.target.indentLevel++;
this.insertAnchor(block);
const subCtx: Context = createContext(ctx, { block, index: currentIndex });
this.compileAST(ast.tElse, subCtx);
this.target.indentLevel--;
this.compileTIfBranch(ast.tElse, block, ctx);
}
this.addLine("}");
if (isNewBlock) {
@@ -891,7 +889,7 @@ export class CodeGenerator {
this.addLine("}");
}
const subCtx: Context = createContext(ctx, { block, index: loopVar });
const subCtx = createContext(ctx, { block, index: loopVar });
this.compileAST(ast.body, subCtx);
if (ast.memo) {
this.addLine(
@@ -938,7 +936,7 @@ export class CodeGenerator {
for (let i = 0, l = ast.content.length; i < l; i++) {
const child = ast.content[i];
const isTSet = child.type === ASTType.TSet;
const subCtx: Context = createContext(ctx, {
const subCtx = createContext(ctx, {
block,
index,
forceNewBlock: !isTSet,
@@ -984,7 +982,7 @@ export class CodeGenerator {
this.addLine(`${ctxVar}[isBoundary] = 1;`);
this.helpers.add("isBoundary");
const nextId = BlockDescription.nextBlockId;
const subCtx: Context = createContext(ctx, { preventRoot: true, ctxVar });
const subCtx = createContext(ctx, { preventRoot: true, ctxVar });
this.compileAST({ type: ASTType.Multi, content: ast.body }, subCtx);
if (nextId !== BlockDescription.nextBlockId) {
this.helpers.add("zero");
@@ -534,13 +534,36 @@ exports[`t-set t-set with t-value (truthy) and body 1`] = `
}"
`;
exports[`t-set t-set, multiple t-ifs, and a specific configuration 1`] = `
"function anonymous(app, bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, comment } = bdom;
let { isBoundary, withDefault, setContextValue } = helpers;
let block1 = createBlock(\`<p><div><span>First div</span></div><div><block-child-0/></div></p>\`);
return function template(ctx, node, key = \\"\\") {
ctx = Object.create(ctx);
ctx[isBoundary] = 1
let b2;
if (ctx['flag']) {
setContextValue(ctx, \\"bouh\\", 2);
}
if (!ctx['flag']) {
b2 = text(\`Second\`);
}
return block1([], [b2]);
}
}"
`;
exports[`t-set t-set, t-if, and mix of expression/body lookup, 1 1`] = `
"function anonymous(app, bdom, helpers
) {
let { text, createBlock, list, multi, html, toggler, comment } = bdom;
let { isBoundary, withDefault, setContextValue } = helpers;
let block1 = createBlock(\`<div><block-child-0/><block-child-0/><block-text-0/></div>\`);
let block1 = createBlock(\`<div><block-text-0/></div>\`);
return function template(ctx, node, key = \\"\\") {
ctx = Object.create(ctx);
@@ -562,7 +585,7 @@ exports[`t-set t-set, t-if, and mix of expression/body lookup, 2 1`] = `
let { text, createBlock, list, multi, html, toggler, comment } = bdom;
let { isBoundary, withDefault, setContextValue } = helpers;
let block1 = createBlock(\`<div><block-child-0/><block-child-0/><block-text-0/></div>\`);
let block1 = createBlock(\`<div><block-text-0/></div>\`);
return function template(ctx, node, key = \\"\\") {
ctx = Object.create(ctx);
+16
View File
@@ -33,6 +33,22 @@ describe("t-set", () => {
expect(renderToString(template, { value: "ok" })).toBe("<div>grimbergen</div>");
});
test("t-set, multiple t-ifs, and a specific configuration", () => {
const template = `
<p>
<div>
<t t-if="flag" t-set="bouh" t-value="2"/>
<span>First div</span>
</div>
<div>
<t t-if="!flag">Second</t>
</div>
</p>`;
expect(renderToString(template)).toBe(
"<p><div><span>First div</span></div><div>Second</div></p>"
);
});
test("set from body literal", () => {
const template = `<t><t t-set="value">ok</t><t t-esc="value"/></t>`;
expect(renderToString(template)).toBe("ok");
@@ -242,7 +242,7 @@ exports[`t-set t-set in t-if 1`] = `
let { text, createBlock, list, multi, html, toggler, comment } = bdom;
let { isBoundary, withDefault, setContextValue } = helpers;
let block1 = createBlock(\`<div><block-child-0/><block-child-0/><block-child-0/><p><block-text-0/></p></div>\`);
let block1 = createBlock(\`<div><p><block-text-0/></p></div>\`);
return function template(ctx, node, key = \\"\\") {
ctx = Object.create(ctx);
@@ -307,7 +307,7 @@ exports[`t-set t-set outside modified in t-if 1`] = `
let { text, createBlock, list, multi, html, toggler, comment } = bdom;
let { isBoundary, withDefault, setContextValue } = helpers;
let block1 = createBlock(\`<div><block-child-0/><block-child-0/><block-child-0/><p><block-text-0/></p></div>\`);
let block1 = createBlock(\`<div><p><block-text-0/></p></div>\`);
return function template(ctx, node, key = \\"\\") {
ctx = Object.create(ctx);