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.

Sign up for GitHub

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

Compress to a single-pass #84

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Compress to a single-pass #84

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eb796d0
failing test for returning false to beforeNodeMorphed during pantry r…
botandrose-machine Dec 23, 2024
1341e04
we can just reuse morphNodeTo instead of this half-working mess.
botandrose-machine Dec 23, 2024
9058644
remove abstraction-leaking beforeNodePantried hook and perform moves …
botandrose-machine Dec 24, 2024
8dc5d27
type-checker appeasement.
botandrose-machine Dec 24, 2024
6efe380
plug a hole in test coverage.
botandrose-machine Dec 25, 2024
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
203 changes: 109 additions & 94 deletions src/idiomorph.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
* @property {function(Element): boolean} [beforeNodeRemoved]
* @property {function(Element): void} [afterNodeRemoved]
* @property {function(string, Element, "update" | "remove"): boolean} [beforeAttributeUpdated]
* @property {function(Element): boolean} [beforeNodePantried]
*/

/**
Expand Down Expand Up @@ -61,7 +60,6 @@
* @property {(function(Node): boolean) | NoOp} beforeNodeRemoved
* @property {(function(Node): void) | NoOp} afterNodeRemoved
* @property {(function(string, Element, "update" | "remove"): boolean) | NoOp} beforeAttributeUpdated
* @property {(function(Node): boolean) | NoOp} beforeNodePantried
*/

