Eliminate circular dependency between MessageQueue and JSTimers modules. (#19526)

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 a8e3c7f578).

[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
This commit is contained in:
Kirill Nikolaev 2018-06-15 17:00:53 -07:00 committed by Facebook Github Bot
parent 18e9ea2112
commit 52f431b4bb
2 changed files with 16 additions and 6 deletions

View File

@ -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();
}

View File

@ -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;