Implementation error while dealing with set value for a FormArray of FormGroups

[shannon-rodricks]

Hi,
Firstly, thanks for this library. Its amazing for dealing with forms and saves a lot of time!

I am currently using it to create/edit events, with the following fields

• Name
• Description
• Date
• Start Time
• Pricing [] (list of formgroups)
  • Category (text field with typeahead)
  • Price (text field with int type)

Defined as so:

var form = FormGroup({
  "name": FormControl<String>(value: "", validators: [Validators.required]),
  "description": FormControl<String>(value: ""),
  "date": FormControl<DateTime>(value: null, validators: [Validators.required]),
  "time": FormControl<DateTime>(value: null, validators: [Validators.required]),
  "pricing": FormArray([])
});

var pricingGroup = FormGroup({
  "category": FormControl<String>(value: "", validators: [Validators.required]),
  "cost": FormControl<int>(value: 0, validators: [Validators.required])
});

The issue arises when I attempt to set a value to the form, to edit an existing event.

form.value = {
  "name": "Event Name",
  "description": "Fake description",
  "date": DateTime.now(),
  "time": DateTime.now(),
  "pricing": [
    pricingGroup..value = {
      "category": "General",
      "cost": 200
    }
  ]
};

What I'm doing is converting the json to a formgroup, for each pricing object, which i need to do to ensure that the validators are applied, or for more advanced requirements (formgroups with nested form arrays etc).
The issue arises in this code in FormArray

@override
  void updateValue(
    List<T?>? value, {
    bool updateParent = true,
    bool emitEvent = true,
  }) {
    for (var i = 0; i < _controls.length; i++) {
      if (value == null || i < value.length) {
        _controls[i].updateValue(
          value?.elementAt(i),
          updateParent: false,
          emitEvent: emitEvent,
        );
      }
    }

    if (value != null && value.length > _controls.length) {
      final newControls = value
          .toList()
          .asMap()
          .entries
          .where((entry) => entry.key >= _controls.length)
          .map((entry) => FormControl<T>(value: entry.value))
          .toList();

      addAll(
        newControls,
        updateParent: updateParent,
        emitEvent: emitEvent,
      );
    } else {
      updateValueAndValidity(
        updateParent: updateParent,
        emitEvent: emitEvent,
      );
    }
  }

specifically

          .map((entry) => FormControl<T>(value: entry.value))