/**
Expand Down Expand Up @@ -95,8 +93,8 @@ var Idiomorph = (function () {
/**
* @typedef {object} MorphContext
*
* @property {Node} target
* @property {Node} newContent
* @property {Element} target
* @property {Element} newContent
* @property {ConfigInternal} config
* @property {ConfigInternal['morphStyle']} morphStyle
* @property {ConfigInternal['ignoreActive']} ignoreActive
Expand Down Expand Up @@ -133,7 +131,6 @@ var Idiomorph = (function () {
beforeNodeRemoved: noOp,
afterNodeRemoved: noOp,
beforeAttributeUpdated: noOp,
beforeNodePantried: noOp,
},
head: {
style: "merge",
Expand Down Expand Up @@ -208,7 +205,7 @@ var Idiomorph = (function () {
// innerHTML, so we are only updating the children
morphChildren(normalizedNewContent, oldNode, ctx);
if (ctx.config.twoPass) {
restoreFromPantry(oldNode, ctx);
ctx.pantry.remove();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when two pass is off ctx.pantry is just a dis joined div so we can just call remove() on it every time to simplify the code

}
return Array.from(oldNode.children);
} else if (ctx.morphStyle === "outerHTML" || ctx.morphStyle == null) {
Expand All @@ -233,7 +230,7 @@ var Idiomorph = (function () {
nextSibling,
);
if (ctx.config.twoPass) {
restoreFromPantry(morphedNode.parentNode, ctx);
ctx.pantry.remove();
}
return elements;
}
Expand Down Expand Up @@ -305,6 +302,7 @@ var Idiomorph = (function () {
} else {
syncNodeFrom(newContent, oldNode, ctx);
if (!ignoreValueOfActiveElement(oldNode, ctx)) {
// @ts-ignore newContent can be a node here because .firstChild will be null
morphChildren(newContent, oldNode, ctx);
}
}
Expand Down Expand Up @@ -332,8 +330,8 @@ var Idiomorph = (function () {
* The two search algorithms terminate if competing node matches appear to outweigh what can be achieved
* with the current node. See findIdSetMatch() and findSoftMatch() for details.
*
* @param {Node} newParent the parent element of the new content
* @param {Node} oldParent the old content that we are merging the new content into
* @param {Element} newParent the parent element of the new content
* @param {Element} oldParent the old content that we are merging the new content into
* @param {MorphContext} ctx the merge context
* @returns {void}
*/
Expand All @@ -342,7 +340,9 @@ var Idiomorph = (function () {
newParent instanceof HTMLTemplateElement &&
oldParent instanceof HTMLTemplateElement
) {
// @ts-ignore we can pretend the DocumentFragment is an Element
newParent = newParent.content;
// @ts-ignore ditto
oldParent = oldParent.content;
}

Expand All @@ -365,12 +365,17 @@ var Idiomorph = (function () {

// if we are at the end of the exiting parent's children, just append
if (insertionPoint == null) {
// skip add callbacks when we're going to be restoring this from the pantry in the second pass
if (
ctx.config.twoPass &&
ctx.persistentIds.has(/** @type {Element} */ (newChild).id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here if twoPass is false the persistentIds is an empty set so it may be possible to remove the twoPass config check here as well. There could be a very small perf hit to check has on an empty set here but its probably not measurable either way.

) {
oldParent.appendChild(newChild);
const movedChild = moveBeforeById(
oldParent,
/** @type {Element} */ (newChild).id,
null,
ctx,
);
morphOldNodeTo(movedChild, newChild, ctx);
} else {
if (ctx.callbacks.beforeNodeAdded(newChild) === false) continue;
oldParent.appendChild(newChild);
Expand Down Expand Up @@ -399,12 +404,26 @@ var Idiomorph = (function () {

// if we found a potential match, remove the nodes until that point and morph
if (idSetMatch) {
insertionPoint = removeNodesBetween(insertionPoint, idSetMatch, ctx);
if (ctx.config.twoPass) {
moveBefore(oldParent, idSetMatch, insertionPoint);
} else {
insertionPoint = removeNodesBetween(insertionPoint, idSetMatch, ctx);
}
morphOldNodeTo(idSetMatch, newChild, ctx);
removeIdsFromConsideration(ctx, newChild);
continue;
}

if (
ctx.config.twoPass && // maybe we can run this in default mode too?
isSoftMatch(insertionPoint, newChild)
) {
morphOldNodeTo(insertionPoint, newChild, ctx);
insertionPoint = insertionPoint.nextSibling;
removeIdsFromConsideration(ctx, newChild);
continue;
}

// no id set match found, so scan forward for a soft match for the current node
let softMatch = findSoftMatch(
newParent,
Expand All @@ -416,7 +435,11 @@ var Idiomorph = (function () {

// if we found a soft match for the current node, morph
if (softMatch) {
insertionPoint = removeNodesBetween(insertionPoint, softMatch, ctx);
if (ctx.config.twoPass) {
moveBefore(oldParent, softMatch, insertionPoint);
} else {
insertionPoint = removeNodesBetween(insertionPoint, softMatch, ctx);
}
morphOldNodeTo(softMatch, newChild, ctx);
removeIdsFromConsideration(ctx, newChild);
continue;
Expand All @@ -425,16 +448,28 @@ var Idiomorph = (function () {
// abandon all hope of morphing, just insert the new child before the insertion point
// and move on

// skip add callbacks when we're going to be restoring this from the pantry in the second pass
if (
ctx.config.twoPass &&
ctx.persistentIds.has(/** @type {Element} */ (newChild).id)
) {
oldParent.insertBefore(newChild, insertionPoint);
const movedChild = moveBeforeById(
oldParent,
/** @type {Element} */ (newChild).id,
insertionPoint,
ctx,
);
morphOldNodeTo(movedChild, newChild, ctx);
} else {
if (ctx.callbacks.beforeNodeAdded(newChild) === false) continue;
oldParent.insertBefore(newChild, insertionPoint);
ctx.callbacks.afterNodeAdded(newChild);
if (ctx.config.twoPass) {
// maybe this should be a bugfix and in both modes?
const newClonedChild = document.importNode(newChild, true); // clone as to not mutate newParent
oldParent.insertBefore(newClonedChild, insertionPoint);
ctx.callbacks.afterNodeAdded(newClonedChild);
} else {
oldParent.insertBefore(newChild, insertionPoint);
ctx.callbacks.afterNodeAdded(newChild);
}
}
removeIdsFromConsideration(ctx, newChild);
}
Expand Down Expand Up @@ -536,6 +571,7 @@ var Idiomorph = (function () {
if (!(from instanceof Element && to instanceof Element)) return;
// @ts-ignore this function is only used on boolean attrs that are reflected as dom properties
const fromLiveValue = from[attributeName],
// @ts-ignore ditto
toLiveValue = to[attributeName];
if (fromLiveValue !== toLiveValue) {
let ignoreUpdate = ignoreAttribute(attributeName, to, "update", ctx);
Expand Down Expand Up @@ -1184,91 +1220,62 @@ var Idiomorph = (function () {

/**
*
* @param {Node} tempNode
* @param {Node} node
* @param {MorphContext} ctx
*/
// TODO: The function handles tempNode as if it's Element but the function is called in
// places where tempNode may be just a Node, not an Element
function removeNode(tempNode, ctx) {
removeIdsFromConsideration(ctx, tempNode);
// skip remove callbacks when we're going to be restoring this from the pantry in the second pass
function removeNode(node, ctx) {
removeIdsFromConsideration(ctx, node);
// skip remove callbacks when we're going to be restoring this from the pantry later
if (
ctx.config.twoPass &&
hasPersistentIdNodes(ctx, tempNode) &&
tempNode instanceof Element
hasPersistentIdNodes(ctx, node) &&
node instanceof Element
) {
moveToPantry(tempNode, ctx);
moveBefore(ctx.pantry, node, null);
} else {
if (ctx.callbacks.beforeNodeRemoved(tempNode) === false) return;
tempNode.parentNode?.removeChild(tempNode);
ctx.callbacks.afterNodeRemoved(tempNode);
if (ctx.callbacks.beforeNodeRemoved(node) === false) return;
node.parentNode?.removeChild(node);
ctx.callbacks.afterNodeRemoved(node);
}
}

/**
* Search for an element by id within the document and pantry, and move it using moveBefore.
*
* @param {Node} node
* @param {Element} parentNode - The parent node to which the element will be moved.
* @param {string} id - The ID of the element to be moved.
* @param {Node | null} after - The reference node to insert the element before.
* If `null`, the element is appended as the last child.
* @param {MorphContext} ctx
* @returns {Element} The found element
*/
function moveToPantry(node, ctx) {
if (ctx.callbacks.beforeNodePantried(node) === false) return;

Array.from(node.childNodes).forEach((child) => {
moveToPantry(child, ctx);
});

// After processing children, process the current node
if (ctx.persistentIds.has(/** @type {Element} */ (node).id)) {
// @ts-ignore - use proposed moveBefore feature
if (ctx.pantry.moveBefore) {
// @ts-ignore - use proposed moveBefore feature
ctx.pantry.moveBefore(node, null);
} else {
ctx.pantry.insertBefore(node, null);
}
} else {
if (ctx.callbacks.beforeNodeRemoved(node) === false) return;
node.parentNode?.removeChild(node);
ctx.callbacks.afterNodeRemoved(node);
}
function moveBeforeById(parentNode, id, after, ctx) {
const target =
/** @type {Element} - will always be found */
(
ctx.target.querySelector(`#${id}`) || ctx.pantry.querySelector(`#${id}`)
);
moveBefore(parentNode, target, after);
return target;
}

/**
* Moves an element before another element within the same parent.
* Uses the proposed `moveBefore` API if available, otherwise falls back to `insertBefore`.
* This is essentialy a forward-compat wrapper.
*
* @param {Node | null} root
* @param {MorphContext} ctx
* @param {Element} parentNode - The parent node containing the after element.
* @param {Node} element - The element to be moved.
* @param {Node | null} after - The reference node to insert `element` before.
* If `null`, `element` is appended as the last child.
*/
function restoreFromPantry(root, ctx) {
if (root instanceof Element) {
Array.from(ctx.pantry.children)
.reverse()
.forEach((element) => {
const matchElement = root.querySelector(`#${element.id}`);
if (matchElement) {
// @ts-ignore - use proposed moveBefore feature
if (matchElement.parentElement?.moveBefore) {
// @ts-ignore - use proposed moveBefore feature
matchElement.parentElement.moveBefore(element, matchElement);
while (matchElement.hasChildNodes()) {
// @ts-ignore - use proposed moveBefore feature
element.moveBefore(matchElement.firstChild, null);
}
} else {
matchElement.before(element);
while (matchElement.firstChild) {
element.insertBefore(matchElement.firstChild, null);
}
}
if (
ctx.callbacks.beforeNodeMorphed(element, matchElement) !== false
) {
syncNodeFrom(matchElement, element, ctx);
ctx.callbacks.afterNodeMorphed(element, matchElement);
}
matchElement.remove();
}
});
ctx.pantry.remove();
function moveBefore(parentNode, element, after) {
// @ts-ignore - use proposed moveBefore feature
if (parentNode.moveBefore) {
// @ts-ignore - use proposed moveBefore feature
parentNode.moveBefore(element, after);
} else {
parentNode.insertBefore(element, after);
}
}

Expand Down Expand Up @@ -1350,12 +1357,12 @@ var Idiomorph = (function () {
* @param {Element} content
* @returns {Element[]}
*/
function nodesWithIds(content) {
let nodes = Array.from(content.querySelectorAll("[id]"));
function elementsWithIds(content) {
let elements = Array.from(content.querySelectorAll("[id]"));
if (content.id) {
nodes.push(content);
elements.push(content);
}
return nodes;
return elements;
}

/**
Expand All @@ -1368,7 +1375,7 @@ var Idiomorph = (function () {
*/
function populateIdMapForNode(node, idMap) {
let nodeParent = node.parentElement;
for (const elt of nodesWithIds(node)) {
for (const elt of elementsWithIds(node)) {
/**
* @type {Element|null}
*/
Expand Down Expand Up @@ -1415,18 +1422,26 @@ var Idiomorph = (function () {
* @returns {Set<string>} the id set of all persistent nodes that exist in both old and new content
*/
function createPersistentIds(oldContent, newContent) {
const toIdTagName = (node) => node.tagName + "#" + node.id;
const oldIdSet = new Set(nodesWithIds(oldContent).map(toIdTagName));
const oldIdSet = new Set(elementsWithIds(oldContent).map(toIdTagName));

let matchIdSet = new Set();
for (const newNode of nodesWithIds(newContent)) {
if (oldIdSet.has(toIdTagName(newNode))) {
matchIdSet.add(newNode.id);
for (const newElement of elementsWithIds(newContent)) {
if (oldIdSet.has(toIdTagName(newElement))) {
matchIdSet.add(newElement.id);
}
}
return matchIdSet;
}

/**
* Generates a string in the format "TAGNAME#id" for a given DOM element.
*
* @param {Element} element - The DOM element to generate the string for.
* @returns {string} The generated string in the format "TAGNAME#id".
*/
function toIdTagName(element) {
return element.tagName + "#" + element.id;
}
//=============================================================================
// This is what ends up becoming the Idiomorph global object
//=============================================================================
Expand Down
Loading
Loading