🐛 [cloud_firestore] sentinel value breaks wrappers

[cedvdb]

Bug report

Following this issue a "sentinel value" was added to query parameters.

From:

  Query<Map<String, dynamic>> where(
    Object fieldOrFilter, {
    Object? isEqualTo,
    Object? isNotEqualTo,
    Object? isLessThan,
    Object? isLessThanOrEqualTo,
    Object? isGreaterThan,
    Object? isGreaterThanOrEqualTo,
    Object? arrayContains,

To:

    Object? isEqualTo = notSetQueryParam,
    Object? isNotEqualTo = notSetQueryParam,
    Object? isLessThan = notSetQueryParam,
    Object? isLessThanOrEqualTo = notSetQueryParam,
    Object? isGreaterThan = notSetQueryParam,
    Object? isGreaterThanOrEqualTo = notSetQueryParam,
    Object? arrayContains = notSetQueryParam,

However this is a breaking change for any kind of wrapper as null will be passed:

  AppQuery<T> where(
    AppFieldPath fieldPath, {
    Object? isEqualTo,
    Object? isNotEqualTo,
    Object? isLessThan,
    Object? isLessThanOrEqualTo,
    Object? isGreaterThan,
    Object? isGreaterThanOrEqualTo,
    Object? arrayContains,
    Iterable<Object?>? arrayContainsAny,
    Iterable<Object?>? whereIn,
    Iterable<Object?>? whereNotIn,
    bool? isNull,
  }) {
    final queryRef = _ref.where(
      fieldPath.toFirestore(),
      isEqualTo: isEqualTo,
      isNotEqualTo: isNotEqualTo,
      isLessThan: isLessThan,
      isLessThanOrEqualTo: isLessThanOrEqualTo,
      isGreaterThan: isGreaterThan,
      isGreaterThanOrEqualTo: isGreaterThanOrEqualTo,
      arrayContains: arrayContains,
      arrayContainsAny: arrayContainsAny,
      whereIn: whereIn,
      whereNotIn: whereNotIn,
      isNull: isNull,
    );
    // ...

I believe this (breaking) change was not required since there is already a isNull parameter. Exposing notSetQueryParam to the public api doesn't sound too good, so if that previous change wasn't necessary I suggest reverting those changes as this will be a breaking change for libraries wrapping firebase. For instance fake_cloud_firestore has diverged in behavior from cloud firestore.

Note: The dart default parameter proposal would have been helpful here

[darshankawar]

/cc @Lyokone

[russellwheatley]

Hey @cedvdb, thanks for the update. Upon review, we have decided that this does constitute a breaking change and will roll this back in the next release 🙏