Skip to content

Commit 8d397b4

Browse files
sebmarkbagefacebook-github-bot-5
authored andcommitted
Decouple Module System from Native Calls
Summary: The JavaScript ecosystem doesn't have the notion of a built-in native module loader. Even Node is decoupled from its module loader. The module loader system is just JS that runs on top of the global `process` object which has all the built-in goodies. Additionally there is no such thing as a global require. That is something unique to our providesModule system. In other module systems such as node, every require is contextual. Even registered npm names are localized by version. The only global namespace that is accessible to the host environment is the global object. Normally module systems attaches itself onto the hooks provided by the host environment on the global object. Currently, we have two forms of dispatch that reaches directly into the module system. executeJSCall which reaches directly into require. Everything now calls through the BatchedBridge module (except one RCTLog edge case that I will fix). I propose that the executors calls directly onto `BatchedBridge` through an instance on the global so that everything is guaranteed to go through it. It becomes the main communication hub. I also propose that we drop the dynamic requires inside of MessageQueue/BatchBridge and instead have the modules register themselves with the bridge. executeJSCall was originally modeled after the XHP equivalent. The XHP equivalent was designed that way because the act of doing the call was the thing that defined a dependency on the module from the page. However, that is not how React Native works. The JS side is driving the dependencies by virtue of requiring new modules and frameworks and the existence of dependencies is driven by the JS side, so this design doesn't make as much sense. The main driver for this is to be able to introduce a new module system like Prepack's module system. However, it also unlocks the possibility to do dead module elimination even in our current module system. It is currently not possible because we don't know which module might be called from native. Since the module system now becomes decoupled we could publish all our providesModule modules as npm/CommonJS modules using a rewrite script. That's what React Core does. That way people could use any CommonJS bundler such as Webpack, Closure Compiler, Rollup or some new innovation to create a JS bundle. This diff expands the executeJSCalls to the BatchedBridge's three individual pieces to make them first class instead of being dynamic. This removes one layer of abstraction. Hopefully we can also remove more of the things that register themselves with the BatchedBridge (various EventEmitters) and instead have everything go through the public protocol. ReactMethod/RCT_EXPORT_METHOD. public Reviewed By: vjeux Differential Revision: D2717535 fb-gh-sync-id: 70114f05483124f5ac5c4570422bb91a60a727f6
1 parent 99bba8c commit 8d397b4

38 files changed

Lines changed: 545 additions & 274 deletions

Examples/UIExplorer/UIExplorerUnitTests/RCTBridgeTests.m

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,22 @@ - (BOOL)isValid
5454
return YES;
5555
}
5656

57-
- (void)executeJSCall:(__unused NSString *)name
58-
method:(__unused NSString *)method
59-
arguments:(__unused NSArray *)arguments
60-
callback:(RCTJavaScriptCallback)onComplete
57+
- (void)flushedQueue:(RCTJavaScriptCallback)onComplete
58+
{
59+
onComplete(nil, nil);
60+
}
61+
62+
- (void)callFunctionOnModule:(NSString *)module
63+
method:(NSString *)method
64+
arguments:(NSArray *)args
65+
callback:(RCTJavaScriptCallback)onComplete
66+
{
67+
onComplete(nil, nil);
68+
}
69+
70+
- (void)invokeCallbackID:(NSNumber *)cbID
71+
arguments:(NSArray *)args
72+
callback:(RCTJavaScriptCallback)onComplete
6173
{
6274
onComplete(nil, nil);
6375
}

Examples/UIExplorer/UIExplorerUnitTests/RCTContextExecutorTests.m

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,13 @@ - (void)testJavaScriptCallSpeed
123123
} \
124124
} \
125125
}; \
126+
var Bridge = { \
127+
callFunctionReturnFlushedQueue: function(module, method, args) { \
128+
modules[module].apply(modules[module], args); \
129+
} \
130+
}; \
126131
function require(module) { \
127-
return modules[module]; \
132+
return Bridge; \
128133
} \
129134
";
130135

