Skip to content

Commit

Permalink
Port skew based child diffing (#4010)
Browse files Browse the repository at this point in the history 
* first attempt at porting skew-based

* down to only failing fragment test

* reduce complexity of placeChild

* 2 failing tests left

* port skew based diff

* add tests for other fixed issues

* simplify children diffing

* experiment

* remove more nextDom st uff

* one more try

* remove unused import

* simplify suspension check, i.e. whether dom is connected by replacing it with _original === null

* simplify if check

* remove lastDom helper

* simplify more checks

* always use insertBefore

* experiment

* add hyrating check back

* simplify matcher

* try removing isHydrating

* Revert "try removing isHydrating"

This reverts commit 787ad17.

* remove isHydrating

* add the !isMounting back
  • Loading branch information
@JoviDeCroock
JoviDeCroock authored Jun 14, 2023
1 parent 841ef82 commit d887539
Show file tree
Hide file tree
Showing 5 changed files with 429 additions and 157 deletions.
244 changes: 121 additions & 123 deletions src/diff/children.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { diff, unmount, applyRef } from './index';
import { createVNode, Fragment } from '../create-element';
import { EMPTY_OBJ, EMPTY_ARR } from '../constants';
import { getDomSibling } from '../component';
import { isArray } from '../util';

/**
Expand Down Expand Up @@ -36,16 +35,25 @@ export function diffChildren(
oldDom,
isHydrating
) {
let i, j, oldVNode, childVNode, newDom, firstChildDom, refs;
let i,
j,
oldVNode,
childVNode,
newDom,
firstChildDom,
refs,
skew = 0;

// This is a compression of oldParentVNode!=null && oldParentVNode != EMPTY_OBJ && oldParentVNode._children || EMPTY_ARR
// as EMPTY_OBJ._children should be `undefined`.
let oldChildren = (oldParentVNode && oldParentVNode._children) || EMPTY_ARR;

let oldChildrenLength = oldChildren.length;
let oldChildrenLength = oldChildren.length,
remainingOldChildren = oldChildrenLength,
newChildrenLength = renderResult.length;

newParentVNode._children = [];
for (i = 0; i < renderResult.length; i++) {
for (i = 0; i < newChildrenLength; i++) {
childVNode = renderResult[i];

if (
Expand Down Expand Up @@ -104,40 +112,22 @@ export function diffChildren(
childVNode._parent = newParentVNode;
childVNode._depth = newParentVNode._depth + 1;

// Check if we find a corresponding element in oldChildren.
// If found, delete the array item by setting to `undefined`.
// We use `undefined`, as `null` is reserved for empty placeholders
// (holes).
oldVNode = oldChildren[i];
let skewedIndex = i + skew;
const matchingIndex = findMatchingIndex(
childVNode,
oldChildren,
skewedIndex,
remainingOldChildren
);

if (
oldVNode === null ||
(oldVNode &&
childVNode.key == oldVNode.key &&
childVNode.type === oldVNode.type)
) {
oldChildren[i] = undefined;
if (matchingIndex === -1) {
oldVNode = EMPTY_OBJ;
} else {
// Either oldVNode === undefined or oldChildrenLength > 0,
// so after this loop oldVNode == null or oldVNode is a valid value.
for (j = 0; j < oldChildrenLength; j++) {
oldVNode = oldChildren[j];
// If childVNode is unkeyed, we only match similarly unkeyed nodes, otherwise we match by key.
// We always match by type (in either case).
if (
oldVNode &&
childVNode.key == oldVNode.key &&
childVNode.type === oldVNode.type
) {
oldChildren[j] = undefined;
break;
}
oldVNode = null;
}
oldVNode = oldChildren[matchingIndex] || EMPTY_OBJ;
oldChildren[matchingIndex] = undefined;
remainingOldChildren--;
}

oldVNode = oldVNode || EMPTY_OBJ;

// Morph the old element into the new one, but don't append it to the dom yet
diff(
parentDom,
Expand All @@ -164,24 +154,60 @@ export function diffChildren(
firstChildDom = newDom;
}

let isMounting = oldVNode === EMPTY_OBJ || oldVNode._original === null;
let hasMatchingIndex = !isMounting && matchingIndex === skewedIndex;
if (isMounting) {
if (matchingIndex == -1) {
skew--;
}
} else if (matchingIndex !== skewedIndex) {
if (matchingIndex === skewedIndex + 1) {
skew++;
hasMatchingIndex = true;
} else if (matchingIndex > skewedIndex) {
if (remainingOldChildren > newChildrenLength - skewedIndex) {
skew += matchingIndex - skewedIndex;
hasMatchingIndex = true;
} else {
// ### Change from keyed: I think this was missing from the algo...
skew--;
}
} else if (matchingIndex < skewedIndex) {
if (matchingIndex == skewedIndex - 1) {
skew = matchingIndex - skewedIndex;
} else {
skew = 0;
}
} else {
skew = 0;
}
}

skewedIndex = i + skew;
hasMatchingIndex =
hasMatchingIndex || (matchingIndex == i && !isMounting);

if (
typeof childVNode.type == 'function' &&
childVNode._children === oldVNode._children
(matchingIndex !== skewedIndex ||
oldVNode._children === childVNode._children)
) {
childVNode._nextDom = oldDom = reorderChildren(
childVNode,
oldDom,
parentDom
);
oldDom = reorderChildren(childVNode, oldDom, parentDom);
} else if (typeof childVNode.type != 'function' && !hasMatchingIndex) {
oldDom = placeChild(parentDom, newDom, oldDom);
} else if (childVNode._nextDom !== undefined) {
// Only Fragments or components that return Fragment like VNodes will
// have a non-undefined _nextDom. Continue the diff from the sibling
// of last DOM child of this child VNode
oldDom = childVNode._nextDom;

// Eagerly cleanup _nextDom. We don't need to persist the value because
// it is only used by `diffChildren` to determine where to resume the diff after
// diffing Components and Fragments. Once we store it the nextDOM local var, we
// can clean up the property
childVNode._nextDom = undefined;
} else {
oldDom = placeChild(
parentDom,
childVNode,
oldVNode,
oldChildren,
newDom,
oldDom
);
oldDom = newDom.nextSibling;
}

if (typeof newParentVNode.type == 'function') {
Expand All @@ -194,14 +220,6 @@ export function diffChildren(
// node's nextSibling.
newParentVNode._nextDom = oldDom;
}
} else if (
oldDom &&
oldVNode._dom == oldDom &&
oldDom.parentNode != parentDom
) {
// The above condition is to handle null placeholders. See test in placeholder.test.js:
// `efficiently replace null placeholders in parent rerenders`
oldDom = getDomSibling(oldVNode);
}
}

Expand All @@ -218,7 +236,8 @@ export function diffChildren(
// If the newParentVNode.__nextDom points to a dom node that is about to
// be unmounted, then get the next sibling of that vnode and set
// _nextDom to it
newParentVNode._nextDom = getLastDom(oldParentVNode).nextSibling;

newParentVNode._nextDom = oldChildren[i]._dom.nextSibling;
}

unmount(oldChildren[i], oldChildren[i]);
Expand All @@ -236,6 +255,7 @@ export function diffChildren(
function reorderChildren(childVNode, oldDom, parentDom) {
// Note: VNodes in nested suspended trees may be missing _children.
let c = childVNode._children;

let tmp = 0;
for (; c && tmp < c.length; tmp++) {
let vnode = c[tmp];
Expand All @@ -249,7 +269,7 @@ function reorderChildren(childVNode, oldDom, parentDom) {
if (typeof vnode.type == 'function') {
oldDom = reorderChildren(vnode, oldDom, parentDom);
} else {
oldDom = placeChild(parentDom, vnode, vnode, c, vnode._dom, oldDom);
oldDom = placeChild(parentDom, vnode._dom, oldDom);
}
}
}
Expand All @@ -276,81 +296,59 @@ export function toChildArray(children, out) {
return out;
}

function placeChild(
parentDom,
childVNode,
oldVNode,
oldChildren,
newDom,
oldDom
) {
let nextDom;
if (childVNode._nextDom !== undefined) {
// Only Fragments or components that return Fragment like VNodes will
// have a non-undefined _nextDom. Continue the diff from the sibling
// of last DOM child of this child VNode
nextDom = childVNode._nextDom;

// Eagerly cleanup _nextDom. We don't need to persist the value because
// it is only used by `diffChildren` to determine where to resume the diff after
// diffing Components and Fragments. Once we store it the nextDOM local var, we
// can clean up the property
childVNode._nextDom = undefined;
} else if (
oldVNode == null ||
newDom != oldDom ||
newDom.parentNode == null
) {
outer: if (oldDom == null || oldDom.parentNode !== parentDom) {
parentDom.appendChild(newDom);
nextDom = null;
} else {
// `j<oldChildrenLength; j+=2` is an alternative to `j++<oldChildrenLength/2`
for (
let sibDom = oldDom, j = 0;
(sibDom = sibDom.nextSibling) && j < oldChildren.length;
j += 1
) {
if (sibDom == newDom) {
break outer;
}
}
parentDom.insertBefore(newDom, oldDom);
nextDom = oldDom;
}
}

// If we have pre-calculated the nextDOM node, use it. Else calculate it now
// Strictly check for `undefined` here cuz `null` is a valid value of `nextDom`.
// See more detail in create-element.js:createVNode
if (nextDom !== undefined) {
oldDom = nextDom;
} else {
oldDom = newDom.nextSibling;
function placeChild(parentDom, newDom, oldDom) {
if (oldDom == null || oldDom.parentNode !== parentDom) {
parentDom.insertBefore(newDom, null);
} else if (newDom != oldDom || newDom.parentNode == null) {
parentDom.insertBefore(newDom, oldDom);
}

return oldDom;
return newDom.nextSibling;
}

/**
* @param {import('../internal').VNode} vnode
* @param {import('../internal').VNode | string} childVNode
* @param {import('../internal').VNode[]} oldChildren
* @param {number} skewedIndex
* @param {number} remainingOldChildren
* @returns {number}
*/
function getLastDom(vnode) {
if (vnode.type == null || typeof vnode.type === 'string') {
return vnode._dom;
}
function findMatchingIndex(
childVNode,
oldChildren,
skewedIndex,
remainingOldChildren
) {
const key = childVNode.key;
const type = childVNode.type;
let x = skewedIndex - 1;
let y = skewedIndex + 1;
let oldVNode = oldChildren[skewedIndex];

if (
oldVNode === null ||
(oldVNode && key == oldVNode.key && type === oldVNode.type)
) {
return skewedIndex;
} else if (remainingOldChildren > (oldVNode != null ? 1 : 0)) {
while (x >= 0 || y < oldChildren.length) {
if (x >= 0) {
oldVNode = oldChildren[x];
if (oldVNode && key == oldVNode.key && type === oldVNode.type) {
return x;
}
x--;
}

if (vnode._children) {
for (let i = vnode._children.length - 1; i >= 0; i--) {
let child = vnode._children[i];
if (child) {
let lastDom = getLastDom(child);
if (lastDom) {
return lastDom;
if (y < oldChildren.length) {
oldVNode = oldChildren[y];
if (oldVNode && key == oldVNode.key && type === oldVNode.type) {
return y;
}
y++;
}
}
}

return null;
return -1;
}
25 changes: 20 additions & 5 deletions test/_util/logCall.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,26 @@ export function logCall(obj, method) {
c += serialize(args[i]);
}

// Normalize removeChild -> remove to keep output clean and readable
const operation =
method != 'removeChild'
? `${serialize(this)}.${method}(${c})`
: `${serialize(c)}.remove()`;
let operation;
switch (method) {
case 'removeChild': {
operation = `${serialize(c)}.remove()`;
break;
}
case 'insertBefore': {
if (args[1] === null && args.length === 2) {
operation = `${serialize(this)}.appendChild(${serialize(args[0])})`;
} else {
operation = `${serialize(this)}.${method}(${c})`;
}
break;
}
default: {
operation = `${serialize(this)}.${method}(${c})`;
break;
}
}

log.push(operation);
return old.apply(this, args);
};
Expand Down
Loading

0 comments on commit d887539

Please sign in to comment.