Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Form generator and package roadmap #176

Closed
vasilich6107 opened this issue Jul 12, 2021 · 28 comments
Closed

Form generator and package roadmap #176

vasilich6107 opened this issue Jul 12, 2021 · 28 comments

Comments

@vasilich6107
Copy link
Contributor

vasilich6107 commented Jul 12, 2021

Hi @joanpablo
I have an idea to write generator which will help to overcome some disadvantages of JSON access to field values
described in this issue #28

Something like

@ReactiveForm
User() {

}

Also I would like to discuss with you if there is a possibility to create some roadmap about form engine features to implement)

If you are ok with that ping me in discord vasilich6107#1453

@joanpablo
Copy link
Owner

Hi @vasilich6107,

Thanks a lot for your help, I've been quite busy recently. Your idea sounds interesting. I would like to understand it better. So I will ping you as soon as I can. I cannot promise you it will be today, but I will let you know as asap.

Thanks a lot. Meanwhile, if you like to argue a little more here I can read a take a better understanding before we talk in discord.

@vasilich6107
Copy link
Contributor Author

vasilich6107 commented Jul 12, 2021

Let me try to give a more detailed example.

Let's assume that we have Login model

@ReactiveForm
class Login {
  final String email;
  final String password;

  Login({
    @FormControl(
      validators: [], /*and other possible params*/
    )
        required this.email,
    @FormControl(
      validators: [], /*and other possible params*/
    )
        required this.password,
  });
}

This will generate class something like

class LoginForm {
  static const String emailControlName = 'email';
  static const String passwordControlName = 'password';

  late FormGroup form;

  LoginForm() {
    form = fb.group(_formElements);
  }

  Map<String, Object> _formElements = {
    LoginForm.emailControlName: FormControl<String>(
      value: null,
      validators: [Validators.email],
    ),
    LoginForm.passwordControlName: FormControl<String>(
      value: null,
      validators: [Validators.required],
    ),
  };

  String? get email => form.value[LoginForm.emailControlName] as String?;

  String? get password => form.value[LoginForm.passwordControlName] as String?;

  bool get containsEmail => form.contains(LoginForm.emailControlName);

  bool get containsPassword => form.contains(LoginForm.emailControlName);

  Map<String, Object>? get emailErrors => form.errors[LoginForm.emailControlName] as Map<String, Object>?;

  Map<String, Object>? get passwordErrors => form.errors[LoginForm.passwordControlName] as  Map<String, Object>?;

  void focusEmail() => form.focus(LoginForm.emailControlName);
 
  void focusPassword() => form.focus(LoginForm.passwordControlName);

  FormControl<String> emailControl() => form.control(LoginForm.emailControlName);
 
  FormControl<String> passwordControl() => form.control(LoginForm.passwordControlName);

  void removeEmail({bool updateParent = true, bool emitEvent = true}) => form.removeControl(LoginForm.emailControlName, updateParent: updateParent, emitEvent:emitEvent);
 
  void removePassword({bool updateParent = true, bool emitEvent = true}) => form.removeControl(LoginForm.passwordControlName, updateParent: updateParent, emitEvent:emitEvent);
///////=========
  void updatePasswordValue(String password, {
    bool updateParent = true,
    bool emitEvent = true,
  }) => form.updateValue({
    LoginForm.passwordControlName: password,
  }, updateParent: updateParent, emitEvent: emitEvent);

  void updateEmailValue(String email, {
    bool updateParent = true,
    bool emitEvent = true,
  }) => form.updateValue({
    LoginForm.emailControlName: email,
  }, updateParent: updateParent, emitEvent: emitEvent);

  void updatePasswordValue(String password, {
    bool updateParent = true,
    bool emitEvent = true,
  }) => form.updateValue({
    LoginForm.passwordControlName: password,
  }, updateParent: updateParent, emitEvent: emitEvent);

  void patchEmailValue(String email, {
    bool updateParent = true,
    bool emitEvent = true,
  }) => form.patchValue({
    LoginForm.emailControlName: email,
  }, updateParent: updateParent, emitEvent: emitEvent);

  void patchPasswordValue(String password, {
    bool updateParent = true,
    bool emitEvent = true,
  }) => form.patchValue({
    LoginForm.passwordControlName: password,
  }, updateParent: updateParent, emitEvent: emitEvent);

  void value(Login login) => form.value({
    LoginForm.emailControlName: login.email,
    LoginForm.passwordControlName: login.password,
  });

