diff --git a/src/runtime/blockdom/block_compiler.ts b/src/runtime/blockdom/block_compiler.ts index 0730ee8a..b04fc804 100644 --- a/src/runtime/blockdom/block_compiler.ts +++ b/src/runtime/blockdom/block_compiler.ts @@ -517,9 +517,12 @@ function createBlockClass(template: HTMLElement, ctx: BlockCtx): BlockClass { return this.el!; } - moveBefore(other: Block | null, afterNode: Node | null) { - const target = other ? other.el! : afterNode; - nodeInsertBefore.call(this.parentEl, this.el!, target); + moveBeforeDOMNode(node: Node | null) { + nodeInsertBefore.call(this.parentEl, this.el!, node); + } + + moveBeforeVNode(other: Block | null, afterNode: Node | null) { + nodeInsertBefore.call(this.parentEl, this.el!, other ? other.el! : afterNode); } toString() { diff --git a/src/runtime/blockdom/event_catcher.ts b/src/runtime/blockdom/event_catcher.ts index c3cc0b2b..7bd4da58 100644 --- a/src/runtime/blockdom/event_catcher.ts +++ b/src/runtime/blockdom/event_catcher.ts @@ -56,8 +56,17 @@ export function createCatcher(eventsSpec: EventsSpec): Catcher { } } - moveBefore(other: VCatcher | null, afterNode: Node | null) { - this.child.moveBefore(other ? other.child : null, afterNode); + moveBeforeDOMNode(node: Node | null) { + this.child.moveBeforeDOMNode(node); + this.parentEl!.insertBefore(this.afterNode!, node); + } + + moveBeforeVNode(other: VCatcher | null, afterNode: Node | null) { + if (other) { + // check this with @ged-odoo for use in foreach + afterNode = other.firstNode() || afterNode; + } + this.child.moveBeforeVNode(other ? other.child : null, afterNode); this.parentEl!.insertBefore(this.afterNode!, afterNode); } diff --git a/src/runtime/blockdom/html.ts b/src/runtime/blockdom/html.ts index 0c01eccf..fe88fa40 100644 --- a/src/runtime/blockdom/html.ts +++ b/src/runtime/blockdom/html.ts @@ -29,14 +29,18 @@ class VHtml { } } - moveBefore(other: VHtml | null, afterNode: Node | null) { - const target = other ? other.content[0] : afterNode; + moveBeforeDOMNode(node: Node | null) { const parent = this.parentEl; for (let elem of this.content) { - nodeInsertBefore.call(parent, elem, target); + nodeInsertBefore.call(parent, elem, node); } } + moveBeforeVNode(other: VHtml | null, afterNode: Node | null) { + const target = other ? other.content[0] : afterNode; + this.moveBeforeDOMNode(target); + } + patch(other: VHtml) { if (this === other) { return; diff --git a/src/runtime/blockdom/index.ts b/src/runtime/blockdom/index.ts index 0bc65806..165fa5d0 100644 --- a/src/runtime/blockdom/index.ts +++ b/src/runtime/blockdom/index.ts @@ -10,7 +10,8 @@ export { createCatcher } from "./event_catcher"; export interface VNode { mount(parent: HTMLElement, afterNode: Node | null): void; - moveBefore(other: T | null, afterNode: Node | null): void; + moveBeforeDOMNode(node: Node | null): void; + moveBeforeVNode(other: T | null, afterNode: Node | null): void; patch(other: T, withBeforeRemove: boolean): void; beforeRemove(): void; remove(): void; diff --git a/src/runtime/blockdom/list.ts b/src/runtime/blockdom/list.ts index 08fd5749..0b7c257a 100644 --- a/src/runtime/blockdom/list.ts +++ b/src/runtime/blockdom/list.ts @@ -38,14 +38,22 @@ class VList { this.parentEl = parent; } - moveBefore(other: VList | null, afterNode: Node | null) { + moveBeforeDOMNode(node: Node | null) { + const children = this.children; + for (let i = 0, l = children.length; i < l; i++) { + children[i].moveBeforeDOMNode(node); + } + this.parentEl!.insertBefore(this.anchor!, node); + } + + moveBeforeVNode(other: VList | null, afterNode: Node | null) { if (other) { const next = other!.children[0]; afterNode = (next ? next.firstNode() : other!.anchor) || null; } const children = this.children; for (let i = 0, l = children.length; i < l; i++) { - children[i].moveBefore(null, afterNode); + children[i].moveBeforeVNode(null, afterNode); } this.parentEl!.insertBefore(this.anchor!, afterNode); } @@ -66,7 +74,7 @@ class VList { patch: cPatch, remove: cRemove, beforeRemove, - moveBefore: cMoveBefore, + moveBeforeVNode: cMoveBefore, firstNode: cFirstNode, } = proto; diff --git a/src/runtime/blockdom/multi.ts b/src/runtime/blockdom/multi.ts index c9278208..b375020f 100644 --- a/src/runtime/blockdom/multi.ts +++ b/src/runtime/blockdom/multi.ts @@ -38,7 +38,22 @@ export class VMulti { this.parentEl = parent; } - moveBefore(other: VMulti | null, afterNode: Node | null) { + moveBeforeDOMNode(node: Node | null) { + const children = this.children; + const parent = this.parentEl; + const anchors = this.anchors; + for (let i = 0, l = children.length; i < l; i++) { + let child = children[i]; + if (child) { + child.moveBeforeDOMNode(node); + } else { + const anchor = anchors![i]; + nodeInsertBefore.call(parent, anchor, node); + } + } + } + + moveBeforeVNode(other: VMulti | null, afterNode: Node | null) { if (other) { const next = other!.children[0]; afterNode = (next ? next.firstNode() : other!.anchors![0]) || null; @@ -49,7 +64,7 @@ export class VMulti { for (let i = 0, l = children.length; i < l; i++) { let child = children[i]; if (child) { - child.moveBefore(null, afterNode); + child.moveBeforeVNode(null, afterNode); } else { const anchor = anchors![i]; nodeInsertBefore.call(parent, anchor, afterNode); diff --git a/src/runtime/blockdom/text.ts b/src/runtime/blockdom/text.ts index 25da7baf..687eb21e 100644 --- a/src/runtime/blockdom/text.ts +++ b/src/runtime/blockdom/text.ts @@ -23,9 +23,12 @@ abstract class VSimpleNode { this.el = node; } - moveBefore(other: VText | null, afterNode: Node | null) { - const target = other ? other.el! : afterNode; - nodeInsertBefore.call(this.parentEl, this.el!, target); + moveBeforeDOMNode(node: Node | null) { + nodeInsertBefore.call(this.parentEl, this.el!, node); + } + + moveBeforeVNode(other: VText | null, afterNode: Node | null) { + nodeInsertBefore.call(this.parentEl, this.el!, other ? other.el! : afterNode); } beforeRemove() {} diff --git a/src/runtime/blockdom/toggler.ts b/src/runtime/blockdom/toggler.ts index d31c84ad..a734813f 100644 --- a/src/runtime/blockdom/toggler.ts +++ b/src/runtime/blockdom/toggler.ts @@ -20,8 +20,12 @@ class VToggler { this.child.mount(parent, afterNode); } - moveBefore(other: VToggler | null, afterNode: Node | null) { - this.child.moveBefore(other ? other.child : null, afterNode); + moveBeforeDOMNode(node: Node | null) { + this.child.moveBeforeDOMNode(node); + } + + moveBeforeVNode(other: VToggler | null, afterNode: Node | null) { + this.moveBeforeDOMNode((other && other.firstNode()) || afterNode); } patch(other: VToggler, withBeforeRemove: boolean) { diff --git a/src/runtime/component_node.ts b/src/runtime/component_node.ts index e6636001..f27867fc 100644 --- a/src/runtime/component_node.ts +++ b/src/runtime/component_node.ts @@ -306,8 +306,12 @@ export class ComponentNode

implements VNode | null, afterNode: Node | null) { + this.bdom!.moveBeforeVNode(other ? other.bdom : null, afterNode); } patch() { diff --git a/src/runtime/portal.ts b/src/runtime/portal.ts index e06b1009..f85f3d62 100644 --- a/src/runtime/portal.ts +++ b/src/runtime/portal.ts @@ -75,7 +75,7 @@ export class Portal extends Component { if (!portal.target) { const target: HTMLElement = document.querySelector(this.props.target); if (target) { - portal.content!.moveBefore(target, null); + portal.content!.moveBeforeVNode(target, null); } else { throw new OwlError("invalid portal target"); } diff --git a/tests/components/__snapshots__/t_foreach.test.ts.snap b/tests/components/__snapshots__/t_foreach.test.ts.snap index 665eeaec..1799a234 100644 --- a/tests/components/__snapshots__/t_foreach.test.ts.snap +++ b/tests/components/__snapshots__/t_foreach.test.ts.snap @@ -149,6 +149,58 @@ exports[`list of components list of sub components inside other nodes 2`] = ` }" `; +exports[`list of components order is correct when slots are not of same type 1`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, comment } = bdom; + let { capture, markRaw } = helpers; + const comp1 = app.createComponent(\`Child\`, true, true, false, true); + + let block2 = createBlock(\`

A
\`); + + function slot1(ctx, node, key = \\"\\") { + let b2; + if (!ctx['state'].active) { + b2 = block2(); + } + return multi([b2]); + } + + function slot2(ctx, node, key = \\"\\") { + return text(\`B\`); + } + + function slot3(ctx, node, key = \\"\\") { + return text(\`C\`); + } + + return function template(ctx, node, key = \\"\\") { + const ctx1 = capture(ctx); + return comp1({slots: markRaw({'a': {__render: slot1, __ctx: ctx1, active: !ctx['state'].active}, 'b': {__render: slot2, __ctx: ctx1, active: true}, 'c': {__render: slot3, __ctx: ctx1, active: ctx['state'].active}})}, key + \`__1\`, node, this, null); + } +}" +`; + +exports[`list of components order is correct when slots are not of same type 2`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, comment } = bdom; + let { prepareList, callSlot, withKey } = helpers; + + return function template(ctx, node, key = \\"\\") { + ctx = Object.create(ctx); + const [k_block1, v_block1, l_block1, c_block1] = prepareList(ctx['slotNames']);; + for (let i1 = 0; i1 < l_block1; i1++) { + ctx[\`slotName\`] = v_block1[i1]; + const key1 = ctx['slotName']; + const slot1 = (ctx['slotName']); + c_block1[i1] = withKey(toggler(slot1, callSlot(ctx, node, key1 + \`__1__\${key1}\`, slot1, true, {})), key1); + } + return list(c_block1); + } +}" +`; + exports[`list of components reconciliation alg works for t-foreach in t-foreach 1`] = ` "function anonymous(app, bdom, helpers ) { diff --git a/tests/components/t_foreach.test.ts b/tests/components/t_foreach.test.ts index f809fc97..4c783b66 100644 --- a/tests/components/t_foreach.test.ts +++ b/tests/components/t_foreach.test.ts @@ -359,4 +359,35 @@ describe("list of components", () => { console.info = consoleInfo; expect(mockConsoleWarn).toBeCalledTimes(1); }); + + test("order is correct when slots are not of same type", async () => { + class Child extends Component { + static template = xml` + + `; + get slotNames() { + return Object.entries(this.props.slots) + .filter((entry: any) => entry[1].active) + .map((entry) => entry[0]); + } + } + + class Parent extends Component { + static template = xml` + +
A
+ B + C +
+ `; + static components = { Child }; + state = useState({ active: false }); + } + + const parent = await mount(Parent, fixture); + expect(fixture.textContent).toBe("AB"); + parent.state.active = true; + await nextTick(); + expect(fixture.textContent).toBe("BC"); + }); });