React-jsonschema-form: Refactoring ideas regarding the template field, fields and widgets

[olzraiti]

I have some refactoring ideas below. I could have written issues for each idea, but I think the issues are interconnected and it is easier to communicate the main goal in a single post.

The fields registry would contain only single functions for representing a single json schema primitive type. The fields registry would then have a well defined purpose. I'm guessing this was the original purpose for the fields registry, but then DescriptionField and TitleField were added so users could control how they are rendered. Currently the DescriptionField and TitleField are not representing primivite schema types but data inside a schema.

The TemplateField system could be extended so that it would handle also titles and descriptions and let the fields registry handle just the primitive types. After #341 has landed, the TemplateField can handle descriptions. Once it has landed, I suggest that we deprecate the DescriptionField (after #341, the DescriptionField will be useful only if you wan't to create a custom description but not a custom template field - this serves only little use because writing a template field isn't much more difficult or time consuming than writing a custom description field. The benefits are small compared to the complexity it adds).

To get rid of the TitleField also, we could add a template field system to the ArrayField and ObjectField. I'm looking at the code of ObjectField and ArrayField, and ObjectField looks like it could be pretty easily refactored to use a template field. ArrayField looks pretty complex, but it should be possible. I think a template field could maybe simplify ArrayField quite a lot (we could for example drop the reordering functionality and pass the onChange() function to the template field, and users who need the ordering could implement it themselves). The ArrayField contains now some hard coded bootstrap elements - the template field system would allow users to choose how they want to render array rows and buttons.

Another way to let users rewrite ArrayField easily would be to make it possible to override it through the fields property. I think a template field would be more usable for users, since most likely they want to just override how the component renders, not the well written and tested logic behind it.

Though allowing to override all fields and widgets would make sense in any case. There is discussion about that in #283 and #310. I suggested #310, and I still think it would be a good way to go - if a user wants to replace the CheckboxWidget, he could just provide a custom CheckboxWidget to the widgets property and the default BooleanField would render the provided CheckboxWidget - the user would again benefit from the well written and already tested logic.

These changes would make this library a flexible backbone with sane defaults that are overridable to the core in a way that let's you override only the relevant stuff (usually the React components rendering) if the logic is otherwise working OK.

[n1k0]

I'm guessing this was the original purpose for the fields registry, but then DescriptionField and TitleField were added so users could control how they are rendered. Currently the DescriptionField and TitleField are not representing primivite schema types but data inside a schema.

Yes. I was uncomfortable landing the title and description field patch, though this was apparently critically needed for the formbuilder back in the days. I should probably have challenged a little more this design decision; I agree with you, these have nothing to do with jsonschema value primitives and make things harder to organize, document and maintain than needed.

After #341 has landed, the TemplateField can handle descriptions. Once it has landed, I suggest that we deprecate the DescriptionField
To get rid of the TitleField also, we could add a template field system to the ArrayField and ObjectField.

This sounds really good. I've been considering removing the title and description fields but was afraid with such a breaking change as people seem to be using them. This would provide quite an acceptable backup plan, if properly documented.

I'm looking at the code of ObjectField and ArrayField, and ObjectField looks like it could be pretty easily refactored to use a template field.

Never thought about that but it makes sense.

ArrayField looks pretty complex, but it should be possible. I think a template field could maybe simplify ArrayField quite a lot

That'd be amazing to get, though I agree this sounds like quite a heavy refactoring. Do you feel like working on this? I could provide reviews and guidance but not much more as I have a bunch on my plate with other projects already.

Another way to let users rewrite ArrayField easily would be to make it possible to override it through the fields property. I think a template field would be more usable for users, since most likely they want to just override how the component renders, not the well written and tested logic behind it.

Yeah, theming is currently the biggest pain point with using the lib, we should focus on making this as simple yet flexible as possible. For advanced use cases there is always custom fields and widgets.

Though allowing to override all fields and widgets would make sense in any case. There is discussion about that in #283 and #310. I suggested #310

The good news is that it's in a good shape for landing :)

These changes would make this library a flexible backbone with sane defaults that are overridable to the core in a way that let's you override only the relevant stuff (usually the React components rendering) if the logic is otherwise working OK.

We're perfectly aligned here :)

[maartenth]

This sounds really good @olzraiti, hope you find time to implement this.

After this is implemented, I would like to implement additionalProperties: {schema} for objects just like additionalItems for arrays, with add and remove buttons for properties. The extracted field template for arrays would be reusable for objects, I think.

[dehli]

Hey @olzraiti, I'm not sure how far you've gotten with this but I'm in need of something like this as well (specifically using it to customize the rendering of arrays). I'm going to take a stab at it and will keep you updated!

[olzraiti]

@dehli That's excellent! I haven't done any work on extending the template field system to object and arrays, so there won't be any work in vain on my part. In fact, I probably wouldn't had time for this for a while, since the pull requests that I contributed to the latest release should allow me to finally start following the upstream project (until now I've used a private fork) and at the moment I wouldn't benefit enough from the extended template fields to make it worth the effort. Good luck! :)

[dehli]

Hey @olzraiti & @maartenth, I finally got around to implementing the custom arrays and would appreciate your input with how it was designed / feedback in how it can be better. I used ui:options but I'm not sure if that is the best solution. Thanks! #395

[crumblix]

I've tried out dehli's patch and it works great (Thanks Dehli! :) ). I was of a similar mind though, ui:options may or may not be the best solution. I suspect it's the most contentious part. It may depend on the long term direction UI wise. But the bulk of the patch looks like a big improvement to me.

Even if you decide not to extend the public interface via ui:options, it makes it potentially so much easier to make your own custom array widget. I posted a quick and dirty proof of concept against dehli's PR.

So even if there isn't agreement on the ui:options part, I like the look of it at the as a refactor. :)

[n1k0]

395 & #437 have landed, I'm closing this.

[MatejBransky]

Related #899