Skip to content

Commit cb3e491

Browse files
committed
Only push newly created nodes on the parent stack
Previously I was pushing nodes on the parent stack regardless of whether they were already in current or not. As a result, insertions during updates were duplicated, and nodes were added to existing parents before commit phase. Luckily we have a test that caught that.
1 parent 1cf3fd5 commit cb3e491

File tree

5 files changed

+12
-5
lines changed

5 files changed

+12
-5
lines changed

scripts/fiber/tests-failing.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
4848
src/renderers/dom/stack/client/__tests__/ReactDOM-test.js
4949
* throws in render() if the mount callback is not a function
5050
* throws in render() if the update callback is not a function
51-
* preserves focus
5251

5352
src/renderers/dom/stack/client/__tests__/ReactMount-test.js
5453
* throws when given a non-node

scripts/fiber/tests-passing.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -934,6 +934,7 @@ src/renderers/dom/stack/client/__tests__/ReactDOM-test.js
934934
* should overwrite props.children with children argument
935935
* should purge the DOM cache when removing nodes
936936
* allow React.DOM factories to be called without warnings
937+
* preserves focus
937938

938939
src/renderers/dom/stack/client/__tests__/ReactMount-test.js
939940
* should render different components in same root

src/renderers/dom/stack/client/__tests__/ReactDOM-test.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ describe('ReactDOM', () => {
224224
});
225225

226226
expect(document.activeElement.id).toBe('one');
227+
227228
ReactDOM.render(<A showTwo={true} />, container);
228229
// input2 gets added, which causes input to get blurred. Then
229230
// componentDidUpdate focuses input2 and that should make it down to here,

src/renderers/shared/fiber/ReactFiberBeginWork.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -233,9 +233,9 @@ module.exports = function<T, P, I, TI, C>(
233233
}
234234
if (!current && workInProgress.stateNode == null) {
235235
const newProps = workInProgress.pendingProps;
236-
const hostParent = getHostParentOnStack();
237236
const hostContainer = getHostContainerOnStack();
238237
const instance = createInstance(workInProgress.type, newProps, hostContainer, workInProgress);
238+
const hostParent = getHostParentOnStack();
239239
if (hostParent) {
240240
// TODO: this breaks reuse?
241241
appendInitialChild(hostParent, instance);
@@ -278,7 +278,9 @@ module.exports = function<T, P, I, TI, C>(
278278
// Abort and don't process children yet.
279279
return null;
280280
} else {
281-
pushHostParent(workInProgress.stateNode);
281+
if (!current) {
282+
pushHostParent(workInProgress.stateNode);
283+
}
282284
if (isContainerType(workInProgress.type)) {
283285
pushHostContainer(workInProgress.stateNode);
284286
}
@@ -416,7 +418,9 @@ module.exports = function<T, P, I, TI, C>(
416418

417419
// Put context on the stack because we will work on children
418420
if (isHostComponent) {
419-
pushHostParent(workInProgress.stateNode);
421+
if (!current) {
422+
pushHostParent(workInProgress.stateNode);
423+
}
420424
if (isContainerType(workInProgress.type)) {
421425
pushHostContainer(workInProgress.stateNode);
422426
}
@@ -503,9 +507,9 @@ module.exports = function<T, P, I, TI, C>(
503507
}
504508
}
505509
if (!current || workInProgress.stateNode == null) {
506-
const hostParent = getHostParentOnStack();
507510
const textInstance = createTextInstance(newText, workInProgress);
508511
workInProgress.stateNode = textInstance;
512+
const hostParent = getHostParentOnStack();
509513
if (hostParent) {
510514
// TODO: this breaks reuse?
511515
appendInitialChild(hostParent, textInstance);

src/renderers/shared/fiber/ReactFiberHostContext.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ let parentIndex = -1;
2525
const containerStack : Array = [];
2626
let containerIndex = -1;
2727

28+
// TODO: this is all likely broken with portals.
29+
2830
exports.getHostParentOnStack = function() : mixed | null {
2931
if (parentIndex === -1) {
3032
return null;

0 commit comments

Comments
 (0)