React-jsonschema-form: There should be a single way to provide widget options in uiSchema

Created on 2 Nov 2016  路  11Comments  路  Source: rjsf-team/react-jsonschema-form

We currently have two distinct ways to provide custom widget options in uiSchema:

  • Through the object format for the ui:widget directive,
  • Through the ui:options one.

I'm in favor of using the latter, and deprecating support for the former. Thoughts?

enhancement

Most helpful comment

I thought about it and I think I have a clean solution here with backwards compatibility.

So there are three ways to set options now, depending on which option you want to set:

  • directly with ui:{optionName} (already used in production, quite good syntax)
  • with ui:options object (not yet used in production, but better syntax?)
  • and with ui:widget.options object (already used in production, but awkward syntax).

To align them all, a getUiOptions(uiSchema) function could be added in utils.js, which should be called in this way from each field for all options:

// in a Field
const {uiSchema} = this.props;
const {widget, barable, fooable} = {
  widget: defaultWidget,
  barable: true,
  fooable: true,
  ...getUiOptions(uiSchema)
};

So the options syntax is abstracted away from the fields into one util function. This makes the field files cleaner and more consequent immediately.

In getUiOptions(), all 3 syntaxes could be accepted for every option, and for one or two syntaxes, a deprecation warning could be logged:

// in utils
export function getUiOptions(uiSchema) {
  // get all passed options from ui:widget, ui:options, and ui:<optionName>
  return Object.keys(uiSchema).filter(key => /^ui:/.test(key)).reduce((options, key) => {
    const value = uiSchema[key];

    if (key === "ui:widget" && typeof value === "object") {
      // potential DEPRECATED warning can be called here
      options["widget"] = value.component;
      if (value.options) {
        setOptions(options, value.options);
      }
    } else if (key === "ui:options" && typeof value === "object") {
      // potential DEPRECATED warning can be called here
      setOptions(options, value.options);
    } else {
      // potential DEPRECATED warning can be called here
      options[key.substring(3)] = value;
    }

    return options;
  }, {});

  function setOptions(dest, src) {
    Object.keys(src).forEach(option => {
      dest[option] = src[option];
    });
  }
}

I would like to know what you think about this. I will PR if you think that this is the right approach.

All 11 comments

Agreed, I have a few "wi-elds" that work with objects, arrays, base types. I like the ui:options structure a bit more myself. Can't really say why, in retrospect they both work pretty well.

I'm also in favor of the ui:options format. Few reasons to add:

  • It's easier to carry around if you have nested fields/widgets (I have deeply nested fields/widgets).
  • It's easier to define in PropTypes and therefore simplifies error detecting.
  • It makes the namespace more easy to handle with.

Sorry for this dumb question, but I only see ui:options mentioned under Array items ordering.

Is exposing ui:options from the uiSchema to custom widgets a new feature in an upcoming release?

If yes, is the plan then to expose the uiSchema options to the custom widgets (through a new prop?) like this;

const uiSchema = {
    "ui:options": {
        "someOtherModel": someOtherModel,
        "onUploadDone": Actions.addAsset,
    },

    images: {
        items: {
            guid: {
                "ui:widget": {
                    "component": "customUploadWidget",
                }
            }
        }
    },

    videos: {
        items: {
            guid: {
                "ui:widget": {
                    "component": "customUploadWidget",
                }
            }
        }
    }
}

Instead of;

const uiSchema = {  
    images: {
        items: {
            guid: {
                "ui:widget": {
                    options: {
                        "someOtherModel": someOtherModel,
                        "onUploadDone": Actions.addAsset,
                    },
                    "component": "customUploadWidget",
                }
            }
        }
    },

    videos: {
        items: {
            guid: {
                "ui:widget": {
                    options: {
                        "someOtherModel": someOtherModel,
                        "onUploadDone": Actions.addAsset,
                    },
                    "component": "customUploadWidget",
                }
            }
        }
    }
}

@matiaslarsson It would look like this:

const uiSchema = {
    images: {
        items: {
            guid: {
                "ui:widget": "customUploadWidget",
                "ui:options: {
                        "someOtherModel": someOtherModel,
                        "onUploadDone": Actions.addAsset,
                 }
            }
        }
    },

    videos: {
        items: {
            guid: 
                "ui:widget": "customUploadWidget",
                "ui:options: {
                        "someOtherModel": someOtherModel,
                        "onUploadDone": Actions.addAsset,
                 }
        }
    }
}

@olzraiti Got it, thanks!

So is this api design change to ui:options instead of ui:widget { options final? I would like to help making 0.41 final so I will look into this then. How about backwards compatibility?

I thought about it and I think I have a clean solution here with backwards compatibility.

So there are three ways to set options now, depending on which option you want to set:

  • directly with ui:{optionName} (already used in production, quite good syntax)
  • with ui:options object (not yet used in production, but better syntax?)
  • and with ui:widget.options object (already used in production, but awkward syntax).

To align them all, a getUiOptions(uiSchema) function could be added in utils.js, which should be called in this way from each field for all options:

// in a Field
const {uiSchema} = this.props;
const {widget, barable, fooable} = {
  widget: defaultWidget,
  barable: true,
  fooable: true,
  ...getUiOptions(uiSchema)
};

So the options syntax is abstracted away from the fields into one util function. This makes the field files cleaner and more consequent immediately.

In getUiOptions(), all 3 syntaxes could be accepted for every option, and for one or two syntaxes, a deprecation warning could be logged:

// in utils
export function getUiOptions(uiSchema) {
  // get all passed options from ui:widget, ui:options, and ui:<optionName>
  return Object.keys(uiSchema).filter(key => /^ui:/.test(key)).reduce((options, key) => {
    const value = uiSchema[key];

    if (key === "ui:widget" && typeof value === "object") {
      // potential DEPRECATED warning can be called here
      options["widget"] = value.component;
      if (value.options) {
        setOptions(options, value.options);
      }
    } else if (key === "ui:options" && typeof value === "object") {
      // potential DEPRECATED warning can be called here
      setOptions(options, value.options);
    } else {
      // potential DEPRECATED warning can be called here
      options[key.substring(3)] = value;
    }

    return options;
  }, {});

  function setOptions(dest, src) {
    Object.keys(src).forEach(option => {
      dest[option] = src[option];
    });
  }
}

I would like to know what you think about this. I will PR if you think that this is the right approach.

@maartenth This indeed sounds like a very good plan, and I think that's the best compromise we could possibly come with in the shortest term :)

We'll probably have to officially deprecate the two other approaches and only document the ui:options one, and when tagging 1.0 we'll remove support for the old ways. Would that work for you folks? If so, I'll be happy to review a PR!

I will try to put together a PR in the next days. It is not trivial, I already saw this approach has some overlap with other util functions like getAlternativeWidget() and I need to understand what's going on in every field and widget with all the passing around of options.

First observation: there appears to be something wrong with current implementation of merging defaultProps.options of custom widgets with custom options from uiSchema, according to a test case I added in my branch. Custom options provided for one instance of a custom widget are applied to all instances of that custom widget, both when that widget is referenced as string and when it is referenced as function.

Two of the four tests fail; which tests fail, depends on the order of the fields...

Although my pull request #378 still has a test error, could someone already give some feedback on my changes? Especially for my question about the failing test. :) Thank you!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

n1k0 picture n1k0  路  3Comments

anttivikman picture anttivikman  路  3Comments

mammad2c picture mammad2c  路  3Comments

ClockerZadq picture ClockerZadq  路  3Comments

FBurner picture FBurner  路  3Comments