From 14e22c428ea0453b530eb61f3ab1036b0727db26 Mon Sep 17 00:00:00 2001 From: MichaelWest22 Date: Fri, 31 Jan 2025 11:25:46 +1300 Subject: [PATCH 1/4] 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/4] 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/4] 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 588c9fff1d773ee4e4749a0cfc79a07184c0ea60 Mon Sep 17 00:00:00 2001 From: MichaelWest22 Date: Sat, 1 Feb 2025 20:23:02 +1300 Subject: [PATCH 4/4] Remove idset changes --- src/idiomorph.js | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/idiomorph.js b/src/idiomorph.js index 2ce60c6..f78733f 100644 --- a/src/idiomorph.js +++ b/src/idiomorph.js @@ -535,26 +535,6 @@ 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. *