@@ -138,12 +143,11 @@ function require(module) { \
138143
for (int j = 0; j < runs; j++) {
139144
@autoreleasepool {
140145
double start = _get_time_nanoseconds();
141-
[_executor executeJSCall:@"module"
142-
method:@"method"
143-
arguments:params
144-
callback:^(id json, __unused NSError *unused) {
145-
XCTAssert([json isEqual:@YES], @"Invalid return");
146-
}];
146+
[_executor callFunctionOnModule:@"module"
147+
method:@"method"
148+
arguments:params
149+
callback:^(__unused id json, __unused NSError *unused) {
150+
}];
147151
double run = _get_time_nanoseconds() - start;
148152
if ((j % frequency) == frequency - 1) { // Warmup
149153
total += run;

IntegrationTests/LoggingTestModule.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@
1010
*/
1111
'use strict';
1212

13+
var BatchedBridge = require('BatchedBridge');
14+
1315
var warning = require('warning');
1416
var invariant = require('invariant');
1517

16-
module.exports = {
18+
var LoggingTestModule = {
1719
logToConsole: function(str) {
1820
console.log(str);
1921
},
@@ -30,3 +32,10 @@ module.exports = {
3032
throw new Error(str);
3133
}
3234
};
35+
36+
BatchedBridge.registerCallableModule(
37+
'LoggingTestModule',
38+
LoggingTestModule
39+
);
40+
41+
module.exports = LoggingTestModule;

Libraries/AppRegistry/AppRegistry.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
*/
1212
'use strict';
1313

14+
var BatchedBridge = require('BatchedBridge');
15+
var ReactNative = require('ReactNative');
16+
1417
var invariant = require('invariant');
1518
var renderApplication = require('renderApplication');
1619

@@ -37,6 +40,10 @@ type AppConfig = {
3740
* for the app and then actually run the app when it's ready by invoking
3841
* `AppRegistry.runApplication`.
3942
*
43+
* To "stop" an application when a view should be destroyed, call
44+
* `AppRegistry.unmountApplicationComponentAtRootTag` with the tag that was
45+
* pass into `runApplication`. These should always be used as a pair.
46+
*
4047
* `AppRegistry` should be `require`d early in the `require` sequence to make
4148
* sure the JS execution environment is setup before other modules are
4249
* `require`d.
@@ -87,6 +94,16 @@ var AppRegistry = {
8794
);
8895
runnables[appKey].run(appParameters);
8996
},
97+
98+
unmountApplicationComponentAtRootTag: function(rootTag : number) {
99+
ReactNative.unmountComponentAtNodeAndRemoveContainer(rootTag);
100+
},
101+
90102
};
91103

104+
BatchedBridge.registerCallableModule(
105+
'AppRegistry',
106+
AppRegistry
107+
);
108+
92109
module.exports = AppRegistry;

Libraries/BatchedBridge/BatchedBridge.js

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,27 @@
1010
*/
1111
'use strict';
1212

13-
let MessageQueue = require('MessageQueue');
13+
const MessageQueue = require('MessageQueue');
1414

15-
let BatchedBridge = new MessageQueue(
15+
const BatchedBridge = new MessageQueue(
1616
__fbBatchedBridgeConfig.remoteModuleConfig,
1717
__fbBatchedBridgeConfig.localModulesConfig,
1818
);
1919

20+
// TODO: Move these around to solve the cycle in a cleaner way.
21+
22+
const BridgeProfiling = require('BridgeProfiling');
23+
const JSTimersExecution = require('JSTimersExecution');
24+
25+
BatchedBridge.registerCallableModule('BridgeProfiling', BridgeProfiling);
26+
BatchedBridge.registerCallableModule('JSTimersExecution', JSTimersExecution);
27+
28+
// Wire up the batched bridge on the global object so that we can call into it.
29+
// Ideally, this would be the inverse relationship. I.e. the native environment
30+
// provides this global directly with its script embedded. Then this module
31+
// would export it. A possible fix would be to trim the dependencies in
32+
// MessageQueue to its minimal features and embed that in the native runtime.
33+
34+
Object.defineProperty(global, '__fbBatchedBridge', { value: BatchedBridge });
35+
2036
module.exports = BatchedBridge;

Libraries/BatchedBridge/BatchedBridgedModules/RCTEventEmitter.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,13 @@
1111
*/
1212
'use strict';
1313

14+
var BatchedBridge = require('BatchedBridge');
1415
var ReactNativeEventEmitter = require('ReactNativeEventEmitter');
1516

17+
BatchedBridge.registerCallableModule(
18+
'RCTEventEmitter',
19+
ReactNativeEventEmitter
20+
);
21+
1622
// Completely locally implemented - no native hooks.
1723
module.exports = ReactNativeEventEmitter;

Libraries/DebugComponentHierarchy/RCTDebugComponentOwnership.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
'use strict';
1717

18+
var BatchedBridge = require('BatchedBridge');
1819
var DebugComponentOwnershipModule = require('NativeModules').DebugComponentOwnershipModule;
1920
var InspectorUtils = require('InspectorUtils');
2021
var ReactNativeTagHandles = require('ReactNativeTagHandles');
@@ -35,7 +36,7 @@ function getRootTagForTag(tag: number): ?number {
3536
return ReactNativeTagHandles.rootNodeIDToTag[rootID];
3637
}
3738

38-
module.exports = {
39+
var RCTDebugComponentOwnership = {
3940

4041
/**
4142
* Asynchronously returns the owner hierarchy as an array of strings. Request id is
@@ -53,3 +54,10 @@ module.exports = {
5354
DebugComponentOwnershipModule.receiveOwnershipHierarchy(requestID, tag, ownerHierarchy);
5455
},
5556
};
57+
58+
BatchedBridge.registerCallableModule(
59+
'RCTDebugComponentOwnership',
60+
RCTDebugComponentOwnership
61+
);
62+
63+
module.exports = RCTDebugComponentOwnership;

Libraries/Device/RCTDeviceEventEmitter.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,13 @@
1212
'use strict';
1313

1414
var EventEmitter = require('EventEmitter');
15+
var BatchedBridge = require('BatchedBridge');
1516

1617
var RCTDeviceEventEmitter = new EventEmitter();
1718

19+
BatchedBridge.registerCallableModule(
20+
'RCTDeviceEventEmitter',
21+
RCTDeviceEventEmitter
22+
);
23+
1824
module.exports = RCTDeviceEventEmitter;

Libraries/NativeApp/RCTNativeAppEventEmitter.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,14 @@
1111
*/
1212
'use strict';
1313

14+
var BatchedBridge = require('BatchedBridge');
1415
var EventEmitter = require('EventEmitter');
1516

1617
var RCTNativeAppEventEmitter = new EventEmitter();
1718

19+
BatchedBridge.registerCallableModule(
20+
'RCTNativeAppEventEmitter',
21+
RCTNativeAppEventEmitter
22+
);
23+
1824
module.exports = RCTNativeAppEventEmitter;

Libraries/Utilities/MessageQueue.js

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,10 @@ var guard = (fn) => {
4343

4444
class MessageQueue {
4545

46-
constructor(remoteModules, localModules, customRequire) {
46+
constructor(remoteModules, localModules) {
4747
this.RemoteModules = {};
4848

49-
this._require = customRequire || require;
49+
this._callableModules = {};
5050
this._queue = [[],[],[]];
5151
this._moduleTable = {};
5252
this._methodTable = {};
@@ -154,8 +154,22 @@ class MessageQueue {
154154
if (__DEV__ && SPY_MODE) {
155155
console.log('N->JS : ' + module + '.' + method + '(' + JSON.stringify(args) + ')');
156156
}
157-
module = this._require(module);
158-
module[method].apply(module, args);
157+
var moduleMethods = this._callableModules[module];
158+
if (!moduleMethods) {
159+
// TODO: Register all the remaining test modules and make this an invariant. #9317773
160+
// Fallback to require temporarily. A follow up diff will clean up the remaining
161+
// modules and make this an invariant.
162+
console.warn('Module is not registered:', module);
163+
moduleMethods = require(module);
164+
/*
165+
invariant(
166+
!!moduleMethods,
167+
'Module %s is not a registered callable module.',
168+
module
169+
);
170+
*/
171+
}
172+
moduleMethods[method].apply(moduleMethods, args);
159173
BridgeProfiling.profileEnd();
160174
}
161175

@@ -337,6 +351,10 @@ class MessageQueue {
337351
return fn;
338352
}
339353

354+
registerCallableModule(name, methods) {
355+
this._callableModules[name] = methods;
356+
}
357+
340358
}
341359

342360
function moduleHasConstants(moduleArray: Array<Object|Array<>>): boolean {

0 commit comments

Comments
 (0)