Tidy up PR

react-native/ios/RealmReact/RealmReact.mm

@@ -52,6 +52,7 @@ - (JSContext *)context;
 @interface RCTBridge (Realm_RCTCxxBridge)
 - (JSGlobalContextRef)jsContextRef;
 - (void *)runtime;
+// Expose the CallInvoker so that we can call `invokeAsync`
 - (std::shared_ptr<facebook::react::CallInvoker>)jsCallInvoker;
 @end
 
@@ -85,7 +86,8 @@ @interface RealmReact () <RCTBridgeModule>
 
 @implementation RealmReact {
     NSMutableDictionary *_eventHandlers;
-    bool waitingForFlush;
+    // Keep track of whether we are already waiting for the React Native UI queue to be flushed asynchronously
+    bool waitingForUiFlush;
 
 #if DEBUG
     GCDWebServer *_webServer;
@@ -111,7 +113,7 @@ - (instancetype)init {
     self = [super init];
     if (self) {
         _eventHandlers = [[NSMutableDictionary alloc] init];
-        waitingForFlush = false;
+        waitingForUiFlush = false;
     }
     return self;
 }
@@ -278,15 +280,15 @@ - (void)dealloc {
 
 typedef JSGlobalContextRef (^JSContextRefExtractor)();
 
-void _initializeOnJSThread(JSContextRefExtractor jsContextExtractor, std::function<void()> flushUiQueue) {
+void _initializeOnJSThread(JSContextRefExtractor jsContextExtractor, std::function<void()> flushUiQueueFn) {
     // Make sure the previous JS thread is completely finished before continuing.
     static __weak NSThread *s_currentJSThread;
     while (s_currentJSThread && !s_currentJSThread.finished) {
         [NSThread sleepForTimeInterval:0.1];
     }
     s_currentJSThread = [NSThread currentThread];
 
-    RJSInitializeInContext(jsContextExtractor(), flushUiQueue);
+    RJSInitializeInContext(jsContextExtractor(), flushUiQueueFn);
 }
 
 - (void)setBridge:(RCTBridge *)bridge {
@@ -330,20 +332,27 @@ - (void)setBridge:(RCTBridge *)bridge {
                 };
                 return static_cast<RealmJSCRuntime*>(bridge.runtime)->ctx_;
             }, ^{
-                // Calling jsCallInvokver->invokeAsync causes React Native to flush any pending UI
-                // updates. We call this after we have called into JS from C++, in order to ensure 
-                // the UI updates in response to any changes, as we bypass the "normal" RN calling
-                // mechanisms (see #4389, facebook/react-native#33006).
+                // Calling jsCallInvokver->invokeAsync tells React Native to execute the lambda passed
+                // in on the JS thread, and then flush the internal "microtask queue", which has the 
+                // effect of flushing any pending UI updates. 
                 //
-                // Calls are debounced using the waitingForFlush flag, so if an async flush is already
-                // pending when another JS to C++ call happens, we don't call invokeAsync again. It 
-                // might be possible to further optimize this, e.g. only call max once per frame.
-                if (!waitingForFlush) {
-                    waitingForFlush = true;
+                // We call this after we have called into JS from C++, in order to ensure that the RN
+                // UI updates in response to any changes from Realm. We need to do this as we bypass
+                // the usual RN bridge mechanism for communicating between C++ and JS, so without doing
+                // this RN has no way to know that a change has occurred which might require an update
+                // (see #4389, facebook/react-native#33006).
+                //
+                // Calls are debounced using the waitingForUiFlush flag, so if an async flush is already
+                // pending when another JS to C++ call happens, we don't call invokeAsync again. This works
+                // because the work is performed before the microtask queue is flushed - see sequence
+                // diagram at https://bit.ly/3kexhHm. It might be possible to further optimize this, 
+                // e.g. only flush the queue a maximum of once per frame, but this seems reasonable.
+                if (!waitingForUiFlush) {
+                    waitingForUiFlush = true;
                     [bridge jsCallInvoker]->invokeAsync([&](){
-                        waitingForFlush = false;
+                        waitingForUiFlush = false;
 
-                        if (waitingForFlush) {
+                        if (waitingForUiFlush) {
                             [bridge jsCallInvoker]->invokeAsync([&](){});
                         }
                     });

src/android/jsc_override.cpp

@@ -144,6 +144,9 @@ static JSGlobalContextRef create_context(JSContextGroupRef group, JSClassRef glo
     // Clear cache from previous instances.
     RJSInvalidateCaches();
 
+    // We pass a no-op lambda for the `flushUiQueue` function, as there's no easy way to
+    // access the React Native context from here and the UI flush bug (#4389) has not been
+    // seen on Android. We should be able to do this properly on Hermes.
     RJSInitializeInContext(ctx, []() {});
     realmContextInjected = true;
     return ctx;

src/jsc/jsc_function.hpp

@@ -23,7 +23,7 @@
 namespace realm {
 namespace js {
 
-// TODO
+// Function passed in from the React Native initialisation code to flush the UI microtask queue
 extern std::function<void()> flush_ui_queue;
 
 template <>

src/jsc/jsc_init.h

@@ -26,7 +26,7 @@ extern "C" {
 #endif
 
 JSObjectRef RJSConstructorCreate(JSContextRef ctx);
-void RJSInitializeInContext(JSContextRef ctx, std::function<void()> send_dummy_event);
+void RJSInitializeInContext(JSContextRef ctx, std::function<void()> flush_ui_queue);
 void RJSInvalidateCaches();
 
 #ifdef __cplusplus