From 52f431b4bb29062abd8ce20e01a4e60b47151a80 Mon Sep 17 00:00:00 2001
From: Kirill Nikolaev <cyriln@google.com>
Date: Fri, 15 Jun 2018 17:00:53 -0700
Subject: [PATCH] Eliminate circular dependency between MessageQueue and
 JSTimers modules. (#19526)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Summary:
This makes JSTimers actively register a callback for callImmediates.

Besides being generally tricky, circular dependency prevent compiling React Native code with bundlers that rely on concatenating module sources rather that evaluating code at the time of requiring, like Google Closure Compiler.

Sadly, Google Closure Compiler setup that prompted this change is complicated and brittle. And there are no good public tools to find circular dependencies among Haste-style modules (with unqualified require paths).

So some advice on a good test plan would be useful. Does Facebook have any tools to find circular dependencies with Haste-style requires?

FWIW, a check that worked for me was to replace all import paths in React Native from Haste style to normal relative paths (which I needed anyway) and then run [`madge`](https://www.npmjs.com/package/madge) on the code base:

```
$ ~/node_modules/.bin/madge --circular react-native/Libraries
Processed 390 files (7.4s) (81 warnings)

✖ Found 2 circular dependencies!

1) BatchedBridge/NativeModules.js > BatchedBridge/BatchedBridge.js > BatchedBridge/MessageQueue.js > Core/Timers/JSTimers.js
2) StyleSheet/flattenStyle.js > StyleSheet/StyleSheet.js
```

(The second cycle is already eliminated in https://github.com/facebook/react-native/commit/a8e3c7f5780516eb0297830632862484ad032c10).

[GENERAL] [MINOR] [MessageQueue] - MessageQueue implementation doesn't have a circular dependency on JSTimers.
Closes https://github.com/facebook/react-native/pull/19526

Reviewed By: hramos

Differential Revision: D8458755

Pulled By: yungsters

fbshipit-source-id: e753139b920ba1ad1a6db10f974c03ca195340c7
---
 Libraries/BatchedBridge/MessageQueue.js | 17 +++++++++++------
 Libraries/Core/Timers/JSTimers.js       |  5 +++++
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/Libraries/BatchedBridge/MessageQueue.js b/Libraries/BatchedBridge/MessageQueue.js
index 7cdc5fe1805..c9b750df9ef 100644
--- a/Libraries/BatchedBridge/MessageQueue.js
+++ b/Libraries/BatchedBridge/MessageQueue.js
@@ -37,9 +37,6 @@ const TRACE_TAG_REACT_APPS = 1 << 17;
 
 const DEBUG_INFO_LIMIT = 32;
 
-// Work around an initialization order issue
-let JSTimers = null;
-
 class MessageQueue {
   _lazyCallableModules: {[key: string]: (void) => Object};
   _queue: [number[], number[], any[], number];
@@ -48,6 +45,7 @@ class MessageQueue {
   _callID: number;
   _lastFlush: number;
   _eventLoopStartTime: number;
+  _immediatesCallback: ?() => void;
 
   _debugInfo: {[number]: [number, number]};
   _remoteModuleTable: {[number]: string};
@@ -63,6 +61,7 @@ class MessageQueue {
     this._callID = 0;
     this._lastFlush = 0;
     this._eventLoopStartTime = new Date().getTime();
+    this._immediatesCallback = null;
 
     if (__DEV__) {
       this._debugInfo = {};
@@ -279,6 +278,13 @@ class MessageQueue {
     }
   }
 
+  // For JSTimers to register its callback. Otherwise a circular dependency
+  // between modules is introduced. Note that only one callback may be
+  // registered at a time.
+  setImmediatesCallback(fn: () => void) {
+    this._immediatesCallback = fn;
+  }
+
   /**
    * Private methods
    */
@@ -310,10 +316,9 @@ class MessageQueue {
 
   __callImmediates() {
     Systrace.beginEvent('JSTimers.callImmediates()');
-    if (!JSTimers) {
-      JSTimers = require('JSTimers');
+    if (this._immediatesCallback != null) {
+      this._immediatesCallback();
     }
-    JSTimers.callImmediates();
     Systrace.endEvent();
   }
 
diff --git a/Libraries/Core/Timers/JSTimers.js b/Libraries/Core/Timers/JSTimers.js
index dbd23f807f8..7cffb4e3d06 100644
--- a/Libraries/Core/Timers/JSTimers.js
+++ b/Libraries/Core/Timers/JSTimers.js
@@ -14,6 +14,7 @@ const Systrace = require('Systrace');
 
 const invariant = require('fbjs/lib/invariant');
 const {Timing} = require('NativeModules');
+const BatchedBridge = require('BatchedBridge');
 
 import type {ExtendedError} from 'parseErrorStack';
 
@@ -494,4 +495,8 @@ if (!Timing) {
   ExportedJSTimers = JSTimers;
 }
 
+BatchedBridge.setImmediatesCallback(
+  ExportedJSTimers.callImmediates.bind(ExportedJSTimers),
+);
+
 module.exports = ExportedJSTimers;