Upgrade Fabric/Crashlytics to new FirebaseCrashlytics#1699
Conversation
...and upgrade everything to new FirebaseCrashlytics API TODO: manual crashing is currently missing
| if (arg.crashlyticsCollectionEnabled && typeof (Crashlytics) !== "undefined") { | ||
| Fabric.with(NSArray.arrayWithObject(Crashlytics.class())); | ||
| } | ||
| // if (arg.crashlyticsCollectionEnabled && typeof (Crashlytics) !== "undefined") { |
There was a problem hiding this comment.
yes, you are right I unfortunately missed to clean up that code.
For reference: now only FIRApp.configure() should be necessary to initialize the new crashlytics SDK (which is done a few lines above).
| } | ||
| } | ||
|
|
||
| export function setString(key: string, value: string): void { |
There was a problem hiding this comment.
Since all these methods deeply call the same method, perhaps mark the parent typings for these methods as deprecated and add a singular function to replace them? Would make it easier to digest / consume.
Seems to be that way for both the Android & iOS versions
There was a problem hiding this comment.
The reason that I did not do what you recommended was to keep the change size and the impact on the existing api minimal, to have a quick solution that can be used on 15th of November. I think that this refactoring should be done, but maybe not right now.
There was a problem hiding this comment.
This would only add to the API, not remove used API's, marking deprecated just prepares people for the future removal is all.
| @@ -5,72 +5,69 @@ declare const com: any; | |||
|
|
|||
There was a problem hiding this comment.
import * as appModule from 'tns-core-modules/application'; should be removed, not ns7 compat
| export function crash(): void { | ||
| if (isCrashlyticsAvailable()) { | ||
| Crashlytics.sharedInstance().crash(); | ||
| // TODO: manually force crash - https://firebase.google.com/docs/crashlytics/test-implementation?authuser=0&platform=ios#force_a_crash_to_test_your_implementation |
There was a problem hiding this comment.
From my understanding, you can crash with:
NSException.exceptionWithNameReasonUserInfo(
"FIRCrashlytics",
"test crash",
null
).raise();I tested this in a demo app I made quickly (since the one in this repo isn't working for me):
***** Fatal JavaScript exception - application has been terminated. *****
NativeScript encountered a fatal error: Uncaught Error: test crash
...
*** First throw call stack:
...
|
Many thanks to all involved! ❤️ I've just published 10.6.0 to npm which includes these changes. I'm now going to attempt to make this {N}7 compatible as well. |
4 participants
As discussed in #1642 Google is deprecating the Fabric Crashlytics SDK on November 15, 2020 so to continue the crashlytics support for this plugin it has to be upgraded to the new FirebaseCrashlytics SDK
My colleague @000panther and myself upgraded the Android and iOS specific SDKs to the new dependencies and new API. For iOS I also was able to remove the custom wrapper library
TNSCrashlyticsLoggersince it is no longer needed - just as its creator @kanclalg suggested.Known issues/open tasks:
Please feel free to test it yourselves and give us feedback.