Skip to content

Commit ac533fd

Browse files
author
Brian Vaughn
authored
Prevent stale legacy root from clearing a container (DRAFT) (#18792)
* Don't clear a container because of a stale legacy root * Added test repro for FB error
1 parent 5b89d35 commit ac533fd

File tree

3 files changed

+62
-4
lines changed

3 files changed

+62
-4
lines changed

packages/react-dom/src/__tests__/ReactDOMFiber-test.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1280,4 +1280,36 @@ describe('ReactDOMFiber', () => {
12801280
);
12811281
expect(didCallOnChange).toBe(true);
12821282
});
1283+
1284+
it('unmounted legacy roots should never clear newer root content from a container', () => {
1285+
const ref = React.createRef();
1286+
1287+
function OldApp() {
1288+
const hideOnFocus = () => {
1289+
// This app unmounts itself inside of a focus event.
1290+
ReactDOM.unmountComponentAtNode(container);
1291+
};
1292+
1293+
return (
1294+
<button onFocus={hideOnFocus} ref={ref}>
1295+
old
1296+
</button>
1297+
);
1298+
}
1299+
1300+
function NewApp() {
1301+
return <button ref={ref}>new</button>;
1302+
}
1303+
1304+
ReactDOM.render(<OldApp />, container);
1305+
ref.current.focus();
1306+
1307+
ReactDOM.render(<NewApp />, container);
1308+
1309+
// Calling focus again will flush previously scheduled discerete work for the old root-
1310+
// but this should not clear out the newly mounted app.
1311+
ref.current.focus();
1312+
1313+
expect(container.textContent).toBe('new');
1314+
});
12831315
});

packages/react-reconciler/src/ReactFiberCompleteWork.new.js

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -687,8 +687,21 @@ function completeWork(
687687
// It's also safe to do for updates too, because current.child would only be null
688688
// if the previous render was null (so the the container would already be empty).
689689
//
690-
// The additional root.hydrate check is required for hydration in legacy mode with no fallback.
691-
workInProgress.effectTag |= Snapshot;
690+
// The additional root.hydrate check above is required for hydration in legacy mode with no fallback.
691+
//
692+
// The root container check below also avoids a potential legacy mode problem
693+
// where unmounting from a container then rendering into it again
694+
// can sometimes cause the container to be cleared after the new render.
695+
const containerInfo = fiberRoot.containerInfo;
696+
const legacyRootContainer =
697+
containerInfo == null ? null : containerInfo._reactRootContainer;
698+
if (
699+
legacyRootContainer == null ||
700+
legacyRootContainer._internalRoot == null ||
701+
legacyRootContainer._internalRoot === fiberRoot
702+
) {
703+
workInProgress.effectTag |= Snapshot;
704+
}
692705
}
693706
}
694707
updateHostContainer(workInProgress);

packages/react-reconciler/src/ReactFiberCompleteWork.old.js

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -685,8 +685,21 @@ function completeWork(
685685
// It's also safe to do for updates too, because current.child would only be null
686686
// if the previous render was null (so the the container would already be empty).
687687
//
688-
// The additional root.hydrate check is required for hydration in legacy mode with no fallback.
689-
workInProgress.effectTag |= Snapshot;
688+
// The additional root.hydrate check above is required for hydration in legacy mode with no fallback.
689+
//
690+
// The root container check below also avoids a potential legacy mode problem
691+
// where unmounting from a container then rendering into it again
692+
// can sometimes cause the container to be cleared after the new render.
693+
const containerInfo = fiberRoot.containerInfo;
694+
const legacyRootContainer =
695+
containerInfo == null ? null : containerInfo._reactRootContainer;
696+
if (
697+
legacyRootContainer == null ||
698+
legacyRootContainer._internalRoot == null ||
699+
legacyRootContainer._internalRoot === fiberRoot
700+
) {
701+
workInProgress.effectTag |= Snapshot;
702+
}
690703
}
691704
}
692705
updateHostContainer(workInProgress);

0 commit comments

Comments
 (0)