From 58cd20459f7d5fac879baaf7944eace0dba3a415 Mon Sep 17 00:00:00 2001
From: Ramanpreet Nara <ramanpreet@fb.com>
Date: Tue, 26 Mar 2019 14:19:07 -0700
Subject: [PATCH] Force JS null to be translated to ObjC nil

Summary:
In D14571128, we made it so that when a JS object's property was `undefined`, we wouldn't insert that property into the corresponding NSDictionary. Here are two important observations about that diff:
1. ALL JS `null`s were now being converted to `NSNull`, and JS `undefined`s were now being converted to `nil`.
2. If a JS object's property was explicitly `null`, then we'd insert `NSNull` into the corresponding dictionary.

Considering that when a property doesn't exist in a `NSDictionary`, property access returns `nil`, I've made it so that if a JS object's property is either `null` or `undefined`, then we simply do not insert it in the corresponding `NSDictionary`. Also, I've reverted #1 and made it so that `undefined` and `null` always map to the ObjC `nil`.

This shouldn't unfix the problem that D14571128 was trying to fix.

Here's my understanding of the problem that D14571128 was trying to fix (to make sure I'm not breaking something by this diff).

This method was invoked from JS.
```
RCT_EXPORT_METHOD(logEvents:(NSDictionary *)events)
{
  RCTAssert(events, @"You should provide events for logger");
  [[NSNotificationCenter defaultCenter] postNotificationName:@"FBReactPerfLoggerDidReceiveEventsNotification"
                                                      object:nil
                                                    userInfo:@{@"FBReactPerfLoggerUserInfoPerfEventsKey" : [events copy]}];
}
```

The above dispatch calls into this method, which appends `events` into `_pendingJSPerfEvents`.

```
- (void)reactPerfLoggerDidReceiveEvents:(NSNotification *)notification
{
  NSDictionary *events = notification.userInfo[@"FBReactPerfLoggerUserInfoPerfEventsKey"];
  if (events) {
    dispatch_async(_eventQueue, ^{
      if (self->_sessionData.perfLoggerFlagId != nil) {
        if ([self processJSPerfEvents:events]) {
          [self reportMetricsIfFinished];
        }
      } else {
        [self->_pendingJSPerfEvents addObject:events];
      }
    });
  }
}
```

Then, in `_processJSPerfEvents`, we do the following (link: https://fburl.com/tr4wr2a7):
```
NSNumber *actionId = events[@"actionId"];
if (actionId) {
  self->_sessionData.actionId = actionId;
}
```

So, if `undefined` or `null` was passed in as the `actionId` property of the `events` JS object in `FBReactPerformanceLogger logEvents:`, then we'd default the `NSDictionary` to have `NSNull` in the corresponding property. This is bad because we had this line in FBReactWildePerfLogger (link: https://fburl.com/2nsywl2n):  `actionId ? [actionId shortValue] : PerfLoggerActions.SUCCESS`. Essentially, this is the same problem that my diff is trying to fix.

Reviewed By: fkgozali

Differential Revision: D14625287

fbshipit-source-id: c701d4b6172484cee62494256175e8b205b23c73
---
 ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.mm | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.mm b/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.mm
index 9cae5a3517b..ed2d68dd3ab 100644
--- a/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.mm
+++ b/ReactCommon/turbomodule/core/platform/ios/RCTTurboModule.mm
@@ -115,12 +115,9 @@ static NSDictionary *convertJSIObjectToNSDictionary(jsi::Runtime &runtime, const
 
 static RCTResponseSenderBlock convertJSIFunctionToCallback(jsi::Runtime &runtime, const jsi::Function &value, std::shared_ptr<react::JSCallInvoker> jsInvoker);
 static id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, std::shared_ptr<react::JSCallInvoker> jsInvoker) {
-  if (value.isUndefined()) {
+  if (value.isUndefined() || value.isNull()) {
     return nil;
   }
-  if (value.isNull()) {
-    return (id)kCFNull;
-  }
   if (value.isBool()) {
     return @(value.getBool());
   }