@samitouri / QOS-React-2 / commits / ea2af878cc

Root API should clear non-empty roots before mounting (#18730)

* Root API should clear non-empty roots before mounting Legacy render-into-subtree API removes children from a container before rendering into it. The root API did not do this previously, but just left the children around in the document. This commit adds a new FiberRoot flag to clear a container's contents before mounting. This is done during the commit phase, to avoid multiple, observable mutations.

Brian Vaughn committed Apr 28, 2020 at 13:07 UTC ea2af878cc3fb139b0e08cf9bc4b2f4178429d69

13 files changed +110 -7

packages/react-art/src/ReactARTHostConfig.js

@@ -427,6 +427,10 @@ export function unhideTextInstance(textInstance, text): void {

427 // Noop

428 }

429

430 +export function clearContainer(container) {

431 + // TODO Implement this

432 +}

433 +

434 export function DEPRECATED_mountResponderInstance(

435 responder: ReactEventResponder<any, any>,

436 responderInstance: ReactEventResponderInstance<any, any>,

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

+32 -4

@@ -113,7 +113,28 @@ describe('ReactDOMRoot', () => {

113 expect(() => Scheduler.unstable_flushAll()).toErrorDev('Extra attributes');

114 });

115

116 - it('does not clear existing children', async () => {

116 + it('clears existing children with legacy API', async () => {

117 + container.innerHTML = '<div>a</div><div>b</div>';

118 + ReactDOM.render(

119 + <div>

120 + <span>c</span>

121 + <span>d</span>

122 + </div>,

123 + container,

124 + );

125 + expect(container.textContent).toEqual('cd');

126 + ReactDOM.render(

127 + <div>

128 + <span>d</span>

129 + <span>c</span>

130 + </div>,

131 + container,

132 + );

133 + Scheduler.unstable_flushAll();

134 + expect(container.textContent).toEqual('dc');

135 + });

136 +

137 + it('clears existing children', async () => {

138 container.innerHTML = '<div>a</div><div>b</div>';

139 const root = ReactDOM.createRoot(container);

140 root.render(

@@ -123,7 +144,7 @@ describe('ReactDOMRoot', () => {

144 </div>,

145 );

146 Scheduler.unstable_flushAll();

126 - expect(container.textContent).toEqual('abcd');

147 + expect(container.textContent).toEqual('cd');

148 root.render(

149 <div>

150 <span>d</span>

@@ -131,7 +152,7 @@ describe('ReactDOMRoot', () => {

152 </div>,

153 );

154 Scheduler.unstable_flushAll();

134 - expect(container.textContent).toEqual('abdc');

155 + expect(container.textContent).toEqual('dc');

156 });

157

158 it('throws a good message on invalid containers', () => {

@@ -220,7 +241,14 @@ describe('ReactDOMRoot', () => {

241 let unmounted = false;

242 expect(() => {

243 unmounted = ReactDOM.unmountComponentAtNode(container);

223 - }).toErrorDev('Did you mean to call root.unmount()?', {withoutStack: true});

244 + }).toErrorDev(

245 + [

246 + 'Did you mean to call root.unmount()?',

247 + // This is more of a symptom but restructuring the code to avoid it isn't worth it:

248 + "The node you're attempting to unmount was rendered by React and is not a top-level container.",

249 + ],

250 + {withoutStack: true},

251 + );

252 expect(unmounted).toBe(false);

253 Scheduler.unstable_flushAll();

254 expect(container.textContent).toEqual('Hi');

packages/react-dom/src/client/ReactDOMHostConfig.js

+11

@@ -634,6 +634,17 @@ export function unhideTextInstance(

634 textInstance.nodeValue = text;

635 }

636

637 +export function clearContainer(container: Container): void {

638 + if (container.nodeType === ELEMENT_NODE) {

639 + ((container: any): Element).textContent = '';

640 + } else if (container.nodeType === DOCUMENT_NODE) {

641 + const body = ((container: any): Document).body;

642 + if (body != null) {

643 + body.textContent = '';

644 + }

645 + }

646 +}

647 +

648 // -------------------

649 // Hydration

650 // -------------------

packages/react-native-renderer/src/ReactNativeHostConfig.js

@@ -482,6 +482,11 @@ export function unhideInstance(instance: Instance, props: Props): void {

482 );

483 }

484

485 +export function clearContainer(container: Container): void {

486 + // TODO Implement this for React Native

487 + // UIManager does not expose a "remove all" type method.

488 +}

489 +

490 export function unhideTextInstance(

491 textInstance: TextInstance,

492 text: string,

packages/react-noop-renderer/src/createReactNoop.js

@@ -155,6 +155,10 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {

155 insertInContainerOrInstanceBefore(parentInstance, child, beforeChild);

156 }

157

158 + function clearContainer(container: Container): void {

159 + container.children.splice(0);

160 + }

161 +

162 function removeChildFromContainerOrInstance(

163 parentInstance: Container | Instance,

164 child: Instance | TextInstance,

@@ -502,6 +506,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {

506 insertInContainerBefore,

507 removeChild,

508 removeChildFromContainer,

509 + clearContainer,

510

511 hideInstance(instance: Instance): void {

512 instance.hidden = true;

@@ -531,6 +536,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {

536 supportsPersistence: true,

537

538 cloneInstance,

539 + clearContainer,

540

541 createContainerChildSet(

542 container: Container,

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

+10 -1

@@ -113,6 +113,7 @@ import {

113 commitHydratedContainer,

114 commitHydratedSuspenseInstance,

115 beforeRemoveInstance,

116 + clearContainer,

117 } from './ReactFiberHostConfig';

118 import {

119 captureCommitPhaseError,

@@ -295,7 +296,15 @@ function commitBeforeMutationLifeCycles(

296 }

297 return;

298 }

298 - case HostRoot:

299 + case HostRoot: {

300 + if (supportsMutation) {

301 + if (finishedWork.effectTag & Snapshot) {

302 + const root = finishedWork.stateNode;

303 + clearContainer(root.containerInfo);

304 + }

305 + }

306 + return;

307 + }

308 case HostComponent:

309 case HostText:

310 case HostPortal:

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

+10 -1

@@ -111,6 +111,7 @@ import {

111 commitHydratedContainer,

112 commitHydratedSuspenseInstance,

113 beforeRemoveInstance,

114 + clearContainer,

115 } from './ReactFiberHostConfig';

116 import {

117 captureCommitPhaseError,

@@ -293,7 +294,15 @@ function commitBeforeMutationLifeCycles(

294 }

295 return;

296 }

296 - case HostRoot:

297 + case HostRoot: {

298 + if (supportsMutation) {

299 + if (finishedWork.effectTag & Snapshot) {

300 + const root = finishedWork.stateNode;

301 + clearContainer(root.containerInfo);

302 + }

303 + }

304 + return;

305 + }

306 case HostComponent:

307 case HostText:

308 case HostPortal:

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

+15 -1

@@ -58,7 +58,13 @@ import {

58 OffscreenComponent,

59 } from './ReactWorkTags';

60 import {NoMode, BlockingMode} from './ReactTypeOfMode';

61 -import {Ref, Update, NoEffect, DidCapture} from './ReactSideEffectTags';

61 +import {

62 + Ref,

63 + Update,

64 + NoEffect,

65 + DidCapture,

66 + Snapshot,

67 +} from './ReactSideEffectTags';

68 import invariant from 'shared/invariant';

70 import {

@@ -675,6 +681,14 @@ function completeWork(

681 // If we hydrated, then we'll need to schedule an update for

682 // the commit side-effects on the root.

683 markUpdate(workInProgress);

684 + } else if (!fiberRoot.hydrate) {

685 + // Schedule an effect to clear this container at the start of the next commit.

686 + // This handles the case of React rendering into a container with previous children.

687 + // It's also safe to do for updates too, because current.child would only be null

688 + // if the previous render was null (so the the container would already be empty).

689 + //

690 + // The additional root.hydrate check is required for hydration in legacy mode with no fallback.

691 + workInProgress.effectTag |= Snapshot;

692 }

693 }

694 updateHostContainer(workInProgress);

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

@@ -61,6 +61,7 @@ import {

61 NoEffect,

62 DidCapture,

63 Deletion,

64 + Snapshot,

65 } from './ReactSideEffectTags';

66 import invariant from 'shared/invariant';

@@ -678,6 +679,14 @@ function completeWork(

679 // If we hydrated, then we'll need to schedule an update for

680 // the commit side-effects on the root.

681 markUpdate(workInProgress);

682 + } else if (!fiberRoot.hydrate) {

683 + // Schedule an effect to clear this container at the start of the next commit.

684 + // This handles the case of React rendering into a container with previous children.

685 + // It's also safe to do for updates too, because current.child would only be null

686 + // if the previous render was null (so the the container would already be empty).

687 + //

688 + // The additional root.hydrate check is required for hydration in legacy mode with no fallback.

689 + workInProgress.effectTag |= Snapshot;

690 }

691 }

692 updateHostContainer(workInProgress);

packages/react-reconciler/src/ReactFiberHostConfigWithNoMutation.js

@@ -37,3 +37,4 @@ export const hideInstance = shim;

37 export const hideTextInstance = shim;

38 export const unhideInstance = shim;

39 export const unhideTextInstance = shim;

40 +export const clearContainer = shim;

packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js

@@ -53,6 +53,7 @@ describe('ReactFiberHostContext', () => {

53 appendChildToContainer: function() {

54 return null;

55 },

56 + clearContainer: function() {},

57 supportsMutation: true,

58 });

@@ -107,6 +108,7 @@ describe('ReactFiberHostContext', () => {

108 appendChildToContainer: function() {

109 return null;

110 },

111 + clearContainer: function() {},

112 supportsMutation: true,

113 });

114

packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js

@@ -107,6 +107,7 @@ export const updateFundamentalComponent =

107 $$$hostConfig.updateFundamentalComponent;

108 export const unmountFundamentalComponent =

109 $$$hostConfig.unmountFundamentalComponent;

110 +export const clearContainer = $$$hostConfig.clearContainer;

111

112 // -------------------

113 // Persistence

packages/react-test-renderer/src/ReactTestHostConfig.js

@@ -126,6 +126,10 @@ export function removeChild(

126 parentInstance.children.splice(index, 1);

127 }

128

129 +export function clearContainer(container: Container): void {

130 + container.children.splice(0);

131 +}

132 +

133 export function getRootHostContext(

134 rootContainerInstance: Container,

135 ): HostContext {