From 208ebed074cfbc25125e709c1645844247601f2f Mon Sep 17 00:00:00 2001
From: Rick Hanlon <rickhanlonii@fb.com>
Date: Thu, 13 Aug 2020 20:00:36 -0700
Subject: [PATCH] Switch TextInput to useLayoutEffect

Summary:
This diff fixes an issue in `TextInput` where `TextInputState.currentlyFocusedInputRef` could maintain a ref to a view that no longer exists. This issue was exposed when upgrading React, where cleanups for passive effects are deferred. This change means that `inputRef.current` would no longer reference the host view *to be* destroyed; it would be null because the view was *already destroyed*.

There are two fixes here that would independently fix the bug and fix the issue better together.

First, we convert `useEffect` to `useLayoutEffect`. `useLayoutEffect` is intended to fire synchronously after all host view mutations, and the cleanup function is intended to fire synchronously before the host view is destroyed, similar to the behavior assumed before. This change is now the correct function to use semantically. However, if we made this change without the second then any change in the order the effects fire would surface the same bug.

So second, move the `inputRefValue).blur()` call to the same effect as unregistering. This is because we currently require the `blur` effect to be called to null out `currentlyFocusedInputRef` in addition to calling `unregisterInput`. That makes the semantic ordering of effects in `TextInput` meaningful. Instead, when a TextInput is unregistered we should always `blur` to clear the `currentlyFocusedInputRef`, which will prevent dispatching events to a view that doesn't exist. If we made this change without the first then there's would be a race condition between calling blur on a TextInput and when that TextInput has been unregistered.

Changelog: [Internal]

Reviewed By: JoshuaGross

Differential Revision: D23035358

fbshipit-source-id: ab686b8046d85e2becd8b879b0b4b7e69e672754
---
 Libraries/Components/TextInput/TextInput.js | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/Libraries/Components/TextInput/TextInput.js b/Libraries/Components/TextInput/TextInput.js
index ba1dccb26f5..654765e7c79 100644
--- a/Libraries/Components/TextInput/TextInput.js
+++ b/Libraries/Components/TextInput/TextInput.js
@@ -31,7 +31,7 @@ import type {PressEvent} from '../../Types/CoreEventTypes';
 import type {HostComponent} from '../../Renderer/shims/ReactNativeTypes';
 import type {TextInputNativeCommands} from './TextInputNativeCommands';
 
-const {useEffect, useRef, useState} = React;
+const {useLayoutEffect, useRef, useState} = React;
 
 type ReactRefSetter<T> = {current: null | T, ...} | ((ref: null | T) => mixed);
 
@@ -869,7 +869,7 @@ function InternalTextInput(props: Props): React.Node {
   // This is necessary in case native updates the text and JS decides
   // that the update should be ignored and we should stick with the value
   // that we have in JS.
-  useEffect(() => {
+  useLayoutEffect(() => {
     const nativeUpdate = {};
 
     if (lastNativeText !== props.value && typeof props.value === 'string') {
@@ -912,7 +912,7 @@ function InternalTextInput(props: Props): React.Node {
     viewCommands,
   ]);
 
-  useEffect(() => {
+  useLayoutEffect(() => {
     const inputRefValue = inputRef.current;
 
     if (inputRefValue != null) {
@@ -920,19 +920,14 @@ function InternalTextInput(props: Props): React.Node {
 
       return () => {
         TextInputState.unregisterInput(inputRefValue);
+
+        if (TextInputState.currentlyFocusedInput() === inputRefValue) {
+          nullthrows(inputRefValue).blur();
+        }
       };
     }
   }, [inputRef]);
 
-  useEffect(() => {
-    // When unmounting we need to blur the input
-    return () => {
-      if (isFocused()) {
-        nullthrows(inputRef.current).blur();
-      }
-    };
-  }, [inputRef]);
-
   function clear(): void {
     if (inputRef.current != null) {
       viewCommands.setTextAndSelection(