  //////////////////////////
  //// TO THINK ////
  //////////////////////////
  // rawValue
  //
  // errors
  // resetState

  Login get data {
    if (form.valid) {
      // need to check if fields have validators Validators.required before using `!`
      return Login(email: email!, password: password!);
    }

    // trying to access non valid form
    throw Error();
  }
}

@vasilich6107
Copy link
Contributor Author

@tgbarker @marcos930807 join to our discussion of the generator

@vasilich6107
Copy link
Contributor Author

vasilich6107 commented Jul 12, 2021

I suppose to start initial implementation in one or two weeks)
I think there will be an alpha version first with tons of breaking changes during implementation)
Possibly we could achieve more stable beta release till the end of the year

@kuhnroyal
Copy link
Contributor

I actually wrote a similar generator for internal use, so I can't share fully.
It started the same way, by defining the backing model and its fields. This proved to be more complicated than expected.
You can not always infer the correct FormControl type from the model value. A List<String> can be a FormControl<List<String>> or a FormArray<String>. Of course you can handle this with additional annotations.

But after the initial tests, I switched to defining the FormGroup with its controls and generating everything from there.

abstract class FormModel<M extends FormModel<M>> {
  Map<String, Object?> toMap();
}

abstract class FormGroupWithModel<M extends FormModel<M>> implements FormGroup {
  M getModel();
  void setModel(M model, {bool updateParent = false, bool emitEvent = false});

  /// Called after addAll(controls)
  void postProcessControls();
}
abstract class LoginFormModel implements $LoginFormModel {
  static const _$LoginFormModelBuilder builder = _$LoginFormModelBuilder();

  // Mapping business model to form model, can be done somewhere else.
  static LoginFormModel build({@required Login login}) {
    return LoginFormModel.builder.build(
      username: login.user,
      password: login.password,
    );
  }
}

abstract class LoginForm implements FormGroupWithModel<LoginFormModel> {
  FormControl<String> get username;
  FormControl<String> get password;

  factory LoginForm() = $LoginForm;

  @override
  void postProcessControls() {
    username.setValidatorsWithoutUpdate([
      Validators.required,
      Validators.minLength(8),
    ]);
    ...
  }
}

The controls are automatically added empty and can be updated afterwards with validators and so on in postProcessControls().

You construct a LoginForm(), you map your Login model to the LoginFormModel and call form.setModel(model) with it.
There are a couple more helper functions in play but this works pretty well.

There are no more string names for fields, you can just access the typed controls.

MagicFormConnector<LoginForm, LoginFormModel>(
  // magic that handles constructing form, model and its updates, depends on state management
  builder: (BuildContext context, LoginForm form) {
    return Column(
      children: [
        ReactiveTextField(
          formControl: form.username,
          ...
        ),
        ReactiveTextField(
          formControl: form.password,
          ...
        ),
      ],        
    );
  }
)

Just some food for thought, hope it helps.

@vasilich6107
Copy link
Contributor Author

@kuhnroyal thanks for insights about FormControl<List<String>> or a FormArray<String>

@vasilich6107
Copy link
Contributor Author

It seems that code generation will require library split into packages.
The dependency on flutter sdk break the code generation.
Also for annotations validation functions should be static…

@vasilich6107
Copy link
Contributor Author

@joanpablo
I asked one guru and he confirmed that code generation requires no flutter deps
I’m working on code gen with mocks

when I will finish I will give you a link to repo.
Then I’ll prepare migration to separate packages

@vasilich6107
Copy link
Contributor Author

vasilich6107 commented Jul 20, 2021

Hi @joanpablo

I'm writing generator.

It already produces from

import 'package:reactive_forms_annotations/reactive_forms_annotations.dart';

@ReactiveForm()
class Login {
  @FormControl(
    validators: const [
      NumberValidator.validate,
    ],
    asyncValidators: const [
      uniqueEmail,
    ],
  )
  final String email;

  @FormControl(
    validators: const [
      NumberValidator.validate,
    ],
    asyncValidators: const [
      uniqueEmail,
    ],
  )
  final String password;

  @FormArray(
    validators: const [
      NumberValidator.validate,
    ],
    asyncValidators: const [
      uniqueEmail,
    ],
  )
  final List<String> categories;

  Login({
    this.email = 'default@e.mail',
    required this.password,
    required this.categories,
  });
}

this

// GENERATED CODE - DO NOT MODIFY BY HAND

