diff --git a/CHANGELOG.md b/CHANGELOG.md index f5a96c0..4656b61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -83,6 +83,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - When re-rendering arrays of changing length, where the length is getting smaller or is zero — the internal bookkeeping was getting messed up. This caused runtime errors in these cases (#303). +- When re-rendering a map array with reordered elements, `connectedCallback` + and `disconnectedCallback` are no longer called because elements are only + moved in the DOM rather than being disconnected and reconnected. Added test + coverage for `connectedMoveCallback` and reference tracking during element + reordering (#254). ## [1.1.2] - 2024-12-16 diff --git a/test/test-scratch.js b/test/test-scratch.js index 4c97e41..f547559 100644 --- a/test/test-scratch.js +++ b/test/test-scratch.js @@ -103,6 +103,11 @@ class TestElement extends XElement { super.adoptedCallback(); this.adopted = true; } + + connectedMoveCallback() { + super.connectedMoveCallback(); + this.moved = true; + } } customElements.define('test-element', TestElement); @@ -241,3 +246,16 @@ it('authors can extend observed attributes', () => { el.setAttribute('custom-observed-attribute', ''); assert(el.customObservedAttributeChange); }); + +it('test connectedMoveCallback', () => { + const el = document.createElement('test-element'); + const container = document.createElement('div'); + const sibling = document.createElement('div'); + document.body.append(container); + container.append(sibling); + container.append(el); + assert(!el.moved); + container.moveBefore(el, sibling); + assert(el.moved); + container.remove(); +}); diff --git a/test/test-template-engine.js b/test/test-template-engine.js index 56b7f4d..5cac926 100644 --- a/test/test-template-engine.js +++ b/test/test-template-engine.js @@ -428,6 +428,66 @@ describe('html rendering', () => { assert(container.querySelector('#target').children[2].classList.contains('true')); }); + // This specifically tests out cases where we _cannot_ reuse DOM on shuffle. + it('renders map arrays with changing templates', () => { + const getTemplate = ({ items }) => { + return html` +
+ ${items.map(item => { + if (item.type === 'div') { + return [item.id, html`
${item.id}
`]; + } else if (item.type === 'span') { + return [item.id, html`${item.id}`]; + } else { + return [item.id, html`

${item.id}

`]; + } + })} +
+ `; + }; + const container = document.createElement('div'); + + // Render with all “div” elements. + render(container, getTemplate({ items: [ + { id: 'a', type: 'div' }, + { id: 'b', type: 'div' }, + { id: 'c', type: 'div' }, + ] })); + assert(container.querySelector('#target').childElementCount === 3); + assert(container.querySelector('#target').children[0].localName === 'div'); + assert(container.querySelector('#target').children[0].textContent === 'a'); + assert(container.querySelector('#target').children[1].localName === 'div'); + assert(container.querySelector('#target').children[1].textContent === 'b'); + assert(container.querySelector('#target').children[2].localName === 'div'); + assert(container.querySelector('#target').children[2].textContent === 'c'); + + // Change first and last to “span” elements. Different template, same order. + render(container, getTemplate({ items: [ + { id: 'a', type: 'span' }, + { id: 'b', type: 'div' }, + { id: 'c', type: 'span' }, + ] })); + assert(container.querySelector('#target').children[0].localName === 'span'); + assert(container.querySelector('#target').children[0].textContent === 'a'); + assert(container.querySelector('#target').children[1].localName === 'div'); + assert(container.querySelector('#target').children[1].textContent === 'b'); + assert(container.querySelector('#target').children[2].localName === 'span'); + assert(container.querySelector('#target').children[2].textContent === 'c'); + + // Reorder _and_ change templates. + render(container, getTemplate({ items: [ + { id: 'b', type: 'span' }, + { id: 'c', type: 'p' }, + { id: 'a', type: 'div' }, + ] })); + assert(container.querySelector('#target').children[0].localName === 'span'); + assert(container.querySelector('#target').children[0].textContent === 'b'); + assert(container.querySelector('#target').children[1].localName === 'p'); + assert(container.querySelector('#target').children[1].textContent === 'c'); + assert(container.querySelector('#target').children[2].localName === 'div'); + assert(container.querySelector('#target').children[2].textContent === 'a'); + }); + it('renders lists with changing length', () => { const getTemplate = ({ items }) => { return html` @@ -845,14 +905,13 @@ describe('html rendering', () => { assert(container.querySelector('#target').childElementCount === 2); }); - // TODO: #254: Uncomment “moves” lines when we leverage “moveBefore”. it('native map does not cause disconnectedCallback on prefix removal', () => { let connects = 0; - // let moves = 0; + let moves = 0; let disconnects = 0; class TestPrefixRemoval extends HTMLElement { connectedCallback() { connects++; } - // connectedMoveCallback() { moves++; } + connectedMoveCallback() { moves++; } disconnectedCallback() { disconnects++; } } customElements.define('test-prefix-removal', TestPrefixRemoval); @@ -871,35 +930,34 @@ describe('html rendering', () => { document.body.append(container); assert(connects === 0); - // assert(moves === 0); + assert(moves === 0); assert(disconnects === 0); render(container, getTemplate({ items: [{ id: 'foo' }, { id: 'bar' }] })); assert(connects === 2); - // assert(moves === 0); + assert(moves === 0); assert(disconnects === 0); render(container, getTemplate({ items: [{ id: 'bar' }] })); assert(connects === 2); - // assert(moves === 1); + assert(moves === 1); assert(disconnects === 1); render(container, getTemplate({ items: [] })); assert(connects === 2); - // assert(moves === 1); + assert(moves === 1); assert(disconnects === 2); container.remove(); }); - // TODO: #254: Uncomment “moves” lines when we leverage “moveBefore”. it('native map does not cause disconnectedCallback on suffix removal', () => { let connects = 0; - // let moves = 0; + let moves = 0; let disconnects = 0; class TestSuffixRemoval extends HTMLElement { connectedCallback() { connects++; } - // connectedMoveCallback() { moves++; } + connectedMoveCallback() { moves++; } disconnectedCallback() { disconnects++; } } customElements.define('test-suffix-removal', TestSuffixRemoval); @@ -918,29 +976,28 @@ describe('html rendering', () => { document.body.append(container); assert(connects === 0); - // assert(moves === 0); + assert(moves === 0); assert(disconnects === 0); render(container, getTemplate({ items: [{ id: 'foo' }, { id: 'bar' }] })); assert(connects === 2); - // assert(moves === 0); + assert(moves === 0); assert(disconnects === 0); render(container, getTemplate({ items: [{ id: 'foo' }] })); assert(connects === 2); - // assert(moves === 0); + assert(moves === 0); assert(disconnects === 1); render(container, getTemplate({ items: [] })); assert(connects === 2); - // assert(moves === 0); + assert(moves === 0); assert(disconnects === 2); container.remove(); }); - // TODO: #254: See https://chromestatus.com/feature/5135990159835136. - it.todo('native map does not cause disconnectedCallback on list shuffle', () => { + it('native map does not cause disconnectedCallback on list shuffle', () => { let connects = 0; let moves = 0; let disconnects = 0; diff --git a/types/x-element.d.ts b/types/x-element.d.ts index a864a7f..411a314 100644 --- a/types/x-element.d.ts +++ b/types/x-element.d.ts @@ -181,6 +181,10 @@ export default class XElement extends HTMLElement { * Extends HTMLElement.prototype.connectedCallback. */ connectedCallback(): void; + /** + * Extends HTMLElement.prototype.connectedMoveCallback. + */ + connectedMoveCallback(): void; /** * Extends HTMLElement.prototype.attributeChangedCallback. * @param {string} attribute diff --git a/types/x-element.d.ts.map b/types/x-element.d.ts.map index 8b0b300..1381f15 100644 --- a/types/x-element.d.ts.map +++ b/types/x-element.d.ts.map @@ -1 +1 @@ -{"version":3,"file":"x-element.d.ts","sourceRoot":"","sources":["../x-element.js"],"names":[],"mappings":"AAEA,uDAAuD;AACvD;IACE;;;OAGG;IACH,iCAFa,MAAM,EAAE,CAKpB;IAGD;;;;;;;;;;;;;;;OAeG;IACH,qBAFa,aAAa,EAAE,CAI3B;IAED;;;;;;OAMG;IAEH;;;;;;;;;;;;;OAaG;IAEH;;;;;;;;;;;;;;;;;;;;OAoBG;IACH,yBAFa;QAAC,CAAC,GAAG,EAAE,MAAM;mBA/BZ,CAAC,KAAK,GAAG,IAAI,EAAE,OAAO,EAAE,KAAK,OAAO,CAAC,GAAG,SAAS;wBACjD,MAAM;oBACN,MAAM,EAAE;sBACR,CAAC,GAAG,IAAI,EAAE,OAAO,EAAE,KAAK,OAAO;6BAXlC,WAAW,SACX,OAAO,YACP,OAAO;sBAWJ,OAAO;uBACP,OAAO;uBACP,OAAO;sBACP,OAAO,GAAG,CAAC,MAAM,OAAO,CAAC;sBACzB,OAAO,GAAG,CAAC,MAAM,OAAO,CAAC;UAsBF;KAAC,CAIrC;IAED;;;;;OAKG;IAEH;;;;;;;;;;;;;OAaG;IACH,wBAFa;QAAC,CAAC,GAAG,EAAE,MAAM,UAhBf,WAAW,SACX,KAAK,SAeoC;KAAC,CAIpD;IAED;;;;;OAKG;IACH,8BAHW,WAAW,GACT,WAAW,GAAC,UAAU,CAIlC;IAED;;;;;OAKG;IAEH;;;;;;;;;;;OAWG;IACH,sBAHW,CAAC,OAAO,EAAE,oBAAoB,EAAE,GAAG,MAAM,EAAE,OAAO,EAAE,KAAK,OAAO,gBAbhE,MAAM,QACN,WAAW,SAiBrB;IAqJD,gEA6BC;IAGD,+FAqCC;IAED,uFA2FC;IAED,gHAMC;IAGD,8FAaC;IAED,6FAQC;IAGD,uGAiBC;IAGD,+EAUC;IAGD,+EAkBC;IAGD,4EAUC;IAGD,+EAaC;IAGD,+EAsBC;IAGD,+EAYC;IAGD,oDAmDC;IAGD,mFAwBC;IAGD,oDA6CC;IAID,sDA6BC;IAGD,kDAMC;IAED,qDAEC;IAED,wDA4BC;IAGD,2DAMC;IAGD;;;MAqBC;IAED,wGAGC;IAED,mDAMC;IAED,2GAGC;IAED,sDAMC;IAED,gEAMC;IAED,iDAYC;IAGD,qDAeC;IAED,wEAcC;IAED,qEAGC;IAED,6EAUC;IAED,uFAQC;IAED,kFAQC;IAED,kFAOC;IAED,oFAoBC;IAGD,kEAEC;IAED,kDAEC;IAED,qDAEC;IAED,iEAOC;IAED,mDAKC;IAED,yDAAqC;IACrC,kDAA8B;IAC9B,+CAA8I;IAC9I,wGAA+D;IAC/D,4CAA4B;IAC5B,qDAAqF;IAl3BrF;;OAEG;IACH,0BAEC;IAQD;;;;;OAKG;IACH,oCAJW,MAAM,YACN,MAAM,GAAC,IAAI,SACX,MAAM,GAAC,IAAI,QAOrB;IAED;;OAEG;IACH,wBAAoB;IAEpB;;OAEG;IACH,6BAEC;IAED;;;;OAIG;IACH,eAYC;IAED;;;;OAIG;IAEH;;;;;;;OAOG;IACH,gBALW,WAAW,QACX,MAAM,oBAPN,KAAK,oBASL,MAAM,QAoBhB;IAED;;;;;;OAMG;IACH,kBALW,WAAW,QACX,MAAM,oBAlCN,KAAK,oBAoCL,MAAM,QAoBhB;IAED;;;OAGG;IACH,qBAFW,KAAK,QAMf;IAED;;;;;OAKG;IACH,gBAFa,MAAM,CAIlB;CA2uBF"} \ No newline at end of file +{"version":3,"file":"x-element.d.ts","sourceRoot":"","sources":["../x-element.js"],"names":[],"mappings":"AAEA,uDAAuD;AACvD;IACE;;;OAGG;IACH,iCAFa,MAAM,EAAE,CAKpB;IAGD;;;;;;;;;;;;;;;OAeG;IACH,qBAFa,aAAa,EAAE,CAI3B;IAED;;;;;;OAMG;IAEH;;;;;;;;;;;;;OAaG;IAEH;;;;;;;;;;;;;;;;;;;;OAoBG;IACH,yBAFa;QAAC,CAAC,GAAG,EAAE,MAAM;mBA/BZ,CAAC,KAAK,GAAG,IAAI,EAAE,OAAO,EAAE,KAAK,OAAO,CAAC,GAAG,SAAS;wBACjD,MAAM;oBACN,MAAM,EAAE;sBACR,CAAC,GAAG,IAAI,EAAE,OAAO,EAAE,KAAK,OAAO;6BAXlC,WAAW,SACX,OAAO,YACP,OAAO;sBAWJ,OAAO;uBACP,OAAO;uBACP,OAAO;sBACP,OAAO,GAAG,CAAC,MAAM,OAAO,CAAC;sBACzB,OAAO,GAAG,CAAC,MAAM,OAAO,CAAC;UAsBF;KAAC,CAIrC;IAED;;;;;OAKG;IAEH;;;;;;;;;;;;;OAaG;IACH,wBAFa;QAAC,CAAC,GAAG,EAAE,MAAM,UAhBf,WAAW,SACX,KAAK,SAeoC;KAAC,CAIpD;IAED;;;;;OAKG;IACH,8BAHW,WAAW,GACT,WAAW,GAAC,UAAU,CAIlC;IAED;;;;;OAKG;IAEH;;;;;;;;;;;OAWG;IACH,sBAHW,CAAC,OAAO,EAAE,oBAAoB,EAAE,GAAG,MAAM,EAAE,OAAO,EAAE,KAAK,OAAO,gBAbhE,MAAM,QACN,WAAW,SAiBrB;IAoJD,gEA6BC;IAGD,+FAqCC;IAED,uFA2FC;IAED,gHAMC;IAGD,8FAaC;IAED,6FAQC;IAGD,uGAiBC;IAGD,+EAUC;IAGD,+EAkBC;IAGD,4EAUC;IAGD,+EAaC;IAGD,+EAsBC;IAGD,+EAYC;IAGD,oDAmDC;IAGD,mFAwBC;IAGD,oDA6CC;IAID,sDA6BC;IAGD,kDAMC;IAED,qDAEC;IAED,wDA4BC;IAGD,2DAMC;IAGD;;;MAqBC;IAED,wGAGC;IAED,mDAMC;IAED,2GAGC;IAED,sDAMC;IAED,gEAMC;IAED,iDAYC;IAGD,qDAeC;IAED,wEAcC;IAED,qEAGC;IAED,6EAUC;IAED,uFAQC;IAED,kFAQC;IAED,kFAOC;IAED,oFAoBC;IAGD,kEAEC;IAED,kDAEC;IAED,qDAEC;IAED,iEAOC;IAED,mDAKC;IAED,yDAAqC;IACrC,kDAA8B;IAC9B,+CAA8I;IAC9I,wGAA+D;IAC/D,4CAA4B;IAC5B,qDAAqF;IAj3BrF;;OAEG;IACH,0BAEC;IAED;;OAEG;IACH,8BAA0B;IAE1B;;;;;OAKG;IACH,oCAJW,MAAM,YACN,MAAM,GAAC,IAAI,SACX,MAAM,GAAC,IAAI,QAOrB;IAED;;OAEG;IACH,wBAAoB;IAEpB;;OAEG;IACH,6BAEC;IAED;;;;OAIG;IACH,eAYC;IAED;;;;OAIG;IAEH;;;;;;;OAOG;IACH,gBALW,WAAW,QACX,MAAM,oBAPN,KAAK,oBASL,MAAM,QAoBhB;IAED;;;;;;OAMG;IACH,kBALW,WAAW,QACX,MAAM,oBAlCN,KAAK,oBAoCL,MAAM,QAoBhB;IAED;;;OAGG;IACH,qBAFW,KAAK,QAMf;IAED;;;;;OAKG;IACH,gBAFa,MAAM,CAIlB;CA2uBF"} \ No newline at end of file diff --git a/types/x-template.d.ts.map b/types/x-template.d.ts.map index d221478..ec7b7b0 100644 --- a/types/x-template.d.ts.map +++ b/types/x-template.d.ts.map @@ -1 +1 @@ -{"version":3,"file":"x-template.d.ts","sourceRoot":"","sources":["../x-template.js"],"names":[],"mappings":"AAiuBA,yBAA2E;AAC3E,uBAAuE"} \ No newline at end of file +{"version":3,"file":"x-template.d.ts","sourceRoot":"","sources":["../x-template.js"],"names":[],"mappings":"AA4qBA,yBAA2E;AAC3E,uBAAuE"} \ No newline at end of file diff --git a/x-element.js b/x-element.js index eb03c5f..c4c3bce 100644 --- a/x-element.js +++ b/x-element.js @@ -153,11 +153,10 @@ export default class XElement extends HTMLElement { XElement.#connectHost(this); } - // TODO: #254: Uncomment once we leverage “moveBefore”. - // /** - // * Extends HTMLElement.prototype.connectedMoveCallback. - // */ - // connectedMoveCallback() {} + /** + * Extends HTMLElement.prototype.connectedMoveCallback. + */ + connectedMoveCallback() {} /** * Extends HTMLElement.prototype.attributeChangedCallback. diff --git a/x-template.js b/x-template.js index d46f22f..ced5715 100644 --- a/x-template.js +++ b/x-template.js @@ -385,15 +385,19 @@ class TemplateEngine { arrayState.map = new Map(); } - // A mapping has already been created — we need to update the items. - const ids = new Set(); // Populated in “parseListValue”. - let index = 0; - for (const entry of entries) { + const idsToRemove = new Set(arrayState.map.keys()); + const ids = new Set(); // Populated in “parseArrayEntry”. + let reference = startNode.nextSibling; + for (let index = 0; index < entries.length; index++) { + const entry = entries[index]; const [id, rawResult] = TemplateEngine.#parseArrayEntry(entry, index, ids); let item = arrayState.map.get(id); if (item) { + idsToRemove.delete(id); if (!TemplateEngine.#canReuseDom(item.preparedResult, rawResult)) { + const referenceWasStartNode = reference === item.startNode; TemplateEngine.#recreateArrayItem(item, rawResult); + reference = referenceWasStartNode ? item.startNode : reference; } else { TemplateEngine.#update(item.preparedResult, rawResult); } @@ -401,82 +405,26 @@ class TemplateEngine { item = TemplateEngine.#createArrayItem(node, id, rawResult); arrayState.map.set(id, item); } - index++; - } - for (const [id, item] of arrayState.map.entries()) { - if (!ids.has(id)) { - TemplateEngine.#removeThrough(item.startNode, item.node); - arrayState.map.delete(id); - } - } - let lastItem; - for (const id of ids) { - const item = arrayState.map.get(id); - // TODO: We should be able to make the following code more performant. - const referenceNode = lastItem ? lastItem.node.nextSibling : startNode.nextSibling; - if (referenceNode !== item.startNode) { + if (item.startNode !== reference) { + // Move to the correct location const nodesToMove = [item.startNode]; while (nodesToMove[nodesToMove.length - 1] !== item.node) { nodesToMove.push(nodesToMove[nodesToMove.length - 1].nextSibling); } - TemplateEngine.#insertAllBefore(referenceNode.parentNode, referenceNode, nodesToMove); + TemplateEngine.#moveAllBefore(reference.parentNode, reference, nodesToMove); } - lastItem = item; + + // Move our position forward. + reference = item.node.nextSibling; } - } - // TODO: #254: Future state where the “moveBefore” API is better-supported. - // // Loops over given array of “entries” to manage an array of nodes. - // static #commitContentArrayEntries(node, startNode, entries) { - // const arrayState = TemplateEngine.#getState(startNode, TemplateEngine.#ARRAY_STATE); - // if (!arrayState.map) { - // // There is no mapping in our state — create an empty one as our base. - // TemplateEngine.#clearObject(arrayState); - // arrayState.map = new Map(); - // } - // - // const idsToRemove = new Set(arrayState.map.keys()); - // const ids = new Set(); // Populated in “parseArrayEntry”. - // let reference = startNode.nextSibling; - // for (let index = 0; index < entries.length; index++) { - // const entry = entries[index]; - // const [id, rawResult] = TemplateEngine.#parseArrayEntry(entry, index, ids); - // let item = arrayState.map.get(id); - // if (item) { - // // Update existing item. - // idsToRemove.delete(id); - // if (!TemplateEngine.#canReuseDom(item.preparedResult, rawResult)) { - // const referenceWasStartNode = reference === item.startNode; - // TemplateEngine.#recreateArrayItem(item, rawResult); - // reference = referenceWasStartNode ? item.startNode : reference; - // } else { - // TemplateEngine.#update(item.preparedResult, rawResult); - // } - // } else { - // // Create new item. - // item = TemplateEngine.#createArrayItem(node, id, rawResult); - // arrayState.map.set(id, item); - // } - // // Move to the correct location - // if (item.startNode !== reference) { - // const nodesToMove = [item.startNode]; - // while (nodesToMove[nodesToMove.length - 1] !== item.node) { - // nodesToMove.push(nodesToMove[nodesToMove.length - 1].nextSibling); - // } - // TemplateEngine.#moveAllBefore(reference.parentNode, reference, nodesToMove); - // } - // - // // Move our position forward. - // reference = item.node.nextSibling; - // } - // - // // Remove any ids which are not longer in the entries. - // for (const id of idsToRemove) { - // const item = arrayState.map.get(id); - // TemplateEngine.#removeThrough(item.startNode, item.node); - // arrayState.map.delete(id); - // } - // } + // Remove any ids which are not longer in the entries. + for (const id of idsToRemove) { + const item = arrayState.map.get(id); + TemplateEngine.#removeThrough(item.startNode, item.node); + arrayState.map.delete(id); + } + } static #commitContentFragmentValue(node, startNode, value) { const previousSibling = node.previousSibling; @@ -669,16 +617,15 @@ class TemplateEngine { return { startNode, node }; } - // TODO: #254: Future state when we leverage “moveBefore”. - // static #moveAllBefore(parentNode, referenceNode, nodes) { - // // Iterate backwards over the live node collection since we’re mutating it. - // // Note that passing “null” as the reference node moves nodes to the end. - // for (let iii = nodes.length - 1; iii >= 0; iii--) { - // const node = nodes[iii]; - // parentNode.moveBefore(node, referenceNode); - // referenceNode = node; - // } - // } + static #moveAllBefore(parentNode, referenceNode, nodes) { + // Iterate backwards over the live node collection since we’re mutating it. + // Note that passing “null” as the reference node moves nodes to the end. + for (let iii = nodes.length - 1; iii >= 0; iii--) { + const node = nodes[iii]; + parentNode.moveBefore(node, referenceNode); + referenceNode = node; + } + } static #insertAllBefore(parentNode, referenceNode, nodes) { // Iterate backwards over the live node collection since we’re mutating it.