Don’t add onclick listener to React root (#13778)

Fixes #13777

As part of #11927 we introduced a regression by adding onclick handler
to the React root. This causes the whole React tree to flash when tapped
on iOS devices (for reasons I outlined in
#12989 (comment)).

To fix this, we should only apply onclick listeners to portal roots. I
verified that my proposed fix indeed works by checking out our DOM
fixtures and adding regression tests.

Strangely, I had to make changes to the DOM fixtures to see the behavior
in the first place. This seems to be caused by our normal sites (and 
thus their React root) being bigger than the viewport:

![](http://cl.ly/3f18f8b85e91/Screen%20Recording%202018-10-05%20at%2001.32%20AM.gif)

An alternative approach to finding out if we're appending to a React
root would be to add a third parameter to `appendChildToContainer` based
on the tag of the parent fiber.

Author: philipp-spiess

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

@@ -2659,4 +2659,44 @@ describe('ReactDOMComponent', () => {
       document.body.removeChild(container);
     }
   });
+
+  describe('iOS Tap Highlight', () => {
+    it('adds onclick handler to elements with onClick prop', () => {
+      const container = document.createElement('div');
+
+      const elementRef = React.createRef();
+      function Component() {
+        return <div ref={elementRef} onClick={() => {}} />;
+      }
+
+      ReactDOM.render(<Component />, container);
+      expect(typeof elementRef.current.onclick).toBe('function');
+    });
+
+    it('adds onclick handler to a portal root', () => {
+      const container = document.createElement('div');
+      const portalContainer = document.createElement('div');
+
+      function Component() {
+        return ReactDOM.createPortal(
+          <div onClick={() => {}} />,
+          portalContainer,
+        );
+      }
+
+      ReactDOM.render(<Component />, container);
+      expect(typeof portalContainer.onclick).toBe('function');
+    });
+
+    it('does not add onclick handler to the React root', () => {
+      const container = document.createElement('div');
+
+      function Component() {
+        return <div onClick={() => {}} />;
+      }
+
+      ReactDOM.render(<Component />, container);
+      expect(typeof container.onclick).not.toBe('function');
+    });
+  });
 });

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

@@ -126,7 +126,7 @@ if (__DEV__) {
 
 ReactControlledComponent.setRestoreImplementation(restoreControlledState);
 
-type DOMContainer =
+export type DOMContainer =
   | (Element & {
       _reactRootContainer: ?Root,
     })

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

@@ -36,6 +36,8 @@ import {
   DOCUMENT_FRAGMENT_NODE,
 } from '../shared/HTMLNodeType';
 
+import type {DOMContainer} from './ReactDOM';
+
 export type Type = string;
 export type Props = {
   autoFocus?: boolean,
@@ -342,7 +344,7 @@ export function appendChild(
 }
 
 export function appendChildToContainer(
-  container: Container,
+  container: DOMContainer,
   child: Instance | TextInstance,
 ): void {
   let parentNode;
@@ -358,9 +360,14 @@ export function appendChildToContainer(
   // through the React tree. However, on Mobile Safari the click would
   // never bubble through the *DOM* tree unless an ancestor with onclick
   // event exists. So we wouldn't see it and dispatch it.
-  // This is why we ensure that containers have inline onclick defined.
+  // This is why we ensure that non React root containers have inline onclick
+  // defined.
   // https://github.com/facebook/react/issues/11918
-  if (parentNode.onclick === null) {
+  const reactRootContainer = container._reactRootContainer;
+  if (
+    (reactRootContainer === null || reactRootContainer === undefined) &&
+    parentNode.onclick === null
+  ) {
     // TODO: This cast may not be sound for SVG, MathML or custom elements.
     trapClickOnNonInteractiveElement(((parentNode: any): HTMLElement));
   }