googlearchive / firebase-dart

Dart wrapper for Firebase
https://pub.dev/packages/firebase
404 stars 144 forks source link

Relax analytics null safe restrictions #389

Closed IchordeDionysos closed 3 years ago

IchordeDionysos commented 3 years ago

This PR aims to relax null safe type restrictions to allow null values where other platform implementations also allow null values.

See this comment for more details: https://github.com/FirebaseExtended/flutterfire/pull/5341#issuecomment-802318246

tl;dr: There are some types in this package that don't allow null values, but on other platforms explicitly allow null values. These null values are often important as the null value signals that a property should be reset.

For example, setting a null screenName or uid resets it in all further analytics events. Or setting a user property to null removes this user property for the user.

Blocks https://github.com/FirebaseExtended/flutterfire/pull/5341

/cc @kevmoo mentioning you here as you've did the null-safe PR :)

kevmoo commented 3 years ago

Looks good. Wait for https://github.com/FirebaseExtended/firebase-dart/pull/390 to land then rebase on that...

kevmoo commented 3 years ago

Landed my fixes @IchordeDionysos – please rebase. Your version is fine. I can publish this ASAP

IchordeDionysos commented 3 years ago

@kevmoo did rebase onto master, thanks for being so quick :) Not sure what's happening with the tests and the SERVICE_ACCOUNT_JSON though :)

IchordeDionysos commented 3 years ago

@kevmoo when do you roughly plan to release the new version to pub.dev? :)

kevmoo commented 3 years ago

@kevmoo when do you roughly plan to release the new version to pub.dev? :)

once https://github.com/FirebaseExtended/firebase-dart/pull/391 is green and lands @IchordeDionysos

kevmoo commented 3 years ago

@IchordeDionysos – published!