diff --git a/src/idiomorph.js b/src/idiomorph.js index bfa9784..f045b70 100644 --- a/src/idiomorph.js +++ b/src/idiomorph.js @@ -290,6 +290,14 @@ var Idiomorph = (function () { // @ts-ignore ditto newParent = newParent.content; } + + // find the last node with an id for better match lookup later + let lastIdNode = endPoint ? endPoint.previousSibling : oldParent.lastChild; + while (lastIdNode && !ctx.idMap.has(lastIdNode) && lastIdNode != insertionPoint) { + lastIdNode = lastIdNode.previousSibling + } + const matchInfo = {lastIdNode, futureBestMatch: null}; + insertionPoint ||= oldParent.firstChild; // run through all the new content @@ -301,6 +309,7 @@ var Idiomorph = (function () { newChild, insertionPoint, endPoint, + matchInfo, ); if (bestMatch) { // if the node to morph is not at the insertion point then remove/move up to it @@ -372,20 +381,36 @@ var Idiomorph = (function () { const findBestMatch = (function () { /** * Scans forward from the startPoint to the endPoint looking for a match - * for the node. It looks for an id set match first, then a soft match. - * We abort softmatching if we find two future soft matches, to reduce churn. + * for the node. First checks if we are at the last node with id to find + * the very best match and then it looks for an id set match, then a soft match. + * Stops searching if there are better id or soft matches we would displace. * @param {Node} node * @param {MorphContext} ctx - * @param {Node | null} startPoint + * @param {Node} startPoint * @param {Node | null} endPoint + * @param {{lastIdNode: Node | null, futureBestMatch: Node | null}} matchInfo * @returns {Node | null} */ - function findBestMatch(ctx, node, startPoint, endPoint) { + function findBestMatch(ctx, node, startPoint, endPoint, matchInfo) { + // when we get to the last old node with id find the best remaining match for it + if (startPoint == matchInfo.lastIdNode) { + matchInfo.futureBestMatch ||= findBestNodeMatch(startPoint, node, ctx); + if (matchInfo.futureBestMatch && matchInfo.futureBestMatch != node) { + matchInfo.futureBestMatch = null; + // there is a better match later so let this node get inserted instead + return null; + } + } + let softMatch = null; let nextSibling = node.nextSibling; let siblingSoftMatchCount = 0; + let displaceMatchCount = 0; + + // max id matches we are willing to displace in our search + const nodeMatchCount = ctx.idMap.get(node)?.size || 0; - let cursor = startPoint; + let cursor = /** @type {Node | null} */ (startPoint); while (cursor && cursor != endPoint) { // soft matching is a prerequisite for id set matching if (isSoftMatch(cursor, node)) { @@ -397,11 +422,24 @@ var Idiomorph = (function () { if (softMatch === null) { // the current soft match will hard match something else in the future, leave it if (!ctx.idMap.has(cursor)) { - // save this as the fallback if we get through the loop without finding a hard match - softMatch = cursor; + // optimization: if node can't id set match, we can just return the soft match immediately + if (!nodeMatchCount) { + return cursor; + } else { + // save this as the fallback if we get through the loop without finding a hard match + softMatch = cursor; + } } } } + // check for ids we may be displaced when matching + displaceMatchCount += ctx.idMap.get(cursor)?.size || 0; + if (displaceMatchCount > nodeMatchCount) { + // if we are going to displace more ids than the node contains then + // we do not have a good candidate for an id match, so return + break; + } + if ( softMatch === null && nextSibling && @@ -472,6 +510,42 @@ var Idiomorph = (function () { ); } + /** + * + * @param {Node} oldNode + * @param {Node} newChild + * @param {MorphContext} ctx + * @returns {Node | null} + */ + function findBestNodeMatch(oldNode, newChild, ctx) { + let currentNode = /** @type {Node | null} */ (newChild); + let bestNode = newChild; + let score = 0; + while (currentNode) { + let newScore = scoreElement(oldNode, currentNode, ctx); + if (newScore > score) { + bestNode = currentNode; + score = newScore; + } + currentNode = currentNode.nextSibling; + } + return bestNode; + } + + /** + * + * @param {Node} oldNode + * @param {Node} newNode + * @param {MorphContext} ctx + * @returns {number} + */ + function scoreElement(oldNode, newNode, ctx) { + if (isSoftMatch(oldNode, newNode)) { + return 0.5 + (ctx.idMap.get(newNode)?.size || 0); + } + return 0; + } + return findBestMatch; })(); @@ -518,6 +592,26 @@ var Idiomorph = (function () { return cursor; } + /** + * @param {Node | null} node - node being removed from consideration + * @param {string} id - The ID of the element to be moved. + * @param {MorphContext} ctx + */ + function removeIdFromMap(node, id, ctx) { + while (node) { + let idSet = ctx.idMap.get(node); + if(idSet) { + idSet?.delete(id); + if (idSet.size == 0) { + ctx.idMap.delete(node) + } else { + ctx.idMap.set(node,idSet); + } + } + node = node.parentNode; + } + } + /** * Search for an element by id within the document and pantry, and move it using moveBefore. * @@ -535,6 +629,8 @@ var Idiomorph = (function () { ctx.target.querySelector(`#${id}`) || ctx.pantry.querySelector(`#${id}`) ); + // prevent this now relocated id from triggering pantry storage on remove + removeIdFromMap(target, id, ctx); moveBefore(parentNode, target, after); return target; } diff --git a/test/hooks.js b/test/hooks.js index dbc289d..21a1dd5 100644 --- a/test/hooks.js +++ b/test/hooks.js @@ -121,6 +121,16 @@ describe("lifecycle hooks", function () { initial.outerHTML.should.equal(""); }); + it("returning false to beforeNodeRemoved prevents removing the node with removed elemnt next to relocated id element", function () { + let initial = make(`
A
B
`); + Idiomorph.morph(initial, `
AB
`, { + callbacks: { + beforeNodeRemoved: (node) => false, + }, + }); + initial.outerHTML.should.equal(`
AB
`); + }); + it("returning false to beforeNodeRemoved prevents removing the node with different tag types", function () { let initial = make("
ABC
"); Idiomorph.morph(initial, "
B
", { diff --git a/test/lib/utilities.js b/test/lib/utilities.js index 5352b4e..97de8ec 100644 --- a/test/lib/utilities.js +++ b/test/lib/utilities.js @@ -90,7 +90,7 @@ function assertNoFocus() { document.activeElement.tagName.should.eql("BODY"); } -function assertOps(before, after, expectedOps) { +function assertOps(before, after, expectedOps, singleNode = true) { let ops = []; let initial = make(before); let final = make(after); @@ -121,7 +121,7 @@ function assertOps(before, after, expectedOps) { console.log('test failing Operations is:'); console.log(ops); } - initial.outerHTML.should.equal(finalCopy.outerHTML); + if(singleNode) initial.outerHTML.should.equal(finalCopy.outerHTML); ops.should.eql(expectedOps); } diff --git a/test/ops.js b/test/ops.js index 1a1422d..ee1aa52 100644 --- a/test/ops.js +++ b/test/ops.js @@ -188,4 +188,199 @@ describe("morphing operations", function () { ], ); }); + + it("findBestMatch rejects morphing node that would lose more IDs", function () { + // here the findBestMatch function when it finds a node with id's it will track how many + // id matches in this node and then as it searches for a matching node it will track + // how many id's in the content it would have to remove before it finds a match + // if it finds more ids are going to match in-between nodes it aborts matching to + // allow better matching with less dom updates. + assertOps( + `
` + + `` + + `` + + `` + + `
`, + + `
` + + `` + + `` + + `` + + `
`, + [ + [ + "Morphed", + '
', + '
', + ], + ["Morphed", "", ""], + ["Morphed", '', ''], + ["Added", ""], + ["Morphed", '', ''], + ["Morphed", "", ""], + ["Morphed", '', ''], + ["Removed", ""], + ], + ); + }); + + it("show bestMatch routine can match the best old node for morphing", function () { + // the findBestMatch can detect when there is a single node with id's remaining and instead + // of matching the first node it will instead find the node with the most mathing id's so + // it can retain the most state possible by displacing the least nodes with id's and their siblings. + assertOps( + `
`+ + ``+ + `

i will get lost

`+ + ``+ + `

i will get saved

`+ + ``+ + `

i will get saved

`+ + `
`, + + `
`+ + ``+ + `

i will get lost

`+ + `
`+ + `
`+ + ``+ + `

i will get saved

`+ + ``+ + `

i will get saved

`+ + `
`, + [ + [ + 'Added', + '

i will get lost

' + ], + [ + 'Morphed', + '
', + '

i will get lost

' + ], + [ + 'Morphed', + '', + '' + ], + [ 'Added', '

i will get lost

' ], + [ + 'Morphed', + '

i will get lost

i will get saved

i will get saved

', + '

i will get saved

i will get saved

' + ], + [ 'Removed', '

i will get lost

' ], + [ + 'Morphed', + '', + '' + ], + [ 'Morphed', '

i will get saved

', '

i will get saved

' ], + [ + 'Morphed', + '', + '' + ], + [ 'Morphed', '

i will get saved

', '

i will get saved

' ] + ], + false, + ); + }); + + it("show bestMatch routine can match the best old node for morphing with deeper content", function () { + // the new bestMatch also handles checking on inner children scans for the best last node with ids + // to match with while before it was only run on the top node + assertOps( + `
`+ + `
`+ + ``+ + `

i will get lost

`+ + ``+ + `

i will get saved

`+ + ``+ + `

i will get saved

`+ + `
`+ + `
`, + + `
`+ + `
`+ + ``+ + `

i will get lost

`+ + `
`+ + `
`+ + ``+ + `

i will get saved

`+ + ``+ + `

i will get saved

`+ + `
`+ + `
`, + [ + [ + 'Morphed', + '

i will get lost

i will get saved

i will get saved

', + '

i will get lost

i will get saved

i will get saved

' + ], + [ + 'Added', + '

i will get lost

' + ], + [ + 'Morphed', + '
', + '

i will get lost

' + ], + [ + 'Morphed', + '', + '' + ], + [ 'Added', '

i will get lost

' ], + [ + 'Morphed', + '

i will get lost

i will get saved

i will get saved

', + '

i will get saved

i will get saved

' + ], + [ 'Removed', '

i will get lost

' ], + [ + 'Morphed', + '', + '' + ], + [ 'Morphed', '

i will get saved

', '

i will get saved

' ], + [ + 'Morphed', + '', + '' + ], + [ 'Morphed', '

i will get saved

', '

i will get saved

' ] + ], + ); + }); + + it("show bestMatch routine can match the best old node for morphing and adding dummy div before", function () { + assertOps( + `
`+ + ``+ + `
`, + + `
`+ + `
`+ + ``+ + `
`, + [ + [ 'Added', '
' ], + [ + 'Morphed', + '
', + '
' + ], + [ + 'Morphed', + '', + '' + ] + ], + false, + ); + }); }); diff --git a/test/preserve-focus.js b/test/preserve-focus.js index 4381f24..17394b7 100644 --- a/test/preserve-focus.js +++ b/test/preserve-focus.js @@ -232,13 +232,7 @@ describe("Preserves focus where possible", function () { "b", false, ); - if (hasMoveBefore()) { - assertFocus("focused"); - // TODO moveBefore loses selection on Chrome 131.0.6778.264 - // expect will be fixed in future release - // assertFocusAndSelection("focused", "b"); - } else { - assertNoFocus(); - } + + assertFocus("focused"); }); });