Skip to content

Commit

Permalink
Merge pull request #75 from MichaelWest22/fix-persistent-ids
Browse files Browse the repository at this point in the history 
Fix persistent ids being softMatched when they shouldn't
  • Loading branch information
@1cg
1cg authored Dec 23, 2024
2 parents 6c47b83 + b44ceff commit 0ef76f0
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 9 deletions.
55 changes: 46 additions & 9 deletions src/idiomorph.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ var Idiomorph = (function () {
* @property {ConfigInternal['ignoreActive']} ignoreActive
* @property {ConfigInternal['ignoreActiveValue']} ignoreActiveValue
* @property {Map<Node, Set<string>>} idMap
* @property {Set<string>} persistentIds
* @property {Set<string>} deadIds
* @property {ConfigInternal['callbacks']} callbacks
* @property {ConfigInternal['head']} head
Expand Down Expand Up @@ -259,7 +260,7 @@ var Idiomorph = (function () {
oldNode.parentNode?.removeChild(oldNode);
ctx.callbacks.afterNodeRemoved(oldNode);
return null;
} else if (!isSoftMatch(oldNode, newContent)) {
} else if (!isSoftMatch(oldNode, newContent, ctx)) {
if (ctx.callbacks.beforeNodeRemoved(oldNode) === false) return oldNode;
if (ctx.callbacks.beforeNodeAdded(newContent) === false) return oldNode;

Expand Down Expand Up @@ -717,6 +718,7 @@ var Idiomorph = (function () {
ignoreActive: mergedConfig.ignoreActive,
ignoreActiveValue: mergedConfig.ignoreActiveValue,
idMap: createIdMap(oldNode, newContent),
persistentIds: persistentIdSet(oldNode, newContent),
deadIds: new Set(),
callbacks: mergedConfig.callbacks,
head: mergedConfig.head
Expand Down Expand Up @@ -751,12 +753,18 @@ var Idiomorph = (function () {
*
* @param {Node | null} node1
* @param {Node | null} node2
* @param {MorphContext} ctx
* @returns {boolean}
*/
function isSoftMatch(node1, node2) {
function isSoftMatch(node1, node2, ctx) {
if (node1 == null || node2 == null) {
return false;
}
// If the id's do not match and either of the id's are persisted through the morph then they can't be soft matches
if ( /** @type {Element} */ (node1).id !== /** @type {Element} */ (node2).id
&& (ctx.persistentIds.has(/** @type {Element} */ (node1).id) || ctx.persistentIds.has(/** @type {Element} */ (node2).id))) {
return false;
}
return node1.nodeType === node2.nodeType &&
// ok to cast: if one is not element, `tagName` will be undefined and we'll compare that
/** @type {Element} */ (node1).tagName === /** @type {Element} */ (node2).tagName
Expand Down Expand Up @@ -876,11 +884,11 @@ var Idiomorph = (function () {
}

// if we have a soft match with the current node, return it
if (isSoftMatch(newChild, potentialSoftMatch)) {
if (isSoftMatch(newChild, potentialSoftMatch, ctx)) {
return potentialSoftMatch;
}

if (isSoftMatch(nextSibling, potentialSoftMatch)) {
if (isSoftMatch(nextSibling, potentialSoftMatch, ctx)) {
// the next new node has a soft match with this node, so
// increment the count of future soft matches
siblingSoftMatchCount++;
Expand Down Expand Up @@ -1051,7 +1059,7 @@ var Idiomorph = (function () {
// TODO: The function handles node1 and node2 as if they are Elements but the function is
// called in places where node1 and node2 may be just Nodes, not Elements
function scoreElement(node1, node2, ctx) {
if (isSoftMatch(node1, node2)) {
if (isSoftMatch(node1, node2, ctx)) {
// ok to cast: isSoftMatch performs a null check
return .5 + getIdIntersectionCount(ctx, /** @type {Node} */ (node1), node2);
}
Expand Down Expand Up @@ -1133,7 +1141,19 @@ var Idiomorph = (function () {
}

/**
* A bottom up algorithm that finds all elements with ids inside of the node
* @param {Element} Content
* @returns {Element[]}
*/
function nodesWithIds(Content) {
let Nodes = Array.from(Content.querySelectorAll('[id]'));
if(Content.id) {
Nodes.push(Content);
}
return Nodes;
}

/**
* A bottom up algorithm that finds all elements with ids in the node
* argument and populates id sets for those nodes and all their parents, generating
* a set of ids contained within all nodes for the entire hierarchy in the DOM
*
Expand All @@ -1142,9 +1162,7 @@ var Idiomorph = (function () {
*/
function populateIdMapForNode(node, idMap) {
let nodeParent = node.parentElement;
// find all elements with an id property
let idElements = node.querySelectorAll('[id]');
for (const elt of idElements) {
for (const elt of nodesWithIds(node)) {
/**
* @type {Element|null}
*/
Expand Down Expand Up @@ -1185,6 +1203,25 @@ var Idiomorph = (function () {
return idMap;
}

/**
* @param {Element} oldContent the old content that will be morphed
* @param {Element} newContent the new content to morph to
* @returns {Set<string>} the id set of all persistent nodes that exist in both old and new content
*/
function persistentIdSet(oldContent, newContent) {
let matchIdSet = new Set();
let oldIdSet = new Set();
for (const oldNode of nodesWithIds(oldContent)) {
oldIdSet.add(oldNode.id+':'+oldNode.tagName);
}
for (const newNode of nodesWithIds(newContent)) {
if (oldIdSet.has(newNode.id+':'+newNode.tagName)) {
matchIdSet.add(newNode.id);
}
}
return matchIdSet;
}

//=============================================================================
// This is what ends up becoming the Idiomorph global object
//=============================================================================
Expand Down
31 changes: 31 additions & 0 deletions test/core.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,4 +434,35 @@ describe("Core morphing tests", function(){
}
});

it('non-attribute state of element with id is not transfered to other elements when moved between different containers', function()
{
getWorkArea().append(make(`
<div>
<div id="left">
<input type="checkbox" id="first">
</div>
<div id="right">
<input type="checkbox" id="second">
</div>
</div>
`));
document.getElementById("first").indeterminate = true

let finalSrc = `
<div>
<div id="left">
<input type="checkbox" id="second">
</div>
<div id="right">
<input type="checkbox" id="first">
</div>
</div>
`;
Idiomorph.morph(getWorkArea(), finalSrc, {morphStyle:'innerHTML'});

getWorkArea().innerHTML.should.equal(finalSrc);
// second checkbox is now in firsts place and we don't want firsts indeterminate state to be retained
// on what is now the second element so we need to prevent softMatch mroph from matching persistent id's
document.getElementById("second").indeterminate.should.eql(false)
});
})

0 comments on commit 0ef76f0

Please sign in to comment.