Skip to content

Commit 4eac31d

Browse files
authored
Merge branch 'main' into lis-minimal-child-moves
2 parents 3c8bf6b + 642aed0 commit 4eac31d

3 files changed

Lines changed: 68 additions & 17 deletions

File tree

src/diff/children.js

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -126,21 +126,19 @@ export function diffChildren(
126126
firstChildDom = newDom;
127127
}
128128

129-
let shouldPlace = childVNode._flags & INSERT_VNODE;
130-
if (shouldPlace || oldVNode._children === childVNode._children) {
129+
if (childVNode._flags & INSERT_VNODE) {
131130
oldDom = insert(
132131
childVNode,
133132
oldDom,
134133
parentDom,
135-
shouldPlace,
136134
oldVNode._original == NULL
137135
);
138136

139137
// When a matched VNode is physically moved via INSERT_VNODE, its old
140138
// _dom pointer becomes a stale positional reference. Clear it so that
141139
// getDomSibling (called from nested diffs) won't return this stale
142140
// reference and mis-place subsequent DOM nodes. See #5065.
143-
if (shouldPlace && oldVNode._dom) {
141+
if (oldVNode._dom) {
144142
oldVNode._dom = NULL;
145143
}
146144
} else if (typeof childVNode.type == 'function' && result !== UNDEFINED) {
@@ -391,11 +389,10 @@ function constructNewChildrenArray(
391389
* @param {VNode} parentVNode
392390
* @param {PreactElement} oldDom
393391
* @param {PreactElement} parentDom
394-
* @param {number} shouldPlace
395392
* @param {boolean} isMounting
396393
* @returns {PreactElement}
397394
*/
398-
function insert(parentVNode, oldDom, parentDom, shouldPlace, isMounting) {
395+
function insert(parentVNode, oldDom, parentDom, isMounting) {
399396
// Note: VNodes in nested suspended trees may be missing _children.
400397
if (typeof parentVNode.type == 'function') {
401398
// Root children live in another container, they never move with the
@@ -409,23 +406,21 @@ function insert(parentVNode, oldDom, parentDom, shouldPlace, isMounting) {
409406
// children's _parent pointer to point to the newVNode (parentVNode
410407
// here).
411408
children[i]._parent = parentVNode;
412-
oldDom = insert(children[i], oldDom, parentDom, shouldPlace, false);
409+
oldDom = insert(children[i], oldDom, parentDom, false);
413410
}
414411
}
415412

416413
return oldDom;
417414
} else if (parentVNode._dom != oldDom) {
418-
if (shouldPlace) {
419-
if (oldDom && parentVNode.type && !oldDom.parentNode) {
420-
oldDom = getDomSibling(parentVNode);
421-
}
415+
if (oldDom && parentVNode.type && !oldDom.parentNode) {
416+
oldDom = getDomSibling(parentVNode);
417+
}
422418

423-
if (HAS_MOVE_BEFORE_SUPPORT && !isMounting) {
424-
// @ts-expect-error This isn't added to TypeScript lib.d.ts yet
425-
parentDom.moveBefore(parentVNode._dom, oldDom);
426-
} else {
427-
parentDom.insertBefore(parentVNode._dom, oldDom || NULL);
428-
}
419+
if (HAS_MOVE_BEFORE_SUPPORT && !isMounting) {
420+
// @ts-expect-error This isn't added to TypeScript lib.d.ts yet
421+
parentDom.moveBefore(parentVNode._dom, oldDom);
422+
} else {
423+
parentDom.insertBefore(parentVNode._dom, oldDom || NULL);
429424
}
430425
oldDom = parentVNode._dom;
431426
}

src/diff/index.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,11 @@ export function diff(
232232
commitQueue.push(c);
233233
}
234234

235+
// Skip over the retained subtree without traversing it; the
236+
// `result` branch in diffChildren picks this up as the next
237+
// oldDom.
238+
oldDom = getDomSibling(oldVNode);
239+
235240
break outer;
236241
}
237242

test/browser/lifecycles/shouldComponentUpdate.test.jsx

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,57 @@ describe('Lifecycle methods', () => {
8888
expect(ShouldNot.prototype.render).toHaveBeenCalledOnce();
8989
});
9090

91+
it('should not traverse a retained subtree after bailing out', () => {
92+
const Inner = () => (
93+
<Fragment>
94+
<span>A</span>
95+
<span>B</span>
96+
</Fragment>
97+
);
98+
99+
class Bailout extends Component {
100+
shouldComponentUpdate() {
101+
return false;
102+
}
103+
104+
render() {
105+
return <Inner />;
106+
}
107+
}
108+
109+
render(
110+
<div>
111+
<Bailout value={0} />
112+
<span>C</span>
113+
</div>,
114+
scratch
115+
);
116+
117+
const root = scratch._children;
118+
const bailoutVNode = root._children[0]._children[0];
119+
const innerVNode = bailoutVNode._children[0];
120+
const children = innerVNode._children;
121+
let reads = 0;
122+
bailoutVNode._children[0] = new Proxy(innerVNode, {
123+
get(target, property, receiver) {
124+
const value = Reflect.get(target, property, receiver);
125+
if (value === children) reads++;
126+
return value;
127+
}
128+
});
129+
130+
render(
131+
<div>
132+
<Bailout value={1} />
133+
<span>C</span>
134+
</div>,
135+
scratch
136+
);
137+
138+
expect(scratch.textContent).to.equal('ABC');
139+
expect(reads).to.equal(0);
140+
});
141+
91142
it('should reorder non-updating text children', () => {
92143
const rows = [
93144
{ id: '1', a: 5, b: 100 },

0 commit comments

Comments
 (0)