From 1ea15e39c09e12de36f6c77eeffa6958b92d2a02 Mon Sep 17 00:00:00 2001 From: Andrew Seier Date: Thu, 7 Nov 2024 18:23:33 -0800 Subject: [PATCH] Perform rendering in a fully depth-first manner. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, we inject dom into our container and then update bindings. This causes all child “connectedCallback” calls to fire _before_ we bind property / attribute / etc. updates. That’s problematic because elements have no good way to observe initial deltas in the render tree. Said another way… We used to do this: 1. Create new nodes and inject new nodes into our container. 2. Bind value updates to new nodes. Now we do this: 1. Create new nodes. 2. Bind value updates to new nodes. 3. Inject new nodes into our container. Closes #197. --- test/test-observed-properties.js | 35 +++++++++++++++++++++ test/test-template-engine.js | 54 ++++++++++++++++++++++++++++++++ x-element.js | 54 ++++++++++++++++++++++---------- 3 files changed, 126 insertions(+), 17 deletions(-) diff --git a/test/test-observed-properties.js b/test/test-observed-properties.js index b5c4891..c583b4c 100644 --- a/test/test-observed-properties.js +++ b/test/test-observed-properties.js @@ -188,3 +188,38 @@ it('x-element observed properties', async () => { 'no re-entrance for observed, reflected properties' ); }); + +it('child properties are bound before initialization', () => { + const observations = []; + class TestInner extends XElement { + static get properties() { + return { + foo: { + type: Boolean, + observe: (host, value) => observations.push(value), + default: false, + }, + }; + } + } + customElements.define('test-inner', TestInner); + class TestOuter extends XElement { + static get properties() { + return { + foo: { + type: Boolean, + default: true, + }, + }; + } + static template(html) { + return ({ foo }) => html``; + } + } + customElements.define('test-outer', TestOuter); + const el = document.createElement('test-outer'); + document.body.append(el); + assert(observations[0] === true, observations[0]); + assert(observations.length === 1, observations); + el.remove(); +}); diff --git a/test/test-template-engine.js b/test/test-template-engine.js index 64196fe..100dd52 100644 --- a/test/test-template-engine.js +++ b/test/test-template-engine.js @@ -622,6 +622,60 @@ describe('html updaters', () => { container.remove(); }); + it('map: renders depth-first', async () => { + const updates = []; + class TestDepthFirstOuter extends HTMLElement { + #item = null; + set item(value) { updates.push(`outer-${value}`); this.#item = value; } + get item() { return this.#item; } + connectedCallback() { + // Prevent property shadowing by deleting before setting on connect. + const item = this.item ?? '???'; + Reflect.deleteProperty(this, 'item'); + Reflect.set(this, 'item', item); + } + } + customElements.define('test-depth-first-outer', TestDepthFirstOuter); + class TestDepthFirstInner extends HTMLElement { + #item = null; + set item(value) { updates.push(`inner-${value}`); this.#item = value; } + get item() { return this.#item; } + connectedCallback() { + // Prevent property shadowing by deleting before setting on connect. + const item = this.item ?? '???'; + Reflect.deleteProperty(this, 'item'); + Reflect.set(this, 'item', item); + } + } + customElements.define('test-depth-first-inner', TestDepthFirstInner); + + const getTemplate = ({ items }) => { + return html` +
+ ${map(items, item => item.id, item => { + return html` + + + + + `; + })} +
+ `; + }; + const container = document.createElement('div'); + document.body.append(container); + const items = [{ id: 'foo' }, { id: 'bar'}]; + render(container, getTemplate({ items })); + await Promise.resolve(); + assert(updates[0] === 'outer-foo', updates[0]); + assert(updates[1] === 'inner-foo', updates[1]); + assert(updates[2] === 'outer-bar', updates[2]); + assert(updates[3] === 'inner-bar', updates[3]); + assert(updates.length === 4, updates); + container.remove(); + }); + it('map: re-renders each time', () => { const getTemplate = ({ items, lookup }) => { return html` diff --git a/x-element.js b/x-element.js index c9cb247..ae4bd19 100644 --- a/x-element.js +++ b/x-element.js @@ -1074,12 +1074,14 @@ class TemplateEngine { const result = TemplateEngine.#resultMap.get(resultReference); if (TemplateEngine.#cannotReuseResult(state.result, result)) { TemplateEngine.#removeWithin(container); - state.result = result; + TemplateEngine.#ready(result); + TemplateEngine.#commit(result); TemplateEngine.#inject(result, container); + state.result = result; } else { TemplateEngine.#assign(state.result, result); + TemplateEngine.#commit(state.result); } - TemplateEngine.#commit(state.result); } else { TemplateEngine.#clearObject(state); TemplateEngine.#removeWithin(container); @@ -1464,12 +1466,14 @@ class TemplateEngine { if (TemplateEngine.#cannotReuseResult(state.result, result)) { TemplateEngine.#removeBetween(startNode, node); TemplateEngine.#clearObject(state); - state.result = result; + TemplateEngine.#ready(result); + TemplateEngine.#commit(result); TemplateEngine.#inject(result, node, { before: true }); + state.result = result; } else { TemplateEngine.#assign(state.result, result); + TemplateEngine.#commit(state.result); } - TemplateEngine.#commit(state.result); } else if (Array.isArray(value)) { TemplateEngine.#mapInner(state, node, startNode, null, null, value, 'array'); } else { @@ -1566,8 +1570,11 @@ class TemplateEngine { const result = TemplateEngine.#resultMap.get(reference); if (result) { const id = identify ? identify(input, index) : String(index); - state.map.set(id, { id, ...TemplateEngine.#createItem(result, node) }); + const cursors = TemplateEngine.#createCursors(node); + TemplateEngine.#ready(result); TemplateEngine.#commit(result); + TemplateEngine.#inject(result, cursors.node, { before: true }); + state.map.set(id, { id, result, ...cursors }); } else { throw new Error(`Unexpected ${name} value "${reference}" provided by callback.`); } @@ -1585,14 +1592,23 @@ class TemplateEngine { if (state.map.has(id)) { const item = state.map.get(id); if (TemplateEngine.#cannotReuseResult(item.result, result)) { - const itemClone = { ...item }; - Object.assign(item, TemplateEngine.#createItem(result, itemClone.startNode)); - TemplateEngine.#removeThrough(itemClone.startNode, itemClone.node); + // Add new comment cursors before removing old comment cursors. + const cursors = TemplateEngine.#createCursors(item.startNode); + TemplateEngine.#removeThrough(item.startNode, item.node); + TemplateEngine.#ready(result); + TemplateEngine.#commit(result); + TemplateEngine.#inject(result, cursors.node, { before: true }); + Object.assign(item, { result, ...cursors }); } else { TemplateEngine.#assign(item.result, result); + TemplateEngine.#commit(item.result); } } else { - const item = { id, ...TemplateEngine.#createItem(result, node) }; + const cursors = TemplateEngine.#createCursors(node); + TemplateEngine.#ready(result); + TemplateEngine.#commit(result); + TemplateEngine.#inject(result, cursors.node, { before: true }); + const item = { id, result, ...cursors }; state.map.set(id, item); } const item = state.map.get(id); @@ -1640,13 +1656,12 @@ class TemplateEngine { return { type, strings, values, lastValues, injected }; } - static #createItem(result, referenceNode) { + static #createCursors(referenceNode) { const startNode = document.createComment(''); const node = document.createComment(''); referenceNode.parentNode.insertBefore(startNode, referenceNode); - TemplateEngine.#inject(result, referenceNode, { before: true }); referenceNode.parentNode.insertBefore(node, referenceNode); - return { result, startNode, node }; + return { startNode, node }; } static #getAnalysis(result) { @@ -1662,7 +1677,7 @@ class TemplateEngine { return analysis; } - static #inject(result, node, options) { + static #ready(result) { if (result.injected) { throw new Error(`Unexpected re-injection of template result.`); } @@ -1670,13 +1685,18 @@ class TemplateEngine { const { initialElement, initialDirections } = TemplateEngine.#getAnalysis(result); const element = initialElement.cloneNode(true); result.directions = TemplateEngine.#getFinalDirections(initialDirections, element.content); + result.element = element; + } + + static #inject(result, node, options) { options?.before ? result.type === 'svg' - ? TemplateEngine.#insertAllBefore(element.content.firstChild.childNodes, node) - : TemplateEngine.#insertAllBefore(element.content.childNodes, node) + ? TemplateEngine.#insertAllBefore(result.element.content.firstChild.childNodes, node) + : TemplateEngine.#insertAllBefore(result.element.content.childNodes, node) : result.type === 'svg' - ? node.append(...element.content.firstChild.childNodes) - : node.append(element.content); + ? node.append(...result.element.content.firstChild.childNodes) + : node.append(result.element.content); + result.element = null; } static #assign(result, newResult) {