Skip to content

Commit

Permalink
Ensure updates are applied when diffInCommitPhase is on (facebook#26977)
Browse files Browse the repository at this point in the history
When we diffInCommitPhase there's no updatePayload, which caused no
update to be applied.

This is unfortunate because it would've been a lot easier to see this
oversight if we didn't have to support both flags.

I also carified that updateHostComponent is unnecessary in the new flag.
We reuse updateHostComponent for HostSingleton and HostHoistables since
it has a somewhat complex path but that means you have to remember when
editing updateHostComponent that it's not just used for that tag.
Luckily with the new flag, this is actually unnecessary since we just
need to mark it for update if any props have changed and then we diff it
later.
  • Loading branch information
sebmarkbage authored and AndyPengc12 committed Apr 15, 2024
1 parent 9b7afc1 commit 8fb9bb0
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 19 deletions.
21 changes: 21 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFloat-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7084,5 +7084,26 @@ background-color: green;
</html>,
);
});

// @gate enableFloat
it('can update title tags', async () => {
const root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(<title data-foo="foo">a title</title>);
});
await waitForAll([]);

expect(getMeaningfulChildren(document.head)).toEqual(
<title data-foo="foo">a title</title>,
);

await act(() => {
root.render(<title data-foo="bar">another title</title>);
});
await waitForAll([]);
expect(getMeaningfulChildren(document.head)).toEqual(
<title data-foo="bar">another title</title>,
);
});
});
});
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -2692,7 +2692,7 @@ function commitMutationEffectsOnFiber(
const updatePayload: null | UpdatePayload =
(finishedWork.updateQueue: any);
finishedWork.updateQueue = null;
if (updatePayload !== null) {
if (updatePayload !== null || diffInCommitPhase) {
try {
commitUpdate(
finishedWork.stateNode,
Expand Down
49 changes: 31 additions & 18 deletions packages/react-reconciler/src/ReactFiberCompleteWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -1160,17 +1160,23 @@ function completeWork(
return null;
} else {
// This is a Hoistable Instance
//
// We may have props to update on the Hoistable instance. We use the
// updateHostComponent path becuase it produces the update queue
// we need for Hoistables.
updateHostComponent(
current,
workInProgress,
type,
newProps,
renderLanes,
);
// We may have props to update on the Hoistable instance.
if (diffInCommitPhase && supportsMutation) {
const oldProps = current.memoizedProps;
if (oldProps !== newProps) {
markUpdate(workInProgress);
}
} else {
// We use the updateHostComponent path becuase it produces
// the update queue we need for Hoistables.
updateHostComponent(
current,
workInProgress,
type,
newProps,
renderLanes,
);
}

// This must come at the very end of the complete phase.
bubbleProperties(workInProgress);
Expand All @@ -1192,13 +1198,20 @@ function completeWork(
const rootContainerInstance = getRootHostContainer();
const type = workInProgress.type;
if (current !== null && workInProgress.stateNode != null) {
updateHostComponent(
current,
workInProgress,
type,
newProps,
renderLanes,
);
if (diffInCommitPhase && supportsMutation) {
const oldProps = current.memoizedProps;
if (oldProps !== newProps) {
markUpdate(workInProgress);
}
} else {
updateHostComponent(
current,
workInProgress,
type,
newProps,
renderLanes,
);
}

if (current.ref !== workInProgress.ref) {
markRef(workInProgress);
Expand Down

0 comments on commit 8fb9bb0

Please sign in to comment.