Support field values in `set` methods

[Rexios80]

Adds generated setters with field value parameters

  /// Sets data on the document, overwriting any existing data. If the document
  /// does not yet exist, it will be created.
  ///
  /// If [SetOptions] are provided, the data can be merged into an existing
  /// document instead of overwriting.
  ///
  /// Any [FieldValue]s provided will replace the corresponding fields in the
  /// [model] during serialization.
  Future<void> set(
    IgnoredGetter model, {
    SetOptions? options,
    FieldValue valueFieldValue,
  });

  /// Writes to the document using the transaction API.
  ///
  /// If the document does not exist yet, it will be created. If you pass
  /// [SetOptions], the provided data can be merged into the existing document.
  ///
  /// Any [FieldValue]s provided will replace the corresponding fields in the
  /// [model] during serialization.
  void transactionSet(
    Transaction transaction,
    IgnoredGetter model, {
    SetOptions? options,
    FieldValue valueFieldValue,
  });

  /// Writes to the document using the batch API.
  ///
  /// If the document does not exist yet, it will be created. If you pass
  /// [SetOptions], the provided data can be merged into the existing document.
  ///
  /// Any [FieldValue]s provided will replace the corresponding fields in the
  /// [model] during serialization.
  void batchSet(
    WriteBatch batch,
    IgnoredGetter model, {
    SetOptions? options,
    FieldValue valueFieldValue,
  });

Note that this is a BREAKING change since I had to modify the signature of the set methods. Also the set methods no longer exist on FirestoreDocumentReference.

I also cleaned up some deprecations. If you want those in a separate PR please let me know.

Closes https://github.com/FirebaseExtended/firestoreodm-flutter/issues/5

Also note I did not do this for the CollectionReference.add method as that would require a lot of refactoring

[Rexios80]

For context, I need to use FieldValue.serverTimestamp() on DateTime fields when setting documents. That might be the only FieldValue that makes sense to use when creating documents, but it doesn't make sense to somehow only support that one instead of all FieldValues.

[Rexios80]

@rrousselGit What do you need from me on this?

[rrousselGit]

My bad, I forgot to press "send" it seems like. Let me reply

[rrousselGit]

Honestly, I wonder if this is the right approach.

What about instead generating a separate set method that doesn't take the class instance as parameter, but instead have every field as required named parameters?

Like:

people.doc('123').setValues(
  name: 'John',
  age: 18,
);

[rrousselGit]

One worry I have with the current approach is: How would one go ahead and create an instance of the class that could be passed to set if one of those fields is initialized by the server?

It'd be odd to have to write:

doc.set(
  MyClass(
     someDate: Data.now(),  /* useless field as we use field-values afterward, but the constructor requires it */
  ),
  someDateFieldValue: FieldValue.serverTimestamp,
)

[Rexios80]

It'd be odd to have to write:

It is a bit odd, but I think it preserves type safety better than any other method. The parameters for a different set method would have to be of type Object which isn't helpful.

[rrousselGit]

update is type-safe, so I'm sure we can have such a set method be type-safe too.

The main challenge will be around having an error if one field is not specified.
One solution could be to have some form of box object:

class Value<T> {
  Value(T value);
  Value.fromFieldValue(FieldValue value);
}

....

setValues({
  required Value<String> name,
  required Value<int> age,
});

Although if we use this pattern, it feels like update should be updated to match this ; for consistency.

[Rexios80]

One issue with having a set method with all the fields is it bypasses any constructor logic of the object

[rrousselGit]

I don't think that's a big deal

[Rexios80]

Is it not though? What about something like this

class Test {
  final int field;

  Test(int? field): field = field ?? 0;
}

[rrousselGit]

Let's merge this. I don't think it's worth blocking.

[dorklein]

Can you update the documentation for this change?

[Rexios80]

@dorklein Is there a specific part of the documentation that needs updated?