From 14e22c428ea0453b530eb61f3ab1036b0727db26 Mon Sep 17 00:00:00 2001 From: MichaelWest22 Date: Fri, 31 Jan 2025 11:25:46 +1300 Subject: [PATCH 1/5] Improve id set matching and preserved hook handling when pantried --- src/idiomorph.js | 47 ++++++++++++++++++++++++++++++++++++++---- test/hooks.js | 10 +++++++++ test/ops.js | 35 +++++++++++++++++++++++++++++++ test/preserve-focus.js | 10 ++------- 4 files changed, 90 insertions(+), 12 deletions(-) diff --git a/src/idiomorph.js b/src/idiomorph.js index bfa9784..81a54c1 100644 --- a/src/idiomorph.js +++ b/src/idiomorph.js @@ -382,8 +382,12 @@ var Idiomorph = (function () { */ function findBestMatch(ctx, node, startPoint, endPoint) { let softMatch = null; - let nextSibling = node.nextSibling; + let nextSibling = node.nextSibling; let siblingSoftMatchCount = 0; + let discardMatchCount = 0; + + // max id matches we are willing to discard in our search + const nodeMatchCount = ctx.idMap.get(node)?.size || 0; let cursor = startPoint; while (cursor && cursor != endPoint) { @@ -397,11 +401,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 (!ctx.idMap.has(node)) { + 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 discarding when matching + discardMatchCount += ctx.idMap.get(cursor)?.size || 0; + if (discardMatchCount > nodeMatchCount) { + // if we are going to discard 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 && @@ -411,7 +428,7 @@ var Idiomorph = (function () { // increment the count of future soft matches siblingSoftMatchCount++; nextSibling = nextSibling.nextSibling; - + // If there are two future soft matches, block soft matching for this node to allow // future siblings to soft match. This is to reduce churn in the DOM when an element // is prepended. @@ -518,6 +535,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 +572,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/ops.js b/test/ops.js index 1a1422d..e5fc4ce 100644 --- a/test/ops.js +++ b/test/ops.js @@ -188,4 +188,39 @@ 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", ""], + ], + ); + }); }); 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"); }); }); From d572eda1de04cf1aa9a666db07703d79df6038c0 Mon Sep 17 00:00:00 2001 From: MichaelWest22 Date: Fri, 31 Jan 2025 11:34:54 +1300 Subject: [PATCH 2/5] whitespace fix and softmatch code opimization --- src/idiomorph.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/idiomorph.js b/src/idiomorph.js index 81a54c1..4d5a496 100644 --- a/src/idiomorph.js +++ b/src/idiomorph.js @@ -382,7 +382,7 @@ var Idiomorph = (function () { */ function findBestMatch(ctx, node, startPoint, endPoint) { let softMatch = null; - let nextSibling = node.nextSibling; + let nextSibling = node.nextSibling; let siblingSoftMatchCount = 0; let discardMatchCount = 0; @@ -402,7 +402,7 @@ var Idiomorph = (function () { // the current soft match will hard match something else in the future, leave it if (!ctx.idMap.has(cursor)) { // optimization: if node can't id set match, we can just return the soft match immediately - if (!ctx.idMap.has(node)) { + if (!nodeMatchCount) { return cursor; } else { // save this as the fallback if we get through the loop without finding a hard match @@ -428,7 +428,7 @@ var Idiomorph = (function () { // increment the count of future soft matches siblingSoftMatchCount++; nextSibling = nextSibling.nextSibling; - + // If there are two future soft matches, block soft matching for this node to allow // future siblings to soft match. This is to reduce churn in the DOM when an element // is prepended. From 515cc9ced13e657c9e3199302031a9a09be6d503 Mon Sep 17 00:00:00 2001 From: MichaelWest22 Date: Fri, 31 Jan 2025 11:41:50 +1300 Subject: [PATCH 3/5] rename discard to displace --- src/idiomorph.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/idiomorph.js b/src/idiomorph.js index 4d5a496..ad601a9 100644 --- a/src/idiomorph.js +++ b/src/idiomorph.js @@ -384,9 +384,9 @@ var Idiomorph = (function () { let softMatch = null; let nextSibling = node.nextSibling; let siblingSoftMatchCount = 0; - let discardMatchCount = 0; + let displaceMatchCount = 0; - // max id matches we are willing to discard in our search + // max id matches we are willing to displace in our search const nodeMatchCount = ctx.idMap.get(node)?.size || 0; let cursor = startPoint; @@ -411,10 +411,10 @@ var Idiomorph = (function () { } } } - // check for ids we may be discarding when matching - discardMatchCount += ctx.idMap.get(cursor)?.size || 0; - if (discardMatchCount > nodeMatchCount) { - // if we are going to discard more ids than the node contains then + // 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; } From b82c8d69485673bfdff63b411eaee08988b5a26b Mon Sep 17 00:00:00 2001 From: MichaelWest22 Date: Fri, 31 Jan 2025 12:30:08 +1300 Subject: [PATCH 4/5] re-implment the old bestmatch feature --- src/idiomorph.js | 71 ++++++++++++++++-- test/lib/utilities.js | 4 +- test/ops.js | 162 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 227 insertions(+), 10 deletions(-) diff --git a/src/idiomorph.js b/src/idiomorph.js index ad601a9..c143b93 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,24 +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 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)) { @@ -428,7 +449,7 @@ var Idiomorph = (function () { // increment the count of future soft matches siblingSoftMatchCount++; nextSibling = nextSibling.nextSibling; - + // If there are two future soft matches, block soft matching for this node to allow // future siblings to soft match. This is to reduce churn in the DOM when an element // is prepended. @@ -489,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; })(); 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 e5fc4ce..ee1aa52 100644 --- a/test/ops.js +++ b/test/ops.js @@ -222,5 +222,165 @@ describe("morphing operations", function () { ["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, + ); + }); }); From 91d8d7c14a2bcb4ec12536167fc54a93f8d04e4f Mon Sep 17 00:00:00 2001 From: MichaelWest22 Date: Fri, 31 Jan 2025 12:33:16 +1300 Subject: [PATCH 5/5] whitespace --- src/idiomorph.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/idiomorph.js b/src/idiomorph.js index c143b93..f045b70 100644 --- a/src/idiomorph.js +++ b/src/idiomorph.js @@ -403,7 +403,7 @@ var Idiomorph = (function () { } let softMatch = null; - let nextSibling = node.nextSibling; + let nextSibling = node.nextSibling; let siblingSoftMatchCount = 0; let displaceMatchCount = 0; @@ -449,7 +449,7 @@ var Idiomorph = (function () { // increment the count of future soft matches siblingSoftMatchCount++; nextSibling = nextSibling.nextSibling; - + // If there are two future soft matches, block soft matching for this node to allow // future siblings to soft match. This is to reduce churn in the DOM when an element // is prepended.