This wraps every formgroup in a formcontrol, which breaks form handling (validations and .value don't work correctly)

To resolve my issue, I've done this

final List<AbstractControl<T>> newControls = value
          .toList()
          .asMap()
          .entries
          .where((entry) => entry.key >= _controls.length)
          .map((entry) {
        if (entry.value is AbstractControl) {
          return entry.value as AbstractControl<T>;
        }
        return FormControl<T>(value: entry.value);
      })
          .toList();

which makes everything work correctly.

Does it make sense to change the code to this?

[joanpablo]

Hi @shannon-rodricks,

Thanks for your kind words, and for the issue. I'm giving it a look and I will let you know my thoughts.

[shannon-rodricks]

@joanpablo Thanks for the quick response! Let me know if I can help in any way

[joanpablo]

Hi @shannon-rodricks,

Thanks for your patience. Although your code works, IMO it introduces an ambiguity with a value of a FormArray. Suppose we have a FormArray, your proposed implementation of the updateValue() is not consistent with the get value() of a FormArray. The first one allows you to set native and AbstractControls, but the last one always returns an array of native values, not an array of AbstractControls, see what I mean there is a semantic inconsistency between get value() and set value().

My advice is just add values like this:

final group = FormGroup({
      "name": FormControl<String>(),
      "pricing": FormArray([])}
    );

    final pricing = group.controls['pricing'] as FormArray;
    pricing.add(
      fb.group({
        "category": fb.control("Some category", [Validators.required]),
        "cost": fb.control(12, [Validators.required])
      })
    );

    group.patchValue({"name": "Some Name"});

[dannypurcell]

@joanpablo First and foremost I would also like to express my appreciation for this excellent project! It really does bring a lot of sanity to creating forms in Flutter!

I think I have another use case to add to the issue here.

Similar to @shannon-rodricks I am finding difficultly working with FormArrays when nested with FormGroups. This use case is slightly different in that I am wishing to set a json data structure to an existing FormArray containing FormGroups as elements.

It seems like it is simply not possible to use part of the FormArray interface in some use cases. Particularly, the .value = setter will produce surprising and inconsistent results any time the elements of a FormArray are FormGroups.

Here follows the use case I've run in to and my best attempt so far to resolve it. I'd love to have your take on the solution and please let me know if I have simply misunderstood the intended use cases for FormArray.

If I understand correctly, doing something like this, is intended to be a first class use case.

FormGroup contactForm = FormGroup({
  'id': FormControl<String>(),
  'contact_addresses': FormArray([]),
  'notes': FormControl<String>(),
})

where contact_addresses is defined in a sub-form leveraging ReactiveFormArray as

  FormGroup _newRecord() {
    return FormGroup({
      'id': FormControl<String>(),
      'type': FormControl<String>(),
      'address': FormControl<String>(),
      'address_line_2': FormControl<String>(),
      'city': FormControl<String>(),
      'state': FormControl<String>(),
      'zip_code': FormControl<String>(),
    });
  }
...
  @override
  Widget build(BuildContext context) {
    return ReactiveFormArray(
      formArrayName: arrayName,
      builder: (context, array, child) => ListView(
        shrinkWrap: true,
        physics: ClampingScrollPhysics(),
        children: [
          ListTile(
            leading: icon ?? Icon(Icons.info),
            title: Row(
              children: [
                Text(arrayName.titleCase()),
                SizedBox(width: 5),
                AddRowButton(array: array, recordBuilder: _newRecord),
              ],
            ),
          ),
          ...array.controls
              .map((r) => _buildRow(context, array, r))
              .zip(array.controls.map((e) => SizedBox(height: 10))),
        ],
      ),
    );
  }

  Widget _buildRow(
      BuildContext context, FormArray array, AbstractControl control) {
    FormGroup record = control as FormGroup; // !!!Expected a value of type 'FormGroup', but got one of type 'FormControl<Object?>'
    return ReactiveForm(
      formGroup: record,
      child: Row(
        mainAxisAlignment: MainAxisAlignment.spaceEvenly,
        children: [
          Expanded(child: rowBuilder(context)),
          SubtractRowButton(array: array, record: record),
        ],
      ),
    );
  }
...

This works fine when adding to the contact_addresses array via FormArray.add in our AddRowButton

@override
  Widget build(BuildContext context) {
    return TextButton(
      onPressed: Actions.handler(
          context, ArrayFormAddRowIntent(array, recordBuilder())),
      child: const Icon(Icons.add),
    );
  }
...
void invoke(covariant ArrayFormAddRowIntent intent) {
    FormArray array = intent.array!;
    FormGroup record = intent.record!;

    array.add(record);
  }

Technically we can add any AbstractControl rather than just the intended FormGroup type signature. So the type checker isn't exactly as helpful here as it could be; but, as long as we're careful to be consistent in our add calls, this is perfectly usable.

Unfortunately, with the way updateValue is currently implemented, setting the value of a complex FormGroup, with arrays containing groups, from something like a json structure results in a mixed array of FormGroup and FormControl<Object?> whenever the incoming data would cause one of the FormArrays to grow.
This is surprising behavior given the behavior of FormControl and FormGroup as well as the suggested method of setting data described in the documentation:
https://pub.dev/packages/reactive_forms#how-to-getset-form-data
https://pub.dev/packages/reactive_forms#dynamic-forms-with-formarray
Ex:

FormGroup contactForm = ...;
Map<String, dynamic>? data = await dataService.getContact(id);
contactForm.value = data;

dataService being an implementation of Supabase/PostgREST's beautifully flexible select queries to fetch a nice fitting record structure for these forms.

return await supabase
        .from('contacts')
        .select('''
          id,
          contact_addresses(id, type, address, address_line_2, city, state, zip_code),
          notes
        ''')
        .eq('id', id)
        .maybeSingle()

This means when we go to display the form with a ReactiveForm widget, we would have to stop to convert those FormControl<Object?> items to FormGroups in order to make the sub-form able to use them.

Our only other option seems to be as you suggest above, which pretty much requires us to drop back to creating individual data model classes for every form we might want in the app, just so we can make sure the FormArrays are at least as big as the incoming data and thereby avoid tripping the above referenced type mismatch error.

  factory Contact.fromJson(Map<String, dynamic>? data) {
    Contact contact = Contact();
    if (data != null) {
      for (var _ in (data['contact_addresses'] as List)) {
        contact.names.add(ContactAddress());
      }
    }
    contact.value = data;
    return contact;
  }

This is totally doable of course, but it's a shame really because the combination of Supabase/PostgREST with ReactiveForms ALMOST eliminates the need to do boilerplate data model classes.

We have json encoder/decoders that can get all of the basic data types we need such as DateTime or Currency so it really is just this one data shaping issue that blocks us from querying data and dropping it straight into the form structures.

In attempting to find a more generic solution, I think I get the difficulty inherent in trying to handle the possible element types allowed by the type signatures.
So far the two ways I can think of are:

• Clone an existing FormGroup in the list and then set the value to the new one,
  • This would require that the list have at least one FormGroup element to start with else the current behavior of adding a FormControl would be the default.
  • That seems...okay-ish? in that it would give people an option to work with which is more consistent with the way FormGroup and FormControl work with regard to the .value = setter. However, this also seems like an unusual kind of interface and usage pattern for Dart/Flutter. I can guess many people would not expect to have to give it an example element just to get the internal type signatures to be consistent.
• Allow the calling code to set an itemBuilder as final FormGroup Function() itemBuilder;.

For that second answer, I am having some success with sub-classing FormArray to do that.

import 'package:reactive_forms/reactive_forms.dart';

class FormGroupArray extends FormArray<Map<String, dynamic>> {
  final FormGroup Function() itemBuilder;

  FormGroupArray(super.controls, {required this.itemBuilder});

  @override
  void updateValue(
    List<dynamic>? value, {
    bool updateParent = true,
    bool emitEvent = true,
  }) {
    for (var i = 0; i < controls.length; i++) {
      if (value == null || i < value.length) {
        controls[i].updateValue(
          value?.elementAt(i),
          updateParent: false,
          emitEvent: emitEvent,
        );
      }
    }

    if (value != null && value.length > controls.length) {
      final newControls = value
          .toList()
          .asMap()
          .entries
          .where((entry) => entry.key >= controls.length)
          .map((entry) => itemBuilder()..value = entry.value)
          .toList();

      addAll(
        newControls,
        updateParent: updateParent,
        emitEvent: emitEvent,
      );
    } else {
      updateValueAndValidity(
        updateParent: updateParent,
        emitEvent: emitEvent,
      );
    }
  }
}

The usage pattern for this isn't as pretty as we'd probably like but it seems close.

FormGroup({
  'id': FormControl<String>(),
  'contact_addresses': FormGroupArray([],
      itemBuilder: () => FormGroup({
            'id': FormControl<String>(),
            'type': FormControl<String>(),
            'address': FormControl<String>(),
            'address_line_2': FormControl<String>(),
            'city': FormControl<String>(),
            'state': FormControl<String>(),
            'zip_code': FormControl<String>(),
          })),
  'notes': FormControl<String>(),
});

Perhaps it could be another either/or situation like the formArray or formArrayName ReactiveFormArray allows for.
Either the calling code provides a controls list or an itemBuilder and the controls list defaults to empty list.

If this solution were to be incorporated into the base implementation, it seems like FormArray would have to take on yet another generic type signature which would define the element type for the list and return type for the itemBuilder, if given. The default itemBuilder could still return a FormControl<Object?>.

If combining both enhancement ideas is desirable, updateValue could clone an existing element if the controls list is non-empty, or use the itemBuilder if it is empty. The itemBuilder would still be defaulted to return a FormControl if not provided in the constructor.

This should provide options for internal type signature consistency in all use cases, though such would still not be enforced by the type checker so we'd still rely on users reading the docs to get it right for their intended use. If we're willing to break backward compatibility, an assertion could be added to require either a non-empty array or an itemBuilder.

[ahmednfwela]

I have created #477 to solve this