[Feature] Support nested objects in schemas out-of-the-box

eric-burel commented 6 years ago

State of the art

For the moment, Vulcan SmartForm does not support nested schemas out the box. A typical use case is handling addresses: it deservers its own field in the object, but often not its own collection.

However, two things are not supported yet:

handling the schema correctly
handling the form generation

A schema with a nested object would look like this:

const schema = new SimpleSchema({
  addresse: {
    type: Object,
  },
  'addresse.street': {
    type: String,
  },
  'addresse.city':{
    type: String,
  }
});

However for the moment Vulcan behaves incorrectly when there are a dot in the fieldName. It sometimes try to build the corresponding gql schema without transforming the name, and GraphQL does not tolerate dots. It also happens with arrays when one tries to create a SmartForm and does not limit fields, so there might be multiple places in the code where gql schemas might be generated incorrectly.

My current working solution is to put a blackbox: true in the object field schema and remove the nested fields schema. This prevent simpl-schema from trying to validate this. I built a custom control component for Object and I pass it the schema directly.

address:{
        type: Object,
        default:{},
        blackbox: true,
        form:{
       // here I must pass the nested schema as a prop so my ObjectForm can build the form
            objectSchema: () => addressSchema, 
        },
        control: ObjectForm,
    },

I then lose the ability to validate the nested object. It is okay for an address but does not scale.

Idea to solve this

Fixing the dot containing field names issue

The issue with dot containing field names in schemas must be solved. It simply needs a condition somewhere but the issue might appear in multiple places when a GraphQL type or schema is built from a simple-schema schema.

Creating an ObjectForm control

A correct ObjectForm (won't share mine, too ugly :)) could be created for free by refactoring the FormComponent component. Indeed the Form component is already able to generate a form for nested object and to create the correct inputs depending on the type, since it does it for the whole schema. However it contains too much logic to be reusable recursively for nested schemas.

Build the nested schema and pass it to the Control

We should be able to pass the nested schema to this ObjectForm automatically. 2 possibilities: reconstruct the schema given the field name. Here, we simply need to find any address.[subfieldName] field and build an object with them, that would be our schema.

Another alternative is to rely on the possibility to pass a schema as the type:

address:{
  type: {
    street: {
       type: String,
    }
   ...
  }
}

This syntax is a bit simpler though I think it is more common to create separate fields with the name convention as shown earlier (address.street and so on).

So, there is not actually not that much to do to get a working solution, mostly bugfixing and refactor, and we could obtain an awesome form generator that supports nested objects :)

eric-burel commented 6 years ago

Also the Datatable and Card could benefit from this.

SachaG commented 6 years ago

This is a great idea. Maybe a good place to start would be adding a new example-nested-schema package to the starter repo containing a full nested schema? Then we can work from there considering what's missing to make it work properly.

eric-burel commented 6 years ago

Nice idea I'll do that as soon as I have some spare time, maybe next week. I think I'll create a more general example-complex-schema because that would be a good place to show altogether examples with arrays, custom structure, callbacks, resolver to distant APIs, custom fields, components in the form options and so on, more broadly complex data structures that you can find in a real life.

Edit: just so I remember the use case I think about:

Nested schemas, to describe complex objects
Refactor the FormCompoent to make the input switch reusable, allow to create smarter custom form for nested stuffs
Arrays, arrays of object
Facultative fields, activated only when something is checked
Stepped form, form with groups of input (the group prop is already there but examples would be nice)
Semi-mandatory field: sometime you'd like to have an escape hatch for fields that are mandatory. Example: I want a photo to be mandatory. But storing photos rely on Cloudinary, which might be sometime unavailable. So I would rather like to make the photo optional, but raise a warning, e.g with a modal if the user try to submit the form with photos.

SachaG commented 6 years ago

Great idea :)

eric-burel commented 6 years ago

Hi, here is the PR : https://github.com/VulcanJS/Vulcan-Starter/pull/19.

This is a draft, the custom controls do not show up yet, but the code is there and could serve as a working basis.

SachaG commented 6 years ago

I spent some time working on this and it almost works :) I had to do a few major changes to how SmartForm works. Mainly:

Every component is now a controlled component. The data object returned by Formsy is not used at all anymore, you have to update currentValues instead.
The field object passed to each FormComponent is stateless, i.e. it never changes. Meaning it doesn't include the field's value, errors, or anything else that might change. Instead, you get those via context.
Form now works with a regular JSON object converted from the SimpleSchema schema, instead of with the SimpleSchema object directly.
The form now supports nesting, using two strategies to get/set data: either use Lodash's get/set/unset with a dotted path (addresses.0.street), or else flatten everything so every field can be accessed from the root (flatSchema.street).

eric-burel commented 6 years ago

This is really excellent, where can we see it/test it? I'll update my example in the starter, maybe this week or next one.

I did not read the code yet but this architecture sounds perfect. I had fun with complex nested forms last week on another project, and this is quite similar to what I ended to imagine after a lot of struggling.

And where is the context/state stored? I ask that because I also noticed on the same project (it does not use Vulcan though, its a React/redux app) that when your reducers goes big, you quickly end up with a few ms latency, and this is quite noticeable with controlled components. Worse, if it triggers rendering of complex element, you can in some cases have fun with this issue https://github.com/facebook/react/issues/955

So I ended storing the state/context as close to the component I could. I guess that's what you already did but just to point that the famous React saying "you should only use controlled input" is only true if the state lives not far from the input.

SachaG commented 6 years ago

It's live on the devel branch, and in example-forms on the starter repo. I ended up modifying a lot of your package code, mainly to simplify it while I was working on the basic logic. But maybe once we're sure it works we can add some of the other features back so we can showcase more of what SmartForm can do.

And the context/state is stored in the Form component. It's true that I've noticed some lag in typing with big forms, so performance is not perfect. But I figured we could work on that later. I actually managed to make performance much better by debouncing change events, but then the problem is that you might miss some events if you submit the form before the debounced function runs…

eric-burel commented 5 years ago

Can be improved but working ok so far, so I close this issue

VulcanJS / Vulcan