Summary:
As per https://twitter.com/olebegemann/status/738656134731599872, our use of "main thread" to mean "main queue" seems to be unsafe.
This diff replaces the `NSThread.isMainQueue` checks with dispatch_get_specific(), which is the recommended approach.
I've also replaced all use of "MainThread" terminology with "MainQueue", and taken the opportunity to deprecate the "sync" param of `RCTExecuteOnMainThread()`, which, while we do still use it in a few places, is incredibly unsafe and shouldn't be encouraged.
Reviewed By: javache
Differential Revision: D3384910
fbshipit-source-id: ea7c216013372267b82eb25a38db5eb4cd46a089
Summary:
This is a solution for the problem I raised in https://www.facebook.com/groups/react.native.community/permalink/768218933313687/
I've added a new native base class, `RCTEventEmitter` as well as an equivalent JS class/module `NativeEventEmitter` (RCTEventEmitter.js and EventEmitter.js were taken already).
Instead of arbitrary modules sending events via `bridge.eventDispatcher`, the idea is that any module that sends events should now subclass `RCTEventEmitter`, and provide an equivalent JS module that subclasses `NativeEventEmitter`.
JS code that wants to observe the events should now observe it via the specific JS module rather than via `RCTDeviceEventEmitter` directly. e.g. to observer a keyboard event, instead of writing:
const RCTDeviceEventEmitter = require('RCTDeviceEventEmitter');
RCTDeviceEventEmitter.addListener('keyboardWillShow', (event) => { ... });
You'd now write:
const Keyboard = require('Keyboard');
Keyboard.addListener('keyboardWillShow', (event) => { ... });
Within a component, you can also use the `Subscribable.Mixin` as you would previously, but instead of:
this.addListenerOn(RCTDeviceEventEmitter, 'keyboardWillShow', ...);
Write:
this.addListenerOn(Keyboard, 'keyboardWillShow', ...);
This approach allows the native `RCTKeyboardObserver` module to be created lazily the first time a listener is added, and to stop sending events when the last listener is removed. It also allows us to validate that the event strings being observed and omitted match the supported events for that module.
As a proof-of-concept, I've converted the `RCTStatusBarManager` and `RCTKeyboardObserver` modules to use the new system. I'll convert the rest in a follow up diff.
For now, the new `NativeEventEmitter` JS module wraps the `RCTDeviceEventEmitter` JS module, and just uses the native `RCTEventEmitter` module for bookkeeping. This allows for full backwards compatibility (code that is observing the event via `RCTDeviceEventEmitter` instead of the specific module will still work as expected, albeit with a warning). Once all legacy calls have been removed, this could be refactored to something more elegant internally, whilst maintaining the same public interface.
Note: currently, all device events still share a single global namespace, since they're really all registered on the same emitter instance internally. We should move away from that as soon as possible because it's not intuitive and will likely lead to strange bugs if people add generic events such as "onChange" or "onError" to their modules (which is common practice for components, where it's not a problem).
Reviewed By: javache
Differential Revision: D3269966
fbshipit-source-id: 1412daba850cd373020e1086673ba38ef9193050
Summary:
Previously, if a module implemented `setBridge:` we assumed that it needs to be initialised on the main thread. This assumption was not really warranted however, and it was a barrier to deferring module initialization.
This diff tweaks the rules so that only modules that override `init` or `constantsToExport**` are assumed to require main thread initialization, and others can be created lazily when they are first used.
WARNING: this will be a breaking change to any 3rd party modules that are assuming `setBridge:` is called on the main thread. Those modules should be rewritten to move any code that requires the main thread into `init` or `constantsToExport` instead.
`**` We will also be examining whether `constantsToExport` can be done lazily, but for now any module that uses it will still be created eagerly when the bridge starts up.
Reviewed By: javache
Differential Revision: D3240682
fb-gh-sync-id: 48f309e3158bbccb52141032baf70def3e609371
fbshipit-source-id: 48f309e3158bbccb52141032baf70def3e609371
Summary:The 200ms timeout was causing resource issues and causing a lot of overhead when you're not running the devtools, since it will basically create a new socket every 200ms.
Also clean up the way we do logging so it's completely compiled out in prod, and standardize all the names we use for threading to lowercase react.
Reviewed By: frantic
Differential Revision: D3115975
fb-gh-sync-id: e6e51c0621d8e9fc4eadb864acd678b8b5d322a1
fbshipit-source-id: e6e51c0621d8e9fc4eadb864acd678b8b5d322a1
Summary: This diff adds support for initializing the bridge on an arbitrary thread. This is helpful if you want to defer bridge creation, or prevent it from delaying your app startup.
Reviewed By: javache
Differential Revision: D2965725
fb-gh-sync-id: 8065fa89e850031c72ee4427351300986985e9de
shipit-source-id: 8065fa89e850031c72ee4427351300986985e9de
Summary:Initializing native modules can block the main thread for tens of milliseconds when it starts up, making it difficult to instantiate the bridge on demand without causing a performance blip.
This diff splits up the initialization of modules so that - although they still happen on the main thread - they don't block the thread continuously.
Reviewed By: javache
Differential Revision: D2965438
fb-gh-sync-id: 38c9c9d281e4672b5874d68b57d4c60d1d268344
shipit-source-id: 38c9c9d281e4672b5874d68b57d4c60d1d268344
Summary: The module initialization process is complex and full of race conditions. This diff adds a set of unit tests that verify that modules setup happens in the correct order, and enforces all the various conditions for main/background init.
Reviewed By: javache
Differential Revision: D2994145
fb-gh-sync-id: 92ea84508cdeeb280ff0fb9e9b2dffa8dbc37e66
shipit-source-id: 92ea84508cdeeb280ff0fb9e9b2dffa8dbc37e66
Summary:
public
Lazy export of module constants required a sync dispatch to the main thread, which was deadlocking in some of our projects.
This moves the constants export to the initial bridge init, which may slightly increase initial startup time, but avoids the deadlock.
Reviewed By: javache
Differential Revision: D2911295
fb-gh-sync-id: 0d14a629ac4fc7ee21acd293c09595c18232659b
shipit-source-id: 0d14a629ac4fc7ee21acd293c09595c18232659b
Summary:
public
Since we don't actually recreate our native modules every time (will fix in follow-up), we'd never update the reference after reloading the bridge, and all navigation would fail.
Reviewed By: majak
Differential Revision: D2811406
fb-gh-sync-id: 4f4fd73bbdecfe510e1e1554668b2354181f22a8
Summary:
public
Fixed a potential deadlock issue if code attempted to access a module via [bridge moduleForName/Class:] while it was being initialized.
Reviewed By: lry
Differential Revision: D2807827
fb-gh-sync-id: 58cafe9b92c094dde632d17245fb9b342a0fe9e0
Summary:
public
The logic inside RCTBatchedBridge contained some race conditions that would occasionally cause an error if modules were loaded in the wrong order. This improves that logic and makes it safer by adding a lock to prevent concurrency.
Reviewed By: jspahrsummers
Differential Revision: D2802930
fb-gh-sync-id: d1ad25fa578649363dcaac029cb24dc3a453ae67
Summary:
public
The implementation of the `methodQueue` lazy initializer in `RCTModuleData` could result in the queue being set twice, because calling `methodQueue` for a module that hasn't been instantiated would call `RCTModuleData.instance` to create the module, which itself calls `methodQueue`.
It's not clear if this was causing a bug, but it may be related to an occasional bug where the `RCTViewManager.methodQueue` returns nil.
Reviewed By: majak
Differential Revision: D2783320
fb-gh-sync-id: 9194da0fd7392f63825da1f5c450363dd300b635
Summary:
public
Thanks to the new lazy initialization system for modules, `RCTDidCreateNativeModules` no longer does what the name implies.
Previously, `RCTDidCreateNativeModules` was fired after all native modules had been initialized. Now, it simply fires each time the bridge is reloaded. Modules are created on demand when they are needed, so most of the assumptions about when `RCTDidCreateNativeModules` will fire are now incorrect.
This diff deprecates `RCTDidCreateNativeModules`, and adds a new notification, `RCTDidInitializeModuleNotification`, which fires each time a module a new module is instantiated.
If you need to access a module at any time you can just call `-[bridge moduleForClass:]` and the module will be instantiated on demand. If you want to access a module *only* after it has already been instantiated, you can use the `RCTDidInitializeModuleNotification` notification.
Reviewed By: tadeuzagallo
Differential Revision: D2755036
fb-gh-sync-id: 25bab6d5eb6fcd35d43125ac45908035eea01487
Summary:
public
A lot of the core modules have to use private methods in the bridge, specially
since the `RCTBatchedBridge` interface is never exposed. That was leading to a
lot of different private bridge categories spread across different modules,
which makes harder to identify which modules are affected by private API changes.
Replace all the categories with a single private header.
Reviewed By: nicklockwood
Differential Revision: D2757564
fb-gh-sync-id: 793158b9082d542b74a6094ed0db4d5dc3a88f78
Summary:
public
This diff replaces the RegEx module method parser with a handwritten recursive descent parser that's faster and easier to maintain.
The new parser is ~8 times faster when tested on the UIManager.managerChildren() method, and uses ~1/10 as much RAM.
The new parser also supports lightweight generics, and is more tolerant of white space.
(This means that you now can – and should – use types like `NSArray<NSString *> *` for your exported properties and method arguments, instead of `NSStringArray`).
Reviewed By: jspahrsummers
Differential Revision: D2736636
fb-gh-sync-id: f6a11431935fa8acc8ac36f3471032ec9a1c8490
Summary:
public
Looping through every `RCTModuleData` to check whether the module responds to `-batchDidComplete` or `-partialBatchDidFlush` is unnecessarily expensive. We can cache the answer at the time that the module instance is actually initialized.
Reviewed By: tadeuzagallo
Differential Revision: D2717594
fb-gh-sync-id: 274a59ec2d6014ce18c93404ce6b9940c1dc9c32
Summary: public
The `bridge.modules` dictionary provides access to all native modules, but this API requires that every module is initialized in advance so that any module can be accessed.
This diff introduces a better API that will allow modules to be initialized lazily as they are needed, and deprecates `bridge.modules` (modules that use it will still work, but should be rewritten to use `bridge.moduleClasses` or `-[bridge moduleForName/Class:` instead.
The rules are now as follows:
* Any module that overrides `init` or `setBridge:` will be initialized on the main thread when the bridge is created
* Any module that implements `constantsToExport:` will be initialized later when the config is exported (the module itself will be initialized on a background queue, but `constantsToExport:` will still be called on the main thread.
* All other modules will be initialized lazily when a method is first called on them.
These rules may seem slightly arcane, but they have the advantage of not violating any assumptions that may have been made by existing code - any module written under the original assumption that it would be initialized synchronously on the main thread when the bridge is created should still function exactly the same, but modules that avoid overriding `init` or `setBridge:` will now be loaded lazily.
I've rewritten most of the standard modules to take advantage of this new lazy loading, with the following results:
Out of the 65 modules included in UIExplorer:
* 16 are initialized on the main thread when the bridge is created
* A further 8 are initialized when the config is exported to JS
* The remaining 41 will be initialized lazily on-demand
Reviewed By: jspahrsummers
Differential Revision: D2677695
fb-gh-sync-id: 507ae7e9fd6b563e89292c7371767c978e928f33
Summary: public
Added lightweight genarics annotations to make the code more readable and help the compiler catch bugs.
Fixed some type bugs and improved bridge validation in a few places.
Reviewed By: javache
Differential Revision: D2600189
fb-gh-sync-id: f81e22f2cdc107bf8d0b15deec6d5b83aacc5b56
Summary: public
UIExplorer tests were broken due to a refactor that didn't update the RCTShadowViewTests + an off-by-one error in the logic for exporting async methods.
Reviewed By: javache
Differential Revision: D2595810
fb-gh-sync-id: c25a8b8956bff1ef2754bba4a8f10d72a16e2954
Summary: public
Use arrays instead of dictionaries for encoding module method information.
This further reduces UIExplorer startup JSON from 16104 bytes to 14119 (12% reduction)
Reviewed By: javache
Differential Revision: D2570057
fb-gh-sync-id: 4a53a9ead4365a136e7caeb650375733e1c24c0e
Summary: public
We're sending a lot of module config data when the app first starts, and much of this is redundant.
UIExplorer current sends 19061 bytes of module config JSON. This diff reduces that to 16104 (15% saving) by stripping modules that have no methods or constants, and removing method types unless method is async.
Reviewed By: tadeuzagallo, javache
Differential Revision: D2570010
fb-gh-sync-id: 8c0abbd1cdee3264b37a4f52e852008caaffb9c5
Summary:
Occasionally people create RCTBridgeModule subclasses or base classes that are not intended to be accessed from JS, and so they don't export them. This was previously flagged as an error by the system. I've now downgraded this error to a warning at startup, and deferred the redbox error until the module is directly accessed by native or JS code.
Summary:
@public
The info about bridge modules (such as id, name, queue, methods...) was spread
across arrays & dictionaries on the bridge, move it into a specific class.
It also removes a lot of information that was statically cached, and now have
the same lifecycle of the bridge.
Also moved RCTModuleMethod, RCTFrameUpdate and RCTBatchedBridge into it's own
files, for organization sake.
NOTE: This diff seems huge, but most of it was just moving code :)
Test Plan:
Tested UIExplorer & UIExplorer tests, Catalyst, MAdMan and Groups. Everything
looks fine.
Nick Lockwood
Marc Horowitz
Pieter De Baets
Alexey Lang
Tadeu Zagallo
Justin Spahr-Summers
David Aurelio