Skip to content

Commit 87b84db

Browse files
botandrose-machinebotandrose
authored andcommitted
inline id function for better perf.
1 parent c0af88e commit 87b84db

File tree

1 file changed

+39
-33
lines changed

1 file changed

+39
-33
lines changed

src/idiomorph.js

Lines changed: 39 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,10 @@ var Idiomorph = (function () {
225225

226226
const results = fn();
227227

228-
if (activeElementId && activeElementId !== id(document.activeElement)) {
228+
if (
229+
activeElementId &&
230+
activeElementId !== document.activeElement?.getAttribute("id")
231+
) {
229232
activeElement = ctx.target.querySelector(`[id="${activeElementId}"]`);
230233
activeElement?.focus();
231234
}
@@ -304,20 +307,23 @@ var Idiomorph = (function () {
304307
}
305308

306309
// if the matching node is elsewhere in the original content
307-
if (
308-
newChild instanceof Element &&
309-
ctx.persistentIds.has(id(newChild))
310-
) {
311-
// move it and all its children here and morph
312-
const movedChild = moveBeforeById(
313-
oldParent,
314-
id(newChild),
315-
insertionPoint,
316-
ctx,
310+
if (newChild instanceof Element) {
311+
// we can pretend the id is non-null because the next `.has` line will reject it if not
312+
const newChildId = /** @type {String} */ (
313+
newChild.getAttribute("id")
317314
);
318-
morphNode(movedChild, newChild, ctx);
319-
insertionPoint = movedChild.nextSibling;
320-
continue;
315+
if (ctx.persistentIds.has(newChildId)) {
316+
// move it and all its children here and morph
317+
const movedChild = moveBeforeById(
318+
oldParent,
319+
newChildId,
320+
insertionPoint,
321+
ctx,
322+
);
323+
morphNode(movedChild, newChild, ctx);
324+
insertionPoint = movedChild.nextSibling;
325+
continue;
326+
}
321327
}
322328

323329
// last resort: insert the new node from scratch
@@ -477,7 +483,9 @@ var Idiomorph = (function () {
477483
// If oldElt has an `id` with possible state and it doesn't match newElt.id then avoid morphing.
478484
// We'll still match an anonymous node with an IDed newElt, though, because if it got this far,
479485
// its not persistent, and new nodes can't have any hidden state.
480-
(!id(oldElt) || id(oldElt) === id(newElt))
486+
// We can't use .id because of form input shadowing, and we can't count on .getAttribute's presence because it could be a document-fragment
487+
(!oldElt.getAttribute?.("id") ||
488+
oldElt.getAttribute?.("id") === newElt.getAttribute?.("id"))
481489
);
482490
}
483491

@@ -531,19 +539,21 @@ var Idiomorph = (function () {
531539
* Search for an element by id within the document and pantry, and move it using moveBefore.
532540
*
533541
* @param {Element} parentNode - The parent node to which the element will be moved.
534-
* @param {string} elementId - The ID of the element to be moved.
542+
* @param {string} id - The ID of the element to be moved.
535543
* @param {Node | null} after - The reference node to insert the element before.
536544
* If `null`, the element is appended as the last child.
537545
* @param {MorphContext} ctx
538546
* @returns {Element} The found element
539547
*/
540-
function moveBeforeById(parentNode, elementId, after, ctx) {
548+
function moveBeforeById(parentNode, id, after, ctx) {
541549
const target =
542550
/** @type {Element} - will always be found */
543551
(
544-
(id(ctx.target) === elementId && ctx.target) ||
545-
ctx.target.querySelector(`[id="${elementId}"]`) ||
546-
ctx.pantry.querySelector(`[id="${elementId}"]`)
552+
// ctx.target.id unsafe because of form input shadowing
553+
// ctx.target could be a document fragment which doesn't have `getAttribute`
554+
(ctx.target.getAttribute?.("id") === id && ctx.target) ||
555+
ctx.target.querySelector(`[id="${id}"]`) ||
556+
ctx.pantry.querySelector(`[id="${id}"]`)
547557
);
548558
removeElementFromAncestorsIdMaps(target, ctx);
549559
moveBefore(parentNode, target, after);
@@ -559,12 +569,13 @@ var Idiomorph = (function () {
559569
* @param {MorphContext} ctx
560570
*/
561571
function removeElementFromAncestorsIdMaps(element, ctx) {
562-
const elementId = id(element);
572+
// we know id is non-null String, because this function is only called on elements with ids
573+
const id = /** @type {String} */ (element.getAttribute("id"));
563574
/** @ts-ignore - safe to loop in this way **/
564575
while ((element = element.parentNode)) {
565576
let idSet = ctx.idMap.get(element);
566577
if (idSet) {
567-
idSet.delete(elementId);
578+
idSet.delete(id);
568579
if (!idSet.size) {
569580
ctx.idMap.delete(element);
570581
}
@@ -1047,7 +1058,8 @@ var Idiomorph = (function () {
10471058
*/
10481059
function findIdElements(root) {
10491060
let elements = Array.from(root.querySelectorAll("[id]"));
1050-
if (id(root)) {
1061+
// root could be a document fragment which doesn't have `getAttribute`
1062+
if (root.getAttribute?.("id")) {
10511063
elements.push(root);
10521064
}
10531065
return elements;
@@ -1066,7 +1078,9 @@ var Idiomorph = (function () {
10661078
*/
10671079
function populateIdMapWithTree(idMap, persistentIds, root, elements) {
10681080
for (const elt of elements) {
1069-
if (persistentIds.has(id(elt))) {
1081+
// we can pretend id is non-null String, because the .has line will reject it immediately if not
1082+
const id = /** @type {String} */ (elt.getAttribute("id"));
1083+
if (persistentIds.has(id)) {
10701084
/** @type {Element|null} */
10711085
let current = elt;
10721086
// walk up the parent hierarchy of that element, adding the id
@@ -1078,7 +1092,7 @@ var Idiomorph = (function () {
10781092
idSet = new Set();
10791093
idMap.set(current, idSet);
10801094
}
1081-
idSet.add(id(elt));
1095+
idSet.add(id);
10821096

10831097
if (current === root) break;
10841098
current = current.parentElement;
@@ -1338,14 +1352,6 @@ var Idiomorph = (function () {
13381352
return { normalizeElement, normalizeParent };
13391353
})();
13401354

1341-
// @ts-ignore Nodeish is fine
1342-
function id(node) {
1343-
// node could be a <form> so we can't trust node.id:
1344-
// https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement#issues_with_naming_elements
1345-
// node could be a document-fragment which doesn't have getAttribute defined
1346-
return node.getAttribute && node.getAttribute("id");
1347-
}
1348-
13491355
//=============================================================================
13501356
// This is what ends up becoming the Idiomorph global object
13511357
//=============================================================================

0 commit comments

Comments
 (0)