Update BaseField.js

[rafinskipg]

Make field properties more important than schema properties

[rafinskipg]

In my case i was wanting to use conditional required value for a field. Maybe the way Custom works is the thing we need https://github.com/aldeed/meteor-simple-schema#custom

[janowsiany]

Isn't it a breaking change?

[rafinskipg]

Yea @janowsiany maybe, and maybe it is not needed xD

I was just trying to make a field required in a conditional way but @radekmie suggested a PR. Might it be enough using the custom attribute from simpleschema?
https://github.com/aldeed/meteor-simple-schema#make-a-field-conditionally-required

This should work, but it is not working.

[janowsiany]

I am WRONG in following statement
required property in uniforms is only dependent on optional: true from Simple Schema, take a look here.
end of WRONG statement
I think that this conditinal requiredness might be handled in CustomField which is IMO one of the biggest advantages of this package(makes it very flexible). Just quick scaffolding:

const Custom => ({required, ...props}, {uniforms: {model}}) => (
  <TextField required={model.otherField !== 1} {...props} />
); 
export default connectField(Custom);

This example should make field required only if value of otherField is not equal 1.
However, i might be wrong it was just my first tought that switching this order might be a breaking change, maybe some further investigation is needed.

[rafinskipg]

@janowsiany With this code i should make one custom field for each type of field i want to use with custom required... Sorry but the amount of work it makes me do... it's not acceptable. (since simpleschema has a custom attribute or i could just pass required=true to fields)

[janowsiany]

I was completely wrong, but uniforms do support even custom validations sorry for this confusion.
@rafinskipg could you please share you custom validator?

[rafinskipg]

This was working with aldeed autoforms.

custom: function() {
            var shouldBeRequired = PlaceTypeEnum.hasPlaceId(this.siblingField('type').value);

            if (shouldBeRequired) {
                // inserts
                if (!this.operator) {
                    if (!this.isSet || this.value === null || this.value === "") return "required";
                }
                // updates
                else if (this.isSet) {
                    if (this.operator === "$set" && this.value === null || this.value === "") return "required";
                    if (this.operator === "$unset") return "required";
                    if (this.operator === "$rename") return "required";
                }
            }
        }

[janowsiany]

Ok so here is a schema which works in demo app:

new SimpleSchema({
    adult: {
        type: Boolean,
        optional: true,
        custom: function() {
            if (this.field('size').value === 'm' && !this.value) {
                return 'required';
            }
        }
    },

    size: {
        type: String,
        defaultValue: 'm',
        allowedValues: ['xs', 's', 'm', 'l', 'xl']
    },
})

Am i missing something?

[radekmie]

Why did I want to see this PR? Because it's more reasonable, to make direct props more important than schema ones. I'm still thinking about consequences, but it's more like a bugfix, than a feature.

[radekmie]

To be honest, I have very mixed feelings about it... I'd love to merge it, but it is a kind of breaking change... No matter what, it's still a kind of bug fix because that's how it should behave... How about this: let's release a new version (v1.17.1 at the moment) and see, how it works.

[rafinskipg]

Probably it will be a breaking change yes. But, yeah.. makes sense. I have not tested that in our project so I don't know if anything is going to break. I suspect there will be major issues. Maybe it will affect more to people with custom fields.

[radekmie]

OK, then I'll release v1.17.1-beta.1 and ask few people to check it in their projects.

[radekmie]

@rafinskipg I had to fix it, because after your changes state took precedence over schemaProps. You can take a look at it here. It's already released as v1.17.1-beta.1.