Skip to content

Get rid of eager unmounting #4484

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

Merged
JoviDeCroock merged 1 commit into from
Aug 28, 2024
Merged

Get rid of eager unmounting #4484

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
58 changes: 16 additions & 42 deletions src/diff/children.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,52 +227,24 @@ function constructNewChildrenArray(newParentVNode, renderResult, oldChildren) {
childVNode = newParentVNode._children[i] = childVNode;
}

const skewedIndex = i + skew;

// Handle unmounting null placeholders, i.e. VNode => null in unkeyed children
if (childVNode == null) {
oldVNode = oldChildren[skewedIndex];
if (
oldVNode &&
oldVNode.key == null &&
oldVNode._dom &&
(oldVNode._flags & MATCHED) === 0
) {
if (oldVNode._dom == newParentVNode._nextDom) {
newParentVNode._nextDom = getDomSibling(oldVNode);
}

unmount(oldVNode, oldVNode, false);

// Explicitly nullify this position in oldChildren instead of just
// setting `_match=true` to prevent other routines (e.g.
// `findMatchingIndex` or `getDomSibling`) from thinking VNodes or DOM
// nodes in this position are still available to be used in diffing when
// they have actually already been unmounted. For example, by only
// setting `_match=true` here, the unmounting loop later would attempt
// to unmount this VNode again seeing `_match==true`. Further,
// getDomSibling doesn't know about _match and so would incorrectly
// assume DOM nodes in this subtree are mounted and usable.
oldChildren[skewedIndex] = null;
remainingOldChildren--;
}
continue;
}

const skewedIndex = i + skew;
childVNode._parent = newParentVNode;
childVNode._depth = newParentVNode._depth + 1;

const matchingIndex = findMatchingIndex(
// Temporarily store the matchingIndex on the _index property so we can pull
// out the oldVNode in diffChildren. We'll override this to the VNode's
// final index after using this property to get the oldVNode
const matchingIndex = (childVNode._index = findMatchingIndex(
childVNode,
oldChildren,
skewedIndex,
remainingOldChildren
);

// Temporarily store the matchingIndex on the _index property so we can pull
// out the oldVNode in diffChildren. We'll override this to the VNode's
// final index after using this property to get the oldVNode
childVNode._index = matchingIndex;
));

oldVNode = null;
if (matchingIndex !== -1) {
Expand Down Expand Up @@ -318,16 +290,18 @@ function constructNewChildrenArray(newParentVNode, renderResult, oldChildren) {
skew--;
} else if (matchingIndex == skewedIndex + 1) {
skew++;
} else if (matchingIndex > skewedIndex) {
skew--;
} else {
skew++;
}
if (matchingIndex > skewedIndex) {
skew--;
} else {
skew++;
}

// Move this VNode's DOM if the original index (matchingIndex) doesn't
// match the new skew index (i + new skew)
if (matchingIndex !== i + skew) {
childVNode._flags |= INSERT_VNODE;
// Move this VNode's DOM if the original index (matchingIndex) doesn't
// match the new skew index (i + new skew)
if (matchingIndex !== i + skew) {
childVNode._flags |= INSERT_VNODE;
}
}
}
}
Expand Down