Skip to content

Perform rendering in a fully depth-first manner. #198

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The above three lines sum up the change neatly. Before, we readied / injected in one line… then committed the update. Now, we ready, commit, and lastly inject.

Yes, in the future we will change all these internal method names… not now though.

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
Loading