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

Dynamic forms #43

Closed
alex35mil opened this issue Dec 16, 2018 · 16 comments
Closed

Dynamic forms #43

alex35mil opened this issue Dec 16, 2018 · 16 comments

Comments

@alex35mil
Copy link
Member

alex35mil commented Dec 16, 2018

After poking around dynamic forms implementation I don't see a sane way how to implement it w/o code generation.

I haven't figured out neither public api or exact internal implementation, just a rough prototype of config dsl:

module NewArticleForm = [%form {|
  Title: string
  Body: string
  @nest Authors: {
    Name: string
    Url: string?
  }
|}];

I think from this I can get all data I need to generate proper field, state and some more internal types and lets to make dynamic forms possible but before going into implementation of this dsl I need to figure out internal implementation first. Also, it will require validators to be passed via props which is not a big deal I guess (comparing to the bloated and confusing config w/o dsl).

Not jumping into implementation yet and appreciate any feedback on it.

@lewisf
Copy link

lewisf commented Dec 29, 2018

Forgive me if this is a bad question, but what would passing validators as props look like?

@alex35mil
Copy link
Member Author

@lewisf It should be the same way as it is right now but list of validators is passed via props.

module Validator = {
  let email = ...
  let password = ...
};

render: _ =>
  <FormContainer
    ...
    validators=[
      Validator.email,
      Validator.password,
    ]
  />

@baransu
Copy link

baransu commented Jan 4, 2019

graphql_ppx uses GraphQL directives to pass pure functions for converting raw GraphQL data into more friendly Reason code. https://github.com/mhallin/graphql_ppx/blob/master/README.md#custom-field-decoders

For me it looks similar and could be possible solution to passing validators.
It all depends if it is a good thing in case we want to reuse form with different validators, then its harder.

@alex35mil
Copy link
Member Author

@baransu it's an option 👍

@baransu
Copy link

baransu commented Jan 8, 2019

I’ll be working a little bit on dynamic forms in the next week so I’ll try to share my feedback with some code samples :)

@alex35mil
Copy link
Member Author

Another option is not dealing w/ custom DSL and use record and attributes:

module NewArticleForm = [%form {
  title: string,
  body: string,
  [@nest] authors: {
    name: string,
    url: option(string),
  }
}];

But in this case nested array item can't be defined inline :(

Prolly, we don't even need [@nest] b/c we can match array type:

module NewArticleForm = [%form {
  title: string,
  body: string,
  authors: array(Author.t),
}];

@alex35mil
Copy link
Member Author

Scratch that, it won’t be possible to parse nested records.

@baransu
Copy link

baransu commented Jan 12, 2019

I've been playing with complex dynamic forms over the last few days that here is what I came up with.


My schema is rather complex. It was initially the same schema as in #34:

type state = {
  name: string,
  parts: list(part)
}
and part = {
  image: string,
  title: string,
  subtitle: string,
  buttons: list(button),
}
and button =
  | UrlButton(url)
  | PluginButton(plugin)
and url = {
  id: string,
  caption: string,
  url: string,
}
and plugin = {
  caption: string,
  pluginId: string,
  instanceId: string,
};

It's complex. I decided there is no way I can handle in the two-level nested record with reasonable API. So I changed the schema a little bit to have a better structure for my form. I ended up with this:

type state = {
  name: string,
  parts: list(part),
  buttons: list(button),
}
and part = {
  image: string,
  title: string,
  subtitle: string,
  buttons: list(string),
}
and button =
  | UrlButton(url)
  | PluginButton(plugin)
and url = {
  id: string,
  caption: string,
  url: string,
}
and plugin = {
  caption: string,
  pluginId: string,
  instanceId: string,
};

Right now it's better. Next step was to come up with a suitable field type:

type field =
  | Name
  | ButtonUrl(string)
  | ButtonCaption(string)
  | ButtonPlugin(string)
  | ButtonContent(string)
  | Part
  | PartImage(string)
  | PartTitle(string)
  | PartSubtitle(string)
  | PartButtons(string);

Thanks to current re-formality API - field can be anything so for record like state it's great to use variant and for something like map schema (where state is just a map) we could use map key or something similar.

