Skip to content

Commit

Permalink
Do typeof string check before looking for String constructor (#4198)
Browse files Browse the repository at this point in the history
In `constructNewChildrenArray` we look for the `String` constructor to support #4151 (i.e. strings created with `new String`). However, doing this check first leads to a megamorphic deopt in `constructorNewChildrenArray` since V8 has many different internal types for strings, leading to a megamorphic access for `childVNode.constructor` in the common case (normal strings and VNodes).

This PR adds back the `typeof childVNode == 'string'` check to catch normal strings first before falling back to `childVNode.constructor == String` to check for manually constructed strings.

Commits:
* Add back typeof string check in diffChildren (+5 B)
* Store matching oldVNode in variable in constructNewChildrenArray  (-4 B)
* Inline insertion checks into skew block (-3 B)
  • Loading branch information
andrewiggins authored Nov 8, 2023
1 parent f269b62 commit b4a1cc2
Showing 1 changed file with 18 additions and 16 deletions.
34 changes: 18 additions & 16 deletions src/diff/children.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,11 @@ function constructNewChildrenArray(newParentVNode, renderResult, oldChildren) {
// or we are rendering a component (e.g. setState) copy the oldVNodes so it can have
// it's own DOM & etc. pointers
else if (
childVNode.constructor == String ||
typeof childVNode == 'string' ||
typeof childVNode == 'number' ||
// eslint-disable-next-line valid-typeof
typeof childVNode == 'bigint'
typeof childVNode == 'bigint' ||
childVNode.constructor == String
) {
childVNode = newParentVNode._children[i] = createVNode(
null,
Expand Down Expand Up @@ -269,25 +270,29 @@ function constructNewChildrenArray(newParentVNode, renderResult, oldChildren) {
// final index after using this property to get the oldVNode
childVNode._index = matchingIndex;

oldVNode = null;
if (matchingIndex !== -1) {
oldVNode = oldChildren[matchingIndex];
remainingOldChildren--;
if (oldChildren[matchingIndex]) {
oldChildren[matchingIndex]._flags |= MATCHED;
if (oldVNode) {
oldVNode._flags |= MATCHED;
}
}

// Here, we define isMounting for the purposes of the skew diffing
// algorithm. Nodes that are unsuspending are considered mounting and we detect
// this by checking if oldVNode._original === null
const isMounting =
matchingIndex === -1 ||
oldChildren[matchingIndex] == null ||
oldChildren[matchingIndex]._original === null;
const isMounting = oldVNode == null || oldVNode._original === null;

if (isMounting) {
if (matchingIndex == -1) {
skew--;
}

// If we are mounting a DOM VNode, mark it for insertion
if (typeof childVNode.type != 'function') {
childVNode._flags |= INSERT_VNODE;
}
} else if (matchingIndex !== skewedIndex) {
if (matchingIndex === skewedIndex + 1) {
skew++;
Expand All @@ -307,15 +312,12 @@ function constructNewChildrenArray(newParentVNode, renderResult, oldChildren) {
} else {
skew = 0;
}
}

// Move this VNode's DOM if the original index (matchingIndex) doesn't match
// the new skew index (i + new skew) or it's a mounting DOM VNode
if (
matchingIndex !== i + skew ||
(typeof childVNode.type != 'function' && isMounting)
) {
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

0 comments on commit b4a1cc2

Please sign in to comment.