// **************************************************************************
// ReactiveFormsGenerator
// **************************************************************************

import 'package:reactive_forms/reactive_forms.dart';

class LoginForm {
  LoginForm(Login login) {
    form = fb.group(_formElements(login));
  }

  static String email = "email";

  static String password = "password";

  static String categories = "categories";

  late FormGroup form;

  String get emailValue => form.value[LoginForm.email] as String;

  String get passwordValue => form.value[LoginForm.password] as String;

  List<String> get categoriesValue =>
      form.value[LoginForm.categories] as List<String>;

  bool get containsEmail => form.contains(LoginForm.email);

  bool get containsPassword => form.contains(LoginForm.password);

  bool get containsCategories => form.contains(LoginForm.categories);

  Map<String, Object> _formElements(Login login) => {
        LoginForm.email: FormControl<String>(
            value: login.email,
            validators: [NumberValidator.validate],
            asyncValidators: [uniqueEmail]),
        LoginForm.password: FormControl<String>(
            value: login.password,
            validators: [NumberValidator.validate],
            asyncValidators: [uniqueEmail]),
        LoginForm.categories: FormArray<String>(login.categories,
            validators: [NumberValidator.validate],
            asyncValidators: [uniqueEmail])
      };
}

@vasilich6107
Copy link
Contributor Author

There is no big problem to generate other methods
I would like to discuss with you the splitting reactive_forms into separate packages.

@vasilich6107
Copy link
Contributor Author

Added support of bundled widget builder

class LoginFormBuilder extends StatefulWidget {
  LoginFormBuilder(
      {Key? key,
      required Login model,
      Widget? child,
      required Widget Function(
              BuildContext context, LoginForm formModel, Widget? child)
          builder});

  @override
  _LoginFormBuilderState createState() => _LoginFormBuilderState();
}

class _LoginFormBuilderState extends State<LoginFormBuilder> {
  late LoginForm _form;

  @override
  void initState() {
    _form = LoginForm(widget.model);
    super.initState();
  }

  @override
  Widget build(BuildContext context) {
    ReactiveFormBuilder(
      child: widget.child,
      form: () => _form.form,
      builder: (context, form, child) {
        return widget.builder(context, _form, widget.child);
      },
    );
  }
}

@vasilich6107
Copy link
Contributor Author

@vasilich6107
Copy link
Contributor Author

Hi @joanpablo
I create PR to illustrate how package split could be done.
The next step I will prepare real example with code generartion

@vasilich6107
Copy link
Contributor Author

@vasilich6107
Copy link
Contributor Author

Please comment)

If you want to try it inside your project just refer to this setup

dependencies:
  flutter:
    sdk: flutter
  reactive_forms_annotations:
    path: ../../reactive_forms_annotations
  reactive_forms:
    git:
      url: https://github.com/artflutter/reactive_forms.git
      ref: package-split
      path: packages/reactive_forms


  # The following adds the Cupertino Icons font to your application.
  # Use with the CupertinoIcons class for iOS style icons.
  cupertino_icons: ^1.0.3

dependency_overrides:
  reactive_forms_core:
    git:
      url: https://github.com/artflutter/reactive_forms.git
      ref: package-split
      path: packages/reactive_forms_core

dev_dependencies:
  reactive_forms_generator:
    path: ../../reactive_forms_generator
  flutter_test:
    sdk: flutter
  build_runner: ^2.0.6

@vasilich6107
Copy link
Contributor Author

The example uses lates 10.5.0 version of reactive forms

@joanpablo
Copy link
Owner

joanpablo commented Jul 22, 2021

Hi @vasilich6107,

Sorry for not answered before. I would like to make some tests in my local env and see the generator in action and see the advantages and disadvantages before including it as part of the official package. So I need to take a look at your great work, and examples.

Meanwhile, I have my first question: Should I be able to use Reactive Forms in a traditional way, as before the generator? Or it's going to be mandatory for the use of the generator and annotations to start using Reactive Forms? Is it going to be compatible with previous versions of the package?

@kuhnroyal
Copy link
Contributor

@joanpablo I don't think the generator is a requirement to use reactive_forms. And it shouldn't be.

@vasilich6107 I understand why you want to split the library but can you instead try using const TypeChecker.fromUri('package:reactive_forms_annotations/src/form_array_annotation.dart#FormArrayAnnotation') (not sure about the correct factory function). This does prevent the runtime dependency and doesn't pull in Flutter.
Same works for types in reactive_forms as well.

@vasilich6107
Copy link
Contributor Author

@joanpablo I answered to @kuhnroyal here #189 (comment)

The next comment will be an answer on @joanpablo questions

@vasilich6107
Copy link
Contributor Author

Hi @joanpablo
Thanks for getting along with my work.

Generator will be an assistive tool. You will be able to use ReactiveForms as before.
There will be no difference for the user.
The only difference is that on pub.dev will be two packages

  • reactive_forms_core - the part that do not require flutter as dependancy
  • reactive_forms - the same as before.

I described in this comment why I need this split #189 (comment)
Before writing this comment I even tried to move required typedefs and AbstractControl in separate files and import them directly via reactive_forms/src/path_to_file to avoid importing flutter as dependancy.
No luck.

Some words about the reasoning.

First of all json naming. Which I described here #28
It is painful to write a separate class to have a single source of truth for names. Maybe this is not painful for you but I suppose it is better to have an ability to give boring work to code generator rather than writing it by myself.
As I mentioned earlier there still will be a possibility to use ReactiveForms as before, let's say in manual mode.

Second reason is this line of code

class FormGroup extends AbstractControl<Map<String, dynamic>>

it resolves into this

final document = DocumentInput(
      file: imageFile != null
          ? UploadFileInput(
              file: imageFile, contentType: imageFile.contentType.mimeType)
          : null,
      subTypeId:
          (form.value[CertificateFormFields.subType] as DocumentSubTypeMixin)
              ?.id,
      documentNumber:
          (form.value[CertificateFormFields.documentNumber] as String),
      countryIsoCode:
          (form.value[CertificateFormFields.country] as CountryMixin)
              ?.isoAlpha2,
      countryOfIssueIsoCode:
          (form.value[CertificateFormFields.country] as CountryMixin)
              ?.isoAlpha2,
      issueDate: (form.value[CertificateFormFields.issueDate] as DateTime)
          ?.toUtcWithOffset(),
      vesselId: (form.value[CertificateFormFields.vessel] as VesselMixin)?.id,
      score: score != null && score.isNotEmpty ? double.tryParse(score) : null,
      expiryDate: (form.value[CertificateFormFields.expiryDate] as DateTime)
          ?.toUtcWithOffset(),
      typeId: DocumentType.certificate.id,
    ).toJson();

It is very painful.
Cause I need to remember all the types behind the field names.
Cause there is no static analysis while coding.

Code generator aims to cover this two issues.

The naked truth is that form.value['login'] as String is ok for login screen or pet project but this is not ok for the big forms or enterprise projects.

What I'm trying to achieve with code generation is to have a model that describes form state with strong typing and to get this model back fulfilled with values.

@vasilich6107
Copy link
Contributor Author

There is also a question What if I fail to finish the generator?

One answer is "The package will still be usable without generator"
Another one "Package split could be reverted without any difference to the user. As far as user will be using reactive_forms as main dependency and all content of reactive_forms_core is reexported from reactive_forms

@vasilich6107
Copy link
Contributor Author

vasilich6107 commented Jul 22, 2021

The roadmap is quite simple:

  • release splitted packages
  • add full support for flat forms
  • release as beta for public use and bug fixes
  • nested forms support (aka nested FormGroups)

@joanpablo
Copy link
Owner

HI @vasilich6107,

Thanks again for all the work you are doing and all the help you give, not only to me, because I feel this package is not mine anymore, but of the community.

I understand now the issues you want to tackle with the generator. I'm just concerned about two things:

1-) I want the package Reactive Forms to be used as always, without the need for the generator (the generator as an optional independent package). As far as I understand from your previous comment, this is possible.

2-) I don't feel comfortable with the idea of splitting the actual Reactive Forms into two separated packages reactive forms and reactive forms core. I feel this will bring confusion to the actual and futures consumers of the library.

I read in the PR comments that @kuhnroyal has a solution that doesn't require splitting the package, is that right or I missed something?

I hope you can understand my concerns.

Best regards.

@vasilich6107
Copy link
Contributor Author

Hi. Thanks for your words!

You are totally correct. We do not need to split the package.

@vasilich6107
Copy link
Contributor Author

@vasilich6107
Copy link
Contributor Author

Added form groups support

@vasilich6107
Copy link
Contributor Author

@vasilich6107
Copy link
Contributor Author

improved support of form arrays + readme sample

https://github.com/artflutter/reactive_forms_generator#dynamic-forms-with-formarray

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants