Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve softMatch to not retain state on morph if oldNode has id #82

Merged
merged 2 commits into from
Dec 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 14 additions & 10 deletions src/idiomorph.js
Original file line number Diff line number Diff line change
Expand Up @@ -744,17 +744,21 @@ var Idiomorph = (function () {

/**
*
* @param {Node | null} node1
* @param {Node | null} node2
* @param {Node | null} oldNode
* @param {Node | null} newNode
* @returns {boolean}
*/
function isSoftMatch(node1, node2) {
if (node1 == null || node2 == null) {
function isSoftMatch(oldNode, newNode) {
if (oldNode == null || newNode == null) {
return false;
}
// ok to cast: if one is not element, `id` or `tagName` will be undefined and we'll compare that
// If oldNode has an `id` with possible state and it doesn't match newNode.id then avoid morphing
if ( /** @type {Element} */ (oldNode).id && /** @type {Element} */ (oldNode).id !== /** @type {Element} */ (newNode).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
return oldNode.nodeType === newNode.nodeType &&
/** @type {Element} */ (oldNode).tagName === /** @type {Element} */ (newNode).tagName
}

/**
Expand Down Expand Up @@ -871,11 +875,11 @@ var Idiomorph = (function () {
}

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

if (isSoftMatch(nextSibling, potentialSoftMatch)) {
if (isSoftMatch(potentialSoftMatch, nextSibling)) {
// the next new node has a soft match with this node, so
// increment the count of future soft matches
siblingSoftMatchCount++;
Expand Down Expand Up @@ -1046,7 +1050,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(node2, node1)) {
// ok to cast: isSoftMatch performs a null check
return .5 + getIdIntersectionCount(ctx, /** @type {Node} */ (node1), node2);
}
Expand Down
2 changes: 1 addition & 1 deletion test/bootstrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe("Bootstrap test", function(){
let d2 = div1.querySelector("#d2")
let d3 = div1.querySelector("#d3")

let morphTo = '<div id="root2"><div><div id="d2">E</div></div><div><div id="d3">F</div></div><div><div id="d1">D</div></div></div>';
let morphTo = '<div id="root1"><div><div id="d2">E</div></div><div><div id="d3">F</div></div><div><div id="d1">D</div></div></div>';
let div2 = make(morphTo)

print(div1);
Expand Down
86 changes: 86 additions & 0 deletions test/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -474,4 +474,90 @@ 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)
});

it('softMatch matches when old and new elt have no id so morphs in place', function()
{
let div = make('<div>A</div>');
getWorkArea().append(parent);

let finalSrc = '<div>B</div>';
Idiomorph.morph(div, finalSrc, {morphStyle:'outerHTML'});
// existing div gets morphed into B
div.innerHTML.should.equal('B');
});

it('softMatch does not match when old elt has id but new elt does not so no morph in place', function()
{
let div = make('<div id="d1">A</div>');
getWorkArea().append(parent);

let finalSrc = '<div>B</div>';
Idiomorph.morph(div, finalSrc, {morphStyle:'outerHTML'});
// existing div gets removed
div.innerHTML.should.equal('A');
});

it('softMatch matches when old and new elt have idhas no id but new elt does so morph in place', function()
{
let div = make('<div>A</div>');
getWorkArea().append(parent);

let finalSrc = '<div id="d1">B</div>';
Idiomorph.morph(div, finalSrc, {morphStyle:'outerHTML'});
// existing div gets morphed into B
div.innerHTML.should.equal('B');
});

it('softMatch does not match when old and new elt have different ids so no morph in place', function()
{
let div = make('<div id="d1">A</div>');
getWorkArea().append(parent);

let finalSrc = '<div id="d2">B</div>';
Idiomorph.morph(div, finalSrc, {morphStyle:'outerHTML'});
// existing div gets removed
div.innerHTML.should.equal('A');
});

it('idSetMatch matches when old and new elt have the same id so morph in place', function()
{
let div = make('<div id="d1">A</div>');
getWorkArea().append(parent);

let finalSrc = '<div id="d1">B</div>';
Idiomorph.morph(div, finalSrc, {morphStyle:'outerHTML'});
// existing div gets morphed into B
div.innerHTML.should.equal('B');
});
})
2 changes: 1 addition & 1 deletion test/fidelity.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe("Tests to ensure that idiomorph merges properly", function(){
it('morphs multiple attributes correctly twice', function ()
{
const a = `<section class="child">A</section>`;
const b = `<section class="thing" data-one="1" data-two="2" data-three="3" data-four="4" id="foo" fizz="buzz" foo="bar">B</section>`;
const b = `<section class="thing" data-one="1" data-two="2" data-three="3" data-four="4" fizz="buzz" foo="bar">B</section>`;
const expectedA = make(a);
const expectedB = make(b);
const initial = make(a);
Expand Down
Loading