Detect React roots created by createRoot

Fixes facebook#14535 Related to facebook#13778 Alternative to facebook#14681 As @gaearon already noted, we can not rely on a container node having a `_reactRootContainer` to detect a React Root since the `createRoot()` API will not set it. Furthermore, the `createRoot()` API is currently only setting a property on the container in DEV mode. We could: 1. Set a property in prod as well. 2. Pass in more information into the `appendChildToContainer()` config. This PR is an attempt to implement 1. It feels better than [the other approach](facebook#14681) since we don't need to change the reconciler API.
philipp-spiess · Jan 24, 2019 · adf0948 · adf0948
1 parent 6cb2677
commit adf0948
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 13 deletions.
diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js
@@ -2688,7 +2688,21 @@ describe('ReactDOMComponent', () => {
       expect(typeof portalContainer.onclick).toBe('function');
     });
 
-    it('does not add onclick handler to the React root', () => {
+    it('does not add onclick handler to a React root', () => {
+      const container = document.createElement('div');
+
+      function Component() {
+        return <div onClick={() => {}} />;
+      }
+
+      const root = ReactDOM.unstable_createRoot(container);
+      root.render(<Component />);
+      jest.runAllTimers();
+
+      expect(typeof container.onclick).not.toBe('function');
+    });
+
+    it('does not add onclick handler to a legacy React root', () => {
       const container = document.createElement('div');
 
       function Component() {

diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js
@@ -159,11 +159,11 @@ setRestoreImplementation(restoreControlledState);
 export type DOMContainer =
   | (Element & {
       _reactRootContainer: ?Root,
-      _reactHasBeenPassedToCreateRootDEV: ?boolean,
+      _reactHasBeenPassedToCreateRoot: ?boolean,
     })
   | (Document & {
       _reactRootContainer: ?Root,
-      _reactHasBeenPassedToCreateRootDEV: ?boolean,
+      _reactHasBeenPassedToCreateRoot: ?boolean,
     });
 
 type Batch = FiberRootBatch & {
@@ -653,7 +653,7 @@ const ReactDOM: Object = {
     );
     if (__DEV__) {
       warningWithoutStack(
-        !container._reactHasBeenPassedToCreateRootDEV,
+        !container._reactHasBeenPassedToCreateRoot,
         'You are calling ReactDOM.hydrate() on a container that was previously ' +
           'passed to ReactDOM.%s(). This is not supported. ' +
           'Did you mean to call root.render(element, {hydrate: true})?',
@@ -681,7 +681,7 @@ const ReactDOM: Object = {
     );
     if (__DEV__) {
       warningWithoutStack(
-        !container._reactHasBeenPassedToCreateRootDEV,
+        !container._reactHasBeenPassedToCreateRoot,
         'You are calling ReactDOM.render() on a container that was previously ' +
           'passed to ReactDOM.%s(). This is not supported. ' +
           'Did you mean to call root.render(element)?',
@@ -728,7 +728,7 @@ const ReactDOM: Object = {
 
     if (__DEV__) {
       warningWithoutStack(
-        !container._reactHasBeenPassedToCreateRootDEV,
+        !container._reactHasBeenPassedToCreateRoot,
         'You are calling ReactDOM.unmountComponentAtNode() on a container that was previously ' +
           'passed to ReactDOM.%s(). This is not supported. Did you mean to call root.unmount()?',
         enableStableConcurrentModeAPIs ? 'createRoot' : 'unstable_createRoot',
@@ -846,8 +846,8 @@ function createRoot(container: DOMContainer, options?: RootOptions): ReactRoot {
         'passed to ReactDOM.render(). This is not supported.',
       enableStableConcurrentModeAPIs ? 'createRoot' : 'unstable_createRoot',
     );
-    container._reactHasBeenPassedToCreateRootDEV = true;
   }
+  container._reactHasBeenPassedToCreateRoot = true;
   const hydrate = options != null && options.hydrate === true;
   return new ReactRoot(container, true, hydrate);
 }

diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js
@@ -379,8 +379,11 @@ export function appendChildToContainer(
   // defined.
   // https://github.com/facebook/react/issues/11918
   const reactRootContainer = container._reactRootContainer;
+  const hasBeenPassedToCreateRoot = container._reactHasBeenPassedToCreateRoot;
   if (
     (reactRootContainer === null || reactRootContainer === undefined) &&
+    (hasBeenPassedToCreateRoot === null ||
+      hasBeenPassedToCreateRoot === undefined) &&
     parentNode.onclick === null
   ) {
     // TODO: This cast may not be sound for SVG, MathML or custom elements.

diff --git a/packages/react-dom/src/fire/ReactFire.js b/packages/react-dom/src/fire/ReactFire.js
@@ -164,11 +164,11 @@ setRestoreImplementation(restoreControlledState);
 export type DOMContainer =
   | (Element & {
       _reactRootContainer: ?Root,
-      _reactHasBeenPassedToCreateRootDEV: ?boolean,
+      _reactHasBeenPassedToCreateRoot: ?boolean,
     })
   | (Document & {
       _reactRootContainer: ?Root,
-      _reactHasBeenPassedToCreateRootDEV: ?boolean,
+      _reactHasBeenPassedToCreateRoot: ?boolean,
     });
 
 type Batch = FiberRootBatch & {
@@ -658,7 +658,7 @@ const ReactDOM: Object = {
     );
     if (__DEV__) {
       warningWithoutStack(
-        !container._reactHasBeenPassedToCreateRootDEV,
+        !container._reactHasBeenPassedToCreateRoot,
         'You are calling ReactDOM.hydrate() on a container that was previously ' +
           'passed to ReactDOM.%s(). This is not supported. ' +
           'Did you mean to call root.render(element, {hydrate: true})?',
@@ -686,7 +686,7 @@ const ReactDOM: Object = {
     );
     if (__DEV__) {
       warningWithoutStack(
-        !container._reactHasBeenPassedToCreateRootDEV,
+        !container._reactHasBeenPassedToCreateRoot,
         'You are calling ReactDOM.render() on a container that was previously ' +
           'passed to ReactDOM.%s(). This is not supported. ' +
           'Did you mean to call root.render(element)?',
@@ -733,7 +733,7 @@ const ReactDOM: Object = {
 
     if (__DEV__) {
       warningWithoutStack(
-        !container._reactHasBeenPassedToCreateRootDEV,
+        !container._reactHasBeenPassedToCreateRoot,
         'You are calling ReactDOM.unmountComponentAtNode() on a container that was previously ' +
           'passed to ReactDOM.%s(). This is not supported. Did you mean to call root.unmount()?',
         enableStableConcurrentModeAPIs ? 'createRoot' : 'unstable_createRoot',
@@ -851,8 +851,8 @@ function createRoot(container: DOMContainer, options?: RootOptions): ReactRoot {
         'passed to ReactDOM.render(). This is not supported.',
       enableStableConcurrentModeAPIs ? 'createRoot' : 'unstable_createRoot',
     );
-    container._reactHasBeenPassedToCreateRootDEV = true;
   }
+  container._reactHasBeenPassedToCreateRoot = true;
   const hydrate = options != null && options.hydrate === true;
   return new ReactRoot(container, true, hydrate);
 }