Now it's a time to write code that updates correct elements in a list. But there comes a realization. We have to define our validators upfront. That's why I had to make some changes to re-formality itself by changing how form.change works:

  1. I changed change signature to accept update function rather than updated value. I'm not sure if it's ok but for me, it simplified the code a lot because I could write update logic in field module and do not worry about passing form.state in the correct place. I think is boilerplate code that can be updated
  2. I moved validators from Make functor to <Form/> props. It's connected with Add enableReinitialize functionality #33 and to have dynamic form work with payload updated from the server I had to generate validators for every field in my schema.
  3. I added validatorsToAdd and validatorsToRemove as a optional labeled arguments. That way, when updating state (by adding/removing or changing type of the value we want to validate) we can specify additional validators we want to add or remove. validatorsToRemove is defacto list of field so to be consistent it should accept whole validator record.
  4. I made field in form.change a labeled argument. With 4 arguments it's better to limit not labeled arguments to a minimum and having update function as only not positional arguments we can pipe it with both |> and ->.

The current implementation works for my use case and I believe that with proper ppx we can reduce boilerplate a lot. Here is part of the application to present our solution to this problem: https://gist.github.com/baransu/4ff1086014693fe27c562d435c1ee461

@baransu
Copy link

baransu commented Mar 4, 2019

@alexfedoseev do you think there is anything we can move into dynamic forms direction?

@alex35mil
Copy link
Member Author

@baransu Sorry for the delay, the current state of things is: I have a draft of types for lexer and tokenizer and it's pretty much it.

My current situation is the following: I have a 2 main projects I'm working on right now—the client one and the personal one. The latter, the main consumer of this library, is in the pre-release mode and I need 1-2 months to clean things up for MVP. Besides the time concern, TBH I don't feel comfortable pushing such a big change without a solid test suite right before release so I would postpone this for some time. I'm really sorry for dragging this for so long but I have to finish some more prioritized work at the moment.

@baransu
Copy link

baransu commented Mar 7, 2019

@alexfedoseev No problem. I have a fork that I'm maintaining for my company project and it works really well. Just today I've changed dynamic validators thing and I'm testing how it works on few forms. I'll post my observations and changes in this issue for the future.

I hope your personal project will be great! Let's get back to the topic once you have time again 🙂

@alex35mil
Copy link
Member Author

Sounds good, thanks for understanding!

@baransu
Copy link

baransu commented Apr 13, 2019

Since I write my description of the changes I've made a few things changed and I would really like to describe how our fork looks right now.

  1. I added validatorsToAdd and validatorsToRemove as a optional labeled arguments. That way, when updating state (by adding/removing or changing type of the value we want to validate) we can specify additional validators we want to add or remove. validatorsToRemove is defacto list of field so to be consistent it should accept whole validator record.

We made one significant change to that. We replace that with concept of optionaly regenerating validators after some change.
Regenerate function (or generate function) looks as follows:

let genValidators: Form.state => list(Form.validator)

and it can be passed as a optional argument to change function:

type state = {
  firstName: string,
  lastName: string
};

type field = | FirstName | LastName

let updateFirstName = (firstName, state) => {...state, firstName};

let initialState = {firstName: "", lastName: ""};

let genValidators = _state => [
     /* firstName and lastName validators */
  ];

let initialValidators = genValidators(initialState);

let change = firstName => 
  form.change(
    ~field=FirstName,
    /* it should probably be named regenerateValidators or something like this */
    ~validators=genValidators,
    updateFirstName(firstName)
  );

It easly allow us to regenerate validators when form shape changes - for example by adding new element to list or removing one. It's also useful if items in your list change shape for example which is sometimes a case.

You can check current state of the fork here: https://github.com/baransu/re-formality

I really count on the feedback as I would really like to introduce those changes to the upstream repository so other can benefit too 🙂 And I think it's a starting point to think about ppx to reduce boilerplate as a this issue stands 🙂

@alex35mil
Copy link
Member Author

@baransu In my initial sketch, I thought about making validators kinda static b/c validators list itself doesn't change over time, form state does. So you can provide all of them at the very beginning and never think about them afterwards. The main problem here is to represent array of entities in the current state. So internal structure needs to be changed to support these entities. Basically, groups of fields. And the config become very verbose for simple cases (which are the most common). This is why I want to introduce DSL (tho I would be very happy not to do so). Does it make sense?

@baransu
Copy link

baransu commented Apr 14, 2019

That makes a lot of sense but to be honest I have no idea how to handle those arrays with static set of validators.
My solutions works with basiclly any kind of form including arrays/maps/changing item types in an array. I think DSL could be an option to reduce boilerplate but I dont known how it should look like and what code it should generate.

What do you think about my solution in particular? I’m trying to think about drawbacks. It works for our usecase but I would be happy to improve it to work for other people cases too

@alex35mil
Copy link
Member Author

I published RC (re-formality@next) with collections support. Closing this one. Let's proceed with the new issue if needed.

@alex35mil alex35mil unpinned this issue Mar 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants