FormControl not updating with different control than previous build

[rebaz94]

Hi.
I have a form that shows some field and when data saved, I update entire FormGroup to a new Form and also all FormControl will updated as well. but the control in the ui not updated and it uses the old FormControl.

Is this intended or a bug?

After that I just modified ReactiveTextField and ReactiveFormField to change controller in didUpdateWidget and everything work perfectly.

you can see modified code

If looks good I can make a PR

[vasilich6107]

@rebaz94 you can just show another group of controls instead of updating the formControl in widget

[rebaz94]

That's what I'm doing. when I change FormGroup it will rebuild, but the FormControl will use the old controller

[vasilich6107]

Let's wait for @joanpablo answer...

[rebaz94]

If you check the ReactiveTextField you can see it will not react to changing controller in didUpdateWidget and not updating TextEditingController value when FormControl value changes.

I think all ReactiveXWidget has this problem

[vasilich6107]

Hi @rebaz94
I just faced the same issue
We could make PR to fix this.
The only way to overcome this bug is to use formControlName instead formControl

[vasilich6107]

@joanpablo could you help us with this issue?

[vasilich6107]

@rebaz94 let's make PR
I will help you with tests
Than will be waiting for @joanpablo to merge

[rebaz94]

Hi @vasilich6107
I'm happy to help you but at that time I don't have any time sorry, maybe later. here a good start

The only way to overcome this bug is to use formControlName instead formControl

In my case I'm using formControlName but it not work, what I did to fix this providing formControl directly and because I am using custom widget I create two other widget for status and value listener which detect formControl changes, after that everything works.

here the code

/// This widget same as [ReactiveValueListenableBuilder] except that it update control in
/// didUpdateWidget anytime the widget updates
class SwiftyValueListenableBuilder<T> extends StatefulWidget {
  /// The name of the control bound to this widgets
  final String? formControlName;

  // The control bound to this widget
  final AbstractControl<T>? formControl;

  /// Optionally child widget
  final Widget? child;

  /// The builder that creates a widget depending on the status of the control.
  final ReactiveListenableWidgetBuilder<Object?> builder;

  /// Creates an instance of [SwiftyValueListenableBuilder].
  ///
  /// The [builder] function must not be null.
  ///
  /// Must provide a [forControlName] or a [formControl] but not both
  /// at the same time.
  ///
  const SwiftyValueListenableBuilder({
    Key? key,
    this.formControlName,
    this.formControl,
    required this.builder,
    this.child,
  })  : assert(
            (formControlName != null && formControl == null) ||
                (formControlName == null && formControl != null),
            'Must provide a formControlName or a formControl, but not both at the same time.'),
        super(key: key);

  @override
  State<SwiftyValueListenableBuilder<T>> createState() => _ReactiveValueListenableBuilderState<T>();
}

class _ReactiveValueListenableBuilderState<T> extends State<SwiftyValueListenableBuilder<T>> {
  AbstractControl<T>? _control;

  @override
  void initState() {
    super.initState();
    _control = _getFormControl();
  }

  @override
  void didUpdateWidget(covariant SwiftyValueListenableBuilder<T> oldWidget) {
    super.didUpdateWidget(oldWidget);
    _control = _getFormControl();
  }

  AbstractControl<T> _getFormControl([bool listen = false]) {
    if (widget.formControl != null) {
      return widget.formControl!;
    } else {
      final form = ReactiveForm.of(context, listen: listen);
      if (form == null || form is! FormControlCollection) {
        throw FormControlParentNotFoundException(widget);
      }

      final collection = form as FormControlCollection;
      return collection.control(widget.formControlName!) as AbstractControl<T>;
    }
  }

  @override
  Widget build(BuildContext context) {
    return StreamBuilder<T?>(
      stream: _control!.valueChanges,
      builder: (context, snapshot) => widget.builder(context, _control!, widget.child),
    );
  }
}

[vasilich6107]

@rebaz94 one of the ways to solve this is to use key: UniqueKey(), in controls

[rebaz94]

@rebaz94 one of the ways to solve this is to use key: UniqueKey(), in controls

First time i solved with the key but for performance reason I removed because every time you save the form it rebuilding entirely.

The problem is shown when you have a custom widget and you need ReactiveValueListenableBuilder or ReactiveStatusListenableBuilder to listen to changes.

just passing formControl with custom value listener it work perfectly

[vasilich6107]

@rebaz94
I released https://pub.dev/packages/reactive_text_field
with some kind of a fix...

But reactive_forms definitely require a fix

[vasilich6107]

@joanpablo could you take a look at this issue

This issue is critical when we are dealing with form arrays
If we add 3 items and then delete first one - the fields will not update their values correctly

[vasilich6107]

Here is small video demo of the bug - as you can see I'm deleting the unchecked item but is does not disappear cause checkbox does not react to updated control reference

Screen.Recording.2022-01-31.at.16.41.52.mov

[joanpablo]

Thanks @rebaz94 and @vasilich6107

I will take a look and see all your comments and release a fix

[joanpablo]

Hi @vasilich6107

in the case of the Video Example the issue just got fix if you use a Key for example a ValueKey as I show in the picture:

My question is: why is this solution (of using a Key) worse than completely reimplementing the didUpdateWidget?

Maybe the Video example is not the same issue that @rebaz94 is having. I will continue reading and understanding @rebaz94 issue.

[joanpablo]

Hi @rebaz94,

could you create a simple example that reproduces your issue, maybe copy and paste your code (the one that create the new FormGroup and controls) to see if I can reproduce the error and fully understand the problem?

[joanpablo]

@rebaz94
I need to understand the context in which you are creating the new FormGroup, how you are listening for changes, and how you are building RectiveForm and its children. That's is why an example will be very useful. And I also need to understand why you say that using Keys decrease performance.

[vasilich6107]

@joanpablo
The key trick could be a fix, but it looks more like a workaround.

My overall thoughts about key vs didUpdateWidget are next

• didUpdateWidget is the right place to do the thing cause it was designed for this purposes
• the key solution is not obvious for flutter ecosystem, in flutter way you do not have to add key to every widget to make it function properly. Adding key is more like exception to cover edge cases. If we will not rewrite didUpdateWidget users will have to add key to every control in order to omit possible bugs.

[joanpablo]

Hi @rebaz94,

Another question that I don't stop asking myself is:

Why do you need to rebuild the FormGroup instead of just using the FormGroup.reset(...) method?
This method was intended to reset values of the FormGroup without creating a new one each time you save it. You can reset the complete FormGroup with new values (if you need to load new data from the server) or you can just initialize FormGroup with default values.

In my experience you don't rebuild the complete FormGroup again, you just reset data. You only rebuild if you are creating a completely new Form UI, with new Fields or Completely different ones. In that case, you rebuild the whole ReactiveForm, you don't reimplement the **didUpdateWidget **.

So again I need to understand your use case, could you bring some light to us, please?

[joanpablo]

@joanpablo The key trick could be a fix, but it looks more like a workaround.

My overall thoughts about key vs didUpdateWidget are next

• didUpdateWidget is the right place to do the thing cause it was designed for this purposes
• the key solution is not obvious for flutter ecosystem, in flutter way you do not have to add key to every widget to make it function properly. Adding key is more like exception to cover edge cases. If we will not rewrite didUpdateWidget users will have to add key to every control in order to omit possible bugs.

Hi @vasilich6107, I understand your point of view, but one of the first things that I learned in Flutter is that Keys are extremely important on a list of similar repeated items (and more important if these items are stateful widgets, like the Reactive Form Widgets).

Here in the Flutter Docs at the end of the page they made a clear explanation of the importance of using Keys in a list.

And also we have this video that also explains the use of Keys in a list.

The first versions of the FLutter Web Site had a very explicit example of Using Keys in a List. Unfortunately, they changed the Web Site and Documentation, and now there is no dedicated example on this topic. But the explanation is quite clear anyway.

[rebaz94]

Hi @joanpablo

ReactiveForm does not have any problem when FormGroup change, it will update it but the problem is that FormControl not updated in widget only when formControllerName is been used for example ReactiveTextField or when using ReactiveStatusListenableBuilder and ReactiveValueListenableBuilder does not react to change.

In may case I have many custom widget and some of widget is just wrapper or have own listening to using ReactiveValueListenableBuilder. anytime user save the form it will send to server and reload the content like its been the first time, so at that time it should update all UI to use the new controller but only FormGroup is updated and the widget that uses FormControl use the old controller and form is broken any further updating will be ignored.

We can use the ValueKey(formGroup) to fix that but as @vasilich6107 said Key is not the solution for that because it will rebuild entire tree as opposed to just calling didUpdateWidget.

And I have some component that reused in every form like save button which hold FormGroup state like updaing,failed and state from controller which is a StateNotifier. if I use Key for that I loose all information and user may or may not see what happening, is it form saved or not, or error happen what is the error.

and there is inconsistency between formControlName and formControl, it should do the same thing no matter how you provide controller. if you use formControl you don't have any problem.

What I did to fix this problem, I use this commit and change all widget to use formControl directly and also use my custom Status & Value listenable for creating custom widget

here a simple example

import 'package:flutter/material.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:reactive_forms/reactive_forms.dart';

class TestStateNotifier extends StateNotifier<FormGroup> {
  TestStateNotifier(FormGroup state) : super(state);

  static final provider = StateNotifierProvider<TestStateNotifier, FormGroup>(
    (_) => TestStateNotifier(
      FormGroup(
        {
          'name': FormControl<String>(value: 'rebaz'),
          'email': FormControl<String>(value: 'rebaz@gmail.com'),
        },
      ),
    ),
  );

  FormControl<String> controlOf(String name) {
    return state.control(name) as FormControl<String>;
  }

  int counter = 0;

  void updateForm() async {
    counter++;
    // 1. check form valid or not

    // 2. disable form
    _disableFormWithAnim();

    await Future.delayed(const Duration(seconds: 1));

    state.markAsEnabled();

    // update comes from server
    state = FormGroup(
      {
        'name': FormControl<String>(value: 'Rebe updated: $counter'),
        'email': FormControl<String>(value: 'rebaz@gmail.com'),
      },
    );
  }

  void _disableFormWithAnim() {
    // int i = 0;
    // for (final c in state.controls.values) {
    //   Future.delayed(Duration(milliseconds: i * 50)).then((_) {
    //     if (mounted) c.markAsDisabled();
    //   });
    //   i++;
    // }
    state.markAsDisabled();
  }
}

class TestScreen extends ConsumerWidget {
  const TestScreen({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final formGroup = ref.watch(TestStateNotifier.provider);

    return Scaffold(
      body: ReactiveForm(
        //key: ValueKey(formGroup),
        formGroup: formGroup,
        child: Builder(
          builder: (context) => Column(
            mainAxisAlignment: MainAxisAlignment.center,
            children: [
              ReactiveTextField(
                formControlName: 'name',
              ),
              ReactiveTextField(
                formControlName: 'email',
              ),
              ElevatedButton(
                onPressed: () => ref.read(TestStateNotifier.provider.notifier).updateForm(),
                child: const Text('Update'),
              )
            ],
          ),
        ),
      ),
    );
  }
}

[rebaz94]

Here if I use Key when save button pressed the result will not be shown. so reloading form with new FormGroup or tapping new item from left side is same process and should use the latest value.

reactive.mov

[joanpablo]

Hi @rebaz94,

I will repeat my question:

Have you tried to use FormGroup.reset method, instead of building a completely whole new FormGroup?

Why are you creating a new FormGroup with new FormControls instead of just using the method reset?

As Far As I see it you don't need to create new FormControls, you only need to reset (set again new values) to existing ones.

[joanpablo]

Hi @rebaz94,

Here is the code documentation of the Method reset. Please, Tell me if you have tried this.

/// Resets the control, marking it as untouched, pristine and setting the
  /// value to null.
  ///
  /// In case of [FormGroup] or [FormArray] all descendants are marked pristine
  /// and untouched, and the value of all descendants are set to null.
  ///
  /// The argument [value] is optional and resets the control with an initial
  /// value.
  ///
  /// The argument [disabled] is optional and resets the disabled status of the
  /// control. If value is `true` then if will disable the control, if value is
  /// `false` then if will enable the control, and if the value is `null` or
  /// not set (the default) then the control will state in the same state that
  /// it previously has.
  ///
  /// The argument [removeFocus] is optional and remove the UI focus from the
  /// control. In case of [FormGroup] or [FormArray] remove the focus from all
  /// descendants.
  ///
  /// When [updateParent] is true or not supplied (the default) each change
  /// affects this control and its parent, otherwise only affects to this
  /// control.
  ///
  /// When [emitEvent] is true or not supplied (the default), both the
  /// *statusChanges* and *valueChanges* events notify listeners with the
  /// latest status and value when the control is reset. When false, no events
  /// are emitted.
  ///
  /// ### FormControl example
  final control = FormControl<String>();
  
  control.reset(value: 'John Doe');
  
  print(control.value); // output: 'John Doe'
  ///
  /// ### FormGroup example
  
   final form = FormGroup({
     'first': FormControl(value: 'first name'),
     'last': FormControl(value: 'last name'),
   });
  
  print(form.value);   // output: {first: 'first name', last: 'last name'}
 
  form.reset(value: { 'first': 'John', 'last': 'last name' });
  
   print(form.value); // output: {first: 'John', last: 'last name'}
  
  
  ///
  /// ### FormArray example
 final array = FormArray<String>([
  FormControl<String>(),
  FormControl<String>(),
  ]);
  
 array.reset(value: ['name', 'last name']);
  print(array.value); // output: ['name', 'last name']
  

[joanpablo]

Here if I use Key when save button pressed the result will not be shown. so reloading form with new FormGroup or tapping new item from left side is same process and should use the latest value.

reactive.mov

BTW very nice UI

[rebaz94]

Hi @joanpablo
Thanks for responding. I know about reset method but in that case if you have a lot of fields there will be a lot of duplicate code to handle initial value and later resting with new value compared to just creating new FormGroup.

Fixing this issue will help a lot and its the right way to handle changes. But now the changes does not reflect in the UI

reactive.mov

BTW very nice UI

Thank you, its because of ReactiveForm I can do easily

[joanpablo]

Hi @rebaz94,

Not sure why duplicated Code if you encapsulate the code in a reusable method. Why it is duplicated Code? Not sure to follow you. Can you give me the real example of your app or something similar?

[rebaz94]

Because creating form control is different than updating value in the reset method. If I create control in a method. I need to create another for all control and just use reset method

[joanpablo]

Yes definitely you need one method for creating a FormGroup and another for Reseting the FormGroup, but the source of the Data is the same, no duplication. Those methods may look similar, but they are not the same.

Reactive Forms is not implemented in a way that you constantly change FormControls of the Widgets, because all those widgets are Stateful, and stateful widgets change state value, your state is the Data, not the FormControls. You are trying to use the FormGroup and all its controls as the State but they are not. Is the data the one that changes, everything else remains the same, all your widgets are the same and the FormGroup with its children controls should remain the same. You only need to update The Data of FormGroup, and the way to do this is the Reset method. If you want to share your code I can help you by giving you suggestions about how to reduce boilerplate code, but data is the one that changes not Controls or Widgets

[joanpablo]

@rebaz94,

In fact you don't need this line of code:

@override
Widget build(BuildContext context, WidgetRef ref) {
    final formGroup = ref.watch(TestStateNotifier.provider);
   ....
}

instead you need this one

@override
Widget build(BuildContext context, WidgetRef ref) {
    final formGroup = ref.read(TestStateNotifier.provider);
   ....
}

The use of ref.read, because all changes to data (values of widgets) will be performed by Reactive Forms.

[rebaz94]

You are right, it should use reset method but if you are a lot of fields with different FormGroup things became harder to manage.
There is a lot of dependency in my app can't shared easily but the example I provide, it similar we have a method to load data and if form changes and need to get data again from server it will again call the loadMethod again...

Anyway thanks for your reply. you can close the issue if you want.

[rebaz94]

@rebaz94,

In fact you don't need this line of code:

@override
Widget build(BuildContext context, WidgetRef ref) {
    final formGroup = ref.watch(TestStateNotifier.provider);
   ....
}

instead you need this one

@override
Widget build(BuildContext context, WidgetRef ref) {
    final formGroup = ref.read(TestStateNotifier.provider);
   ....
}

The use of ref.read, because all changes to data (values of widgets) will be performed by Reactive Forms.

this is a simple example, in my case there is a class with different state like Data, Loading, Error and..

[joanpablo]

Yes in this simple example you don't need the ref.watch, maybe if you are listening some other providers you should use read.watch, but my suggestion is always separate Widgets with their specific logic. If the Form Widget will not listen to any other provider then separate it in a Specific Widget that just Read for the Provider. Any other Widget that listens for changes to other providers should be Created In a separated widget with its own refresh logic.

[rebaz94]

Sorry the problem is all about using different FormGroup and when changes the UI use the new ones but it does not do that, it used the old controller. thats all about it. as I said in my case I am fixing by providing controller directly but ReactiveForm should do that

[rebaz94]

Anyway thanks for your reply
Regards

[joanpablo]

@rebaz94 Thanks for the issue and for using Reactive Forms, don't make me wrong.

I now fully understand your issue, and I can tell you that Reactive Forms is intentionally not implemented the way you are proposing. Its not implemented in the way that you constantly change FormControl instances. If you are constantly changing instances of FormControls, then why we would have methods like FormControl.valuesChanges or FormControl.statusChanged or FormControl.touchChanges if all these controls are going to be ephemeral and are going to constantly be destroyed and re-created?

The idea is that You create a FormControl then bind it with a Widget, and then just change Data, and all Widgets will refresh automatically, without New Build of the context at all, just using async events (Streams). If you are Familiar with Angular development you will understand the logic behind Reactive Forms implementation.

But anyway, thanks again and please I am willing to help you to improve your code if you want to share some examples. Not the complete project source code, maybe some methods or any class. I will be glad to help you.

[rebaz94]

@joanpablo I fully understand you, thank you so much for all information. I changed my mind and I will refactor my code to use reset method as its recommended and also I think it should have a better performance that recreating the controller. just some extra code :)