Widget types error message

[solpark]

- Summary
User sees specific error message when widget fields are of wrong data type.
Closes #1074

- Test plan
These are three of the many types of errors.

It should catch wrong data types of widget fields no matter how nested the fields are.

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

[verythorough]

Deploy preview for netlify-cms-www ready!

Built with commit ef599b2

https://deploy-preview-1131--netlify-cms-www.netlify.com

[verythorough]

Deploy preview for cms-demo ready!

Built with commit ef599b2

https://deploy-preview-1131--cms-demo.netlify.com

[erquhart]

Thanks for this @solpark!

First I'll port some relevant comments from Gitter:

Sol Park @solpark Feb 22 12:06
Hi everyone! I was wondering if someone could help me with this issue: #1074 (comment)
I have an idea of how to throw a more descriptive error message when the searchFields is not an array. However that is very specific to just the searchFields widget. Is that okay?

Sol Park @solpark Feb 22 12:30
Or also, the approach I am thinking is to first iterate through the collections, and then for each collection iterate through the current collection's fields array, and then check each type of field, i.e. name and label must be a string and searchFields must be an array

Shawn Erquhart @erquhart Feb 22 15:39
@solpark I think either of those approaches could work, but throwing an error is less involved and might be ideal given our lack of structure for the kind of validation your second option involves.

I misread your first comment a bit and assumed you meant throwing a descriptive error in the console, not sure where I got that from. The approach you took here is the ideal approach, but it's going to require more work to implement because widgets need to provide their own validation rules. For example, there's a rule that says options has to be an array, but a widget could easily start supporting it's own options that takes an object, and the validation would fail.

It's also not a small thing to extend the widget API so that individual widgets can provide it's own config validation.

I'm wondering if this is the right time to take our global error component further and allow various areas of the codebase to trigger it in a controlled manner, or else trigger error components with smaller scopes so that the entire app doesn't shut down (although that's often ideal since most errors are showstoppers).

Thoughts?

cc/ @tech4him1 @talves

[erquhart]

Hey @solpark, is this still something you're able to work on?

[solpark]

@erquhart Hi! Yes, I am still interested to continue to work on this. I was waiting to hear thoughts from the others tagged :)

How should I proceed?

[erquhart]

@solpark so sorry, I thought I responded already!

So this is getting into API stuff, but I'm thinking (very loosely):

• each widget provides an optional config validation function at registration time
• we call the validation function for the widget of each top level field
• widgets that can contain nested fields, e.g. object and list, should call their child widget's validation function

I'm imagining this function being provided in an options object to registerWidget:

function validateConfig(field) {...}

registerWidget('string', StringControl, StringPreview, { validateConfig }

What do you think?

[solpark]

@erquhart Interesting! I would love to take a stab at this.

[erquhart]

Found a problem with my last comment: developers implementing the CMS usually won't be writing those config validation functions - they'll be provided by the widget authors. Having developers retrieve and register that function with each widget that provides it is just a bad idea.

The issue here is sort of built in to Netlify CMS API's - extensions are generally just a registered entity that does one thing, which is intentional and makes things straightforward, but also disallows expected norms for "plugins", like the ability to register additional functionality from with the plugin itself.

Example:

Widget registration right now:

registerWidget(name, ControlComponent, PreviewComponent)

Widget registration as it probably should be:

registerWidget(Widget)

Widget in the above example is a function that would probably return an object specifying the default name, control component, preview component, and potentially all sorts of other bits, like a config validation function.

We're now talking about a major 2.0 initiative, redesign of the widget registration API. My example shows what I think 90% of folks will probably do, no need for custom naming of the registered widget in most cases, and we would also accept an expanded signature for tighter control, probably:

registerWidget({
  name: 'widgetName',
  widget: Widget,
})

Everything could be explicitly registered with that approach, including specific control and preview components as is possible currently.

Thoughts?

If you'd like to play around with any of this while considering, registration happens in lib/registry.js.

[erquhart]

Closing as stale.