From 7ab34c5ca54915d88eb743a10d2da7a9db31d175 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Wed, 28 Sep 2022 09:34:30 +0200 Subject: [PATCH] [FIX] prevent crash in case with t-foreach and t-out with components The t-out directive is compiled internally into a LazyValue, which represents a value that may or may not be created sometimes in the future. It can also be reused more than once, and this is where there may be an issue: if a component is contained in the lazyvalue, it needs a unique key (coming from the t-foreach) to be properly indexed in the parent children map. However, the LazyValue does not keep the key information, so it is not able to provide it to its content. The fix is then quite clear: the LazyValue class should store the key information, and provides it to its content. This allows the LazyValue to be used multiple times, in any place in a template. closes #1270 --- src/compiler/code_generator.ts | 23 ++++++----- src/runtime/template_helpers.ts | 7 +++- .../compiler/__snapshots__/t_esc.test.ts.snap | 2 +- .../compiler/__snapshots__/t_out.test.ts.snap | 4 +- .../compiler/__snapshots__/t_set.test.ts.snap | 14 +++---- .../__snapshots__/props.test.ts.snap | 2 +- .../__snapshots__/t_out.test.ts.snap | 39 +++++++++++++++++++ .../__snapshots__/t_set.test.ts.snap | 10 ++--- tests/components/t_out.test.ts | 36 +++++++++++++++++ 9 files changed, 110 insertions(+), 27 deletions(-) create mode 100644 tests/components/__snapshots__/t_out.test.ts.snap create mode 100644 tests/components/t_out.test.ts diff --git a/src/compiler/code_generator.ts b/src/compiler/code_generator.ts index f7cef3fd..3343b405 100644 --- a/src/compiler/code_generator.ts +++ b/src/compiler/code_generator.ts @@ -214,6 +214,14 @@ class CodeTarget { result.push(`}`); return result.join("\n "); } + + currentKey(ctx: Context) { + let key = this.loopLevel ? `key${this.loopLevel}` : "key"; + if (ctx.tKeyExpr) { + key = `${ctx.tKeyExpr} + ${key}`; + } + return key; + } } const TRANSLATABLE_ATTRS = ["label", "title", "placeholder", "alt"]; @@ -365,19 +373,15 @@ export class CodeGenerator { insertBlock(expression: string, block: BlockDescription, ctx: Context): void { let blockExpr = block.generateExpr(expression); - const tKeyExpr = ctx.tKeyExpr; if (block.parentVar) { - let keyArg = `key${this.target.loopLevel}`; - if (tKeyExpr) { - keyArg = `${tKeyExpr} + ${keyArg}`; - } + let key = this.target.currentKey(ctx); this.helpers.add("withKey"); - this.addLine(`${block.parentVar}[${ctx.index}] = withKey(${blockExpr}, ${keyArg});`); + this.addLine(`${block.parentVar}[${ctx.index}] = withKey(${blockExpr}, ${key});`); return; } - if (tKeyExpr) { - blockExpr = `toggler(${tKeyExpr}, ${blockExpr})`; + if (ctx.tKeyExpr) { + blockExpr = `toggler(${ctx.tKeyExpr}, ${blockExpr})`; } if (block.isRoot && !ctx.preventRoot) { @@ -1053,7 +1057,8 @@ export class CodeGenerator { this.helpers.add("LazyValue"); const bodyAst: AST = { type: ASTType.Multi, content: ast.body }; const name = this.compileInNewTarget("value", bodyAst, ctx); - let value = `new LazyValue(${name}, ctx, this, node)`; + let key = this.target.currentKey(ctx); + let value = `new LazyValue(${name}, ctx, this, node, ${key})`; value = ast.value ? (value ? `withDefault(${expr}, ${value})` : expr) : value; this.addLine(`ctx[\`${ast.name}\`] = ${value};`); } else { diff --git a/src/runtime/template_helpers.ts b/src/runtime/template_helpers.ts index b87e6b1e..abdf14be 100644 --- a/src/runtime/template_helpers.ts +++ b/src/runtime/template_helpers.ts @@ -111,15 +111,18 @@ class LazyValue { ctx: any; component: any; node: any; - constructor(fn: any, ctx: any, component: any, node: any) { + key: any; + + constructor(fn: any, ctx: any, component: any, node: any, key: any) { this.fn = fn; this.ctx = capture(ctx); this.component = component; this.node = node; + this.key = key; } evaluate(): any { - return this.fn.call(this.component, this.ctx, this.node); + return this.fn.call(this.component, this.ctx, this.node, this.key); } toString() { diff --git a/tests/compiler/__snapshots__/t_esc.test.ts.snap b/tests/compiler/__snapshots__/t_esc.test.ts.snap index b264b276..290cc58c 100644 --- a/tests/compiler/__snapshots__/t_esc.test.ts.snap +++ b/tests/compiler/__snapshots__/t_esc.test.ts.snap @@ -126,7 +126,7 @@ exports[`t-esc t-esc is escaped 1`] = ` return function template(ctx, node, key = \\"\\") { ctx = Object.create(ctx); ctx[isBoundary] = 1 - ctx[\`var\`] = new LazyValue(value1, ctx, this, node); + ctx[\`var\`] = new LazyValue(value1, ctx, this, node, key); let txt1 = ctx['var']; return block1([txt1]); } diff --git a/tests/compiler/__snapshots__/t_out.test.ts.snap b/tests/compiler/__snapshots__/t_out.test.ts.snap index 097889fb..d06f9f1d 100644 --- a/tests/compiler/__snapshots__/t_out.test.ts.snap +++ b/tests/compiler/__snapshots__/t_out.test.ts.snap @@ -161,7 +161,7 @@ exports[`t-out t-out bdom 1`] = ` return function template(ctx, node, key = \\"\\") { ctx = Object.create(ctx); ctx[isBoundary] = 1 - ctx[\`var\`] = new LazyValue(value1, ctx, this, node); + ctx[\`var\`] = new LazyValue(value1, ctx, this, node, key); const b3 = safeOutput(ctx['var']); return block1([], [b3]); } @@ -310,7 +310,7 @@ exports[`t-out t-out switch markup on bdom 1`] = ` ctx = Object.create(ctx); ctx[isBoundary] = 1 let b3,b5; - ctx[\`bdom\`] = new LazyValue(value1, ctx, this, node); + ctx[\`bdom\`] = new LazyValue(value1, ctx, this, node, key); if (ctx['hasBdom']) { const b4 = safeOutput(ctx['bdom']); b3 = block3([], [b4]); diff --git a/tests/compiler/__snapshots__/t_set.test.ts.snap b/tests/compiler/__snapshots__/t_set.test.ts.snap index ed9b08e3..aec7b5b3 100644 --- a/tests/compiler/__snapshots__/t_set.test.ts.snap +++ b/tests/compiler/__snapshots__/t_set.test.ts.snap @@ -106,7 +106,7 @@ exports[`t-set set from body literal (with t-if/t-else 1`] = ` return function template(ctx, node, key = \\"\\") { ctx = Object.create(ctx); ctx[isBoundary] = 1 - ctx[\`value\`] = new LazyValue(value1, ctx, this, node); + ctx[\`value\`] = new LazyValue(value1, ctx, this, node, key); return text(ctx['value']); } }" @@ -142,7 +142,7 @@ exports[`t-set set from body lookup 1`] = ` return function template(ctx, node, key = \\"\\") { ctx = Object.create(ctx); ctx[isBoundary] = 1 - ctx[\`stuff\`] = new LazyValue(value1, ctx, this, node); + ctx[\`stuff\`] = new LazyValue(value1, ctx, this, node, key); let txt1 = ctx['stuff']; return block1([txt1]); } @@ -206,7 +206,7 @@ exports[`t-set t-set body is evaluated immediately 1`] = ` ctx = Object.create(ctx); ctx[isBoundary] = 1 setContextValue(ctx, \\"v1\\", 'before'); - ctx[\`v2\`] = new LazyValue(value1, ctx, this, node); + ctx[\`v2\`] = new LazyValue(value1, ctx, this, node, key); setContextValue(ctx, \\"v1\\", 'after'); const b3 = safeOutput(ctx['v2']); return block1([], [b3]); @@ -471,7 +471,7 @@ exports[`t-set t-set with content and sub t-esc 1`] = ` return function template(ctx, node, key = \\"\\") { ctx = Object.create(ctx); ctx[isBoundary] = 1 - ctx[\`setvar\`] = new LazyValue(value1, ctx, this, node); + ctx[\`setvar\`] = new LazyValue(value1, ctx, this, node, key); let txt1 = ctx['setvar']; return block1([txt1]); } @@ -497,7 +497,7 @@ exports[`t-set t-set with t-value (falsy) and body 1`] = ` ctx[isBoundary] = 1 setContextValue(ctx, \\"v3\\", false); setContextValue(ctx, \\"v1\\", 'before'); - ctx[\`v2\`] = withDefault(ctx['v3'], new LazyValue(value1, ctx, this, node)); + ctx[\`v2\`] = withDefault(ctx['v3'], new LazyValue(value1, ctx, this, node, key)); setContextValue(ctx, \\"v1\\", 'after'); setContextValue(ctx, \\"v3\\", true); const b3 = safeOutput(ctx['v2']); @@ -525,7 +525,7 @@ exports[`t-set t-set with t-value (truthy) and body 1`] = ` ctx[isBoundary] = 1 setContextValue(ctx, \\"v3\\", 'Truthy'); setContextValue(ctx, \\"v1\\", 'before'); - ctx[\`v2\`] = withDefault(ctx['v3'], new LazyValue(value1, ctx, this, node)); + ctx[\`v2\`] = withDefault(ctx['v3'], new LazyValue(value1, ctx, this, node, key)); setContextValue(ctx, \\"v1\\", 'after'); setContextValue(ctx, \\"v3\\", false); const b3 = safeOutput(ctx['v2']); @@ -638,7 +638,7 @@ exports[`t-set value priority (with non text body 1`] = ` return function template(ctx, node, key = \\"\\") { ctx = Object.create(ctx); ctx[isBoundary] = 1 - ctx[\`value\`] = withDefault(1, new LazyValue(value1, ctx, this, node)); + ctx[\`value\`] = withDefault(1, new LazyValue(value1, ctx, this, node, key)); let txt1 = ctx['value']; return block1([txt1]); } diff --git a/tests/components/__snapshots__/props.test.ts.snap b/tests/components/__snapshots__/props.test.ts.snap index ebe50251..d9fe1083 100644 --- a/tests/components/__snapshots__/props.test.ts.snap +++ b/tests/components/__snapshots__/props.test.ts.snap @@ -163,7 +163,7 @@ exports[`basics t-set with a body expression can be passed in props, and then t- return function template(ctx, node, key = \\"\\") { ctx = Object.create(ctx); ctx[isBoundary] = 1 - ctx[\`abc\`] = new LazyValue(value1, ctx, this, node); + ctx[\`abc\`] = new LazyValue(value1, ctx, this, node, key); const b3 = comp1({val: ctx['abc']}, key + \`__1\`, node, this, null); return block1([], [b3]); } diff --git a/tests/components/__snapshots__/t_out.test.ts.snap b/tests/components/__snapshots__/t_out.test.ts.snap new file mode 100644 index 00000000..164ba07f --- /dev/null +++ b/tests/components/__snapshots__/t_out.test.ts.snap @@ -0,0 +1,39 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`components in t-out simple list 1`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, comment } = bdom; + let { prepareList, isBoundary, withDefault, LazyValue, safeOutput, withKey } = helpers; + const comp1 = app.createComponent(\`Child\`, true, false, false, true); + + function value1(ctx, node, key = \\"\\") { + return comp1({}, key + \`__1\`, node, this, null); + } + + return function template(ctx, node, key = \\"\\") { + ctx = Object.create(ctx); + ctx[isBoundary] = 1 + ctx = Object.create(ctx); + const [k_block1, v_block1, l_block1, c_block1] = prepareList([1,2]);; + for (let i1 = 0; i1 < l_block1; i1++) { + ctx[\`n\`] = v_block1[i1]; + const key1 = ctx['n']; + ctx[\`blabla\`] = new LazyValue(value1, ctx, this, node, key1); + c_block1[i1] = withKey(safeOutput(ctx['blabla']), key1); + } + return list(c_block1); + } +}" +`; + +exports[`components in t-out simple list 2`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, comment } = bdom; + + return function template(ctx, node, key = \\"\\") { + return text(\`child\`); + } +}" +`; diff --git a/tests/components/__snapshots__/t_set.test.ts.snap b/tests/components/__snapshots__/t_set.test.ts.snap index b591ce2f..deeb39af 100644 --- a/tests/components/__snapshots__/t_set.test.ts.snap +++ b/tests/components/__snapshots__/t_set.test.ts.snap @@ -59,7 +59,7 @@ exports[`t-set slots with a t-set with a component in body 1`] = ` function slot1(ctx, node, key = \\"\\") { ctx = Object.create(ctx); ctx[isBoundary] = 1 - ctx[\`v\`] = new LazyValue(value1, ctx, this, node); + ctx[\`v\`] = new LazyValue(value1, ctx, this, node, key); const b3 = text(\` in slot \`); const b4 = safeOutput(ctx['v']); return multi([b3, b4]); @@ -114,7 +114,7 @@ exports[`t-set slots with an t-set with a component in body 1`] = ` function slot1(ctx, node, key = \\"\\") { ctx = Object.create(ctx); ctx[isBoundary] = 1 - ctx[\`v\`] = new LazyValue(value1, ctx, this, node); + ctx[\`v\`] = new LazyValue(value1, ctx, this, node, key); const b5 = text(\` tea \`); const b6 = safeOutput(ctx['v']); return multi([b5, b6]); @@ -169,7 +169,7 @@ exports[`t-set slots with an unused t-set with a component in body 1`] = ` function slot1(ctx, node, key = \\"\\") { ctx = Object.create(ctx); ctx[isBoundary] = 1 - ctx[\`v\`] = new LazyValue(value1, ctx, this, node); + ctx[\`v\`] = new LazyValue(value1, ctx, this, node, key); return text(\` in slot \`); } @@ -343,7 +343,7 @@ exports[`t-set t-set with a component in body 1`] = ` return function template(ctx, node, key = \\"\\") { ctx = Object.create(ctx); ctx[isBoundary] = 1 - ctx[\`v\`] = new LazyValue(value1, ctx, this, node); + ctx[\`v\`] = new LazyValue(value1, ctx, this, node, key); const b3 = safeOutput(ctx['v']); return block1([], [b3]); } @@ -377,7 +377,7 @@ exports[`t-set t-set with something in body 1`] = ` return function template(ctx, node, key = \\"\\") { ctx = Object.create(ctx); ctx[isBoundary] = 1 - ctx[\`v\`] = new LazyValue(value1, ctx, this, node); + ctx[\`v\`] = new LazyValue(value1, ctx, this, node, key); const b3 = safeOutput(ctx['v']); return block1([], [b3]); } diff --git a/tests/components/t_out.test.ts b/tests/components/t_out.test.ts new file mode 100644 index 00000000..9b4aaa7c --- /dev/null +++ b/tests/components/t_out.test.ts @@ -0,0 +1,36 @@ +import { Component, mount, xml } from "../../src/index"; +import { makeTestFixture, snapshotEverything } from "../helpers"; + +snapshotEverything(); + +// ----------------------------------------------------------------------------- +// t-out +// ----------------------------------------------------------------------------- + +describe("components in t-out", () => { + let fixture: HTMLElement; + + beforeEach(() => { + fixture = makeTestFixture(); + }); + + test("simple list", async () => { + class Child extends Component { + static template = xml`child`; + } + + class Parent extends Component { + static template = xml` + + + + + + `; + static components = { Child }; + } + + await mount(Parent, fixture); + expect(fixture.innerHTML).toBe("childchild"); + }); +});