Update iOS initialization to configure from code instead of plist #62
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 5.0 #62 +/- ##
==========================================
+ Coverage 78.21% 78.48% +0.27%
==========================================
Files 28 28
Lines 1207 1218 +11
Branches 93 96 +3
==========================================
+ Hits 944 956 +12
+ Misses 263 262 -1
|
| public init(from: NSDictionary) { | ||
| for (key, value) in from { | ||
| if let key = key as? String { | ||
| // Keys defined in packages/core/interfaces/Config.ts | ||
| if key == "appId", let value = value as? String { | ||
| appId = value | ||
| } else if key == "appGroupId", let value = value as? String { | ||
| appGroupId = value | ||
| } else if key == "disableCrashReporter", let value = value as? Bool { | ||
| disableCrashReporter = value | ||
| } else if key == "disableAutomaticViewCapture", let value = value as? Bool { | ||
| disableAutomaticViewCapture = value | ||
| } else if key == "endpointBaseUrl", let value = value as? String { | ||
| endpointBaseUrl = value | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I dont see any advantage to use for if we are going to check with if each variable inside. Maybe just decode the json with JSONDecoder (but it will fail if there is a typo error, you could handle the error)
or just something like this
self.appGroupId = from["appGroupId"] as? String
self.disableCrashReporter = from["disableCrashReporter"] as? Bool ?? false
self.disableAutomaticViewCapture = from["disableAutomaticViewCapture"] as? Bool ?? false
self.endpointBaseUrl = from["endpointBaseUrl"] as? String```
There was a problem hiding this comment.
yep that's much better, I didn't think to do it that way
| ios?: { | ||
| appId: string; | ||
| appGroupId?: string; | ||
| disableCrashReporter?: boolean; |
There was a problem hiding this comment.
mmm shouldn't be there other attribute to set other crash reporter than us? like crashlitics or something like that, or that is managed in other place
There was a problem hiding this comment.
this was to keep feature parity for what we currently talk about in the docs: https://embrace.io/docs/react-native/integration/add-embrace-sdk/
so allowing the user to just disable our crash reporter. Automatically configuring crashlytics in place of ours would be more complicated and something we decided not to provide for now, a customer would have to do this themselves in objective-c / swift
Documentation changes in progress separately
Updating the JS side to accept a configuration object that for now is just used for iOS setup and passing that through to the swift code. Only a subset of configuration is exposed to get feature parity with what we allowed to be configured in 5.x