Skip to content

Commit

Permalink
Perform rendering in a fully depth-first manner.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
theengineear committed Nov 8, 2024
1 parent fdc562a commit 1ea15e3
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 17 deletions.
35 changes: 35 additions & 0 deletions test/test-observed-properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`<test-inner .foo="${foo}"></test-inner>`;
}
}
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();
});
54 changes: 54 additions & 0 deletions test/test-template-engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`
<div id="target">
${map(items, item => item.id, item => {
return html`
<test-depth-first-outer class="outer" .item="${item.id}">
<test-depth-first-inner class="inner" .item="${item.id}">
</test-depth-first-inner>
</test-depth-first-outer>
`;
})}
</div>
`;
};
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`
Expand Down
54 changes: 37 additions & 17 deletions x-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.`);
}
Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand All @@ -1662,21 +1677,26 @@ class TemplateEngine {
return analysis;
}

static #inject(result, node, options) {
static #ready(result) {
if (result.injected) {
throw new Error(`Unexpected re-injection of template result.`);
}
result.injected = true;
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) {
Expand Down

0 comments on commit 1ea15e3

Please sign in to comment.