Support deep type conversions in FirebaseFunctions data and add crashlytics log function

[vincentborko]

Thank you for contributing to the Skip project! Please use this space to describe your change and add any labels (bug, enhancement, documentation, etc.) to help categorize your contribution.

Firestore test doesnt pass for Kotlin (12/11 passed) but I didnt touch Firestore

Skip Pull Request Checklist:

• REQUIRED: I have signed the Contributor Agreement
• REQUIRED: I have tested my change locally with swift test
• OPTIONAL: I have tested my change on an iOS simulator or device
• OPTIONAL: I have tested my change on an Android emulator or device

[marcprux]

What do you think about harmonizing with SkipFirestore by moving its similar deepSwift implementation:

skip-firebase/Sources/SkipFirebaseFirestore/SkipFirebaseFirestore.swift

Lines 1107 to 1119 in 9b64623

	fileprivate func deepSwift(value: Any) -> Any {
	if let str = value as? String {
	return str // needed to not be treated as a Collection
	} else if let ts = value as? com.google.firebase.Timestamp {
	return Timestamp(timestamp: ts)
	} else if let map = value as? kotlin.collections.Map<Any, Any> {
	return deepSwift(map: map)
	} else if let collection = value as? kotlin.collections.Collection<Any> {
	return deepSwift(collection: collection)
	} else {
	return value
	}
	}

... into SkipFirebaseCore.swift? That way we would share the same implementation for both Functions and Firestore…

[vincentborko]

I'll give it a try, hope I won't run into merge conflicts this time

[vincentborko]

I'll have to move Timestamp into Core then as well

[marcprux]

I'll have to move Timestamp into Core then as well

I think that's fine, since it is a core type for Firebase.

[vincentborko]

should we somehow "reimport" this in skipfirestore? I noticed now to access Timestamps we need to import SkipFirebaseCore + skipfirebasefirestore for android, while only firestore for iOS

[marcprux]

Oh, right … that is a problem. We don't currently handle @_exported import … imports, and so anything defined in SkipFirebaseCore won't come in automatically when importing the other two packages.

Maybe you could add a public typealias Timestamp = SkipFirebaseCore.Timestamp in each of SkipFirebaseFirestore.swift and SkipFirebaseFunction.swift to work around this for the time being?

[marcprux]

…and if that does't work out, we can always just go back to duplicating the deepSwift logic in both packages.

[marcprux]

I think only the top deepSwift(value: Any) function needs to be public. These other ones can be private.

[vincentborko]

of course, fixed

[marcprux]

Hmm, seeing a CI failure:

e: file:///Users/runner/work/skip-firebase/skip-firebase/.build/plugins/outputs/skip-firebase/SkipFirebaseFirestore/destination/skipstone/SkipFirebaseFirestore/src/main/kotlin/skip/firebase/firestore/SkipFirebaseFirestore.kt:1041:20 Cannot access 'fun <T> deepSwift(map: Map<T, Any>): Dictionary<T, Any>': it is private in file.
e: file:///Users/runner/work/skip-firebase/skip-firebase/.build/plugins/outputs/skip-firebase/SkipFirebaseFirestore/destination/skipstone/SkipFirebaseFirestore/src/main/kotlin/skip/firebase/firestore/SkipFirebaseFirestore.kt:1080:20 Cannot access 'fun <T> deepSwift(map: Map<T, Any>): Dictionary<T, Any>': it is private in file.
e: file:///Users/runner/work/skip-firebase/skip-firebase/.build/plugins/outputs/skip-firebase/SkipFirebaseFirestore/destination/skipstone/SkipFirebaseFirestore/src/main/kotlin/skip/firebase/firestore/SkipFirebaseFirestore.kt:1041:20 Cannot access 'fun <T> deepSwift(map: Map<T, Any>): Dictionary<T, Any>': it is private in file.

[vincentborko]

yea, for e.g. in querydocumentsnapshot in skipfirestore we need

override public func data() -> [String: Any] {
    if let data = doc.getData() {
        return deepSwift(map: data)
    } else {
        return [:]
    }
}

I'll make them internal, agree?

[vincentborko]

oh it looks like Kotlin's internal might work file-level instead of module-scoped? I guess I have to revert to public, tests are failing with internal as well...

[marcprux]

I think that'll be fine.

[marcprux]

Thanks! One last request, can you add typealiases for Timestamp in SkipFirebaseFirestore.swift‎ and SkipFirebaseFunctions.swift like:

public typealias Timestamp = SkipFirebaseCore.Timestamp

That way, if anyone was relying on getting Timestamp by importing SkipFirebaseFirestore, it should continue to work for them without having the explicitly import SkipFirebaseCore.

[marcprux]

Thanks for this!