FaridSafi / react-native-gifted-form

📝 « One React-Native form component to rule them all »
MIT License
1.44k stars 214 forks source link

Fix check for undefined validator. #103

Closed mzehngut closed 7 years ago

cooperka commented 7 years ago

Hi @mzehngut, this code has been around a long time without causing issues that I know of, can you please explain why this change is needed and/or link to documentation? I agree it's weird to check for 'undefined' as a string, but it might be intentional.

mzehngut commented 7 years ago

@cooperka I want to change the validator for a form field after the field is rendered, so I initially want the field created with an undefined validator, such as:

    this.state = {
        validator: {
            title: ‘My Title’,
            validate: [{
            }]
        }
    };

Because this.state.validator.validate[0].validator is undefined, the original code in GiftedFormManager.js tries to do the validation and hits the warning at L44:

console.warn('GiftedForm Error: Validator is not correct for: '+k);

I can work around this by changing my code to this:

    this.state = {
        validator: {
            title: ‘My Title’,
            validate: [{
                validator: 'undefined'
            }]
        }
    };

So, the code as it exists in GiftedFormManager.js is not a showstopper for me, and I am fine with you not accepting this PR, but I wanted to point it out as a likely bug. An alternative change that maintains compatibility with the existing code would be to change L9 to:

if (typeof validate[i].validator === 'undefined' || validate[i].validator === 'undefined') { continue; }

but that is probably overkill.

cooperka commented 7 years ago

Looking at the code further down (e.g. here), it looks like validator can be set to a string in several other cases as well. I'd prefer to keep the check for 'undefined', but how about:

if (!validate[i].validator || validate[i].validator === 'undefined')
mzehngut commented 7 years ago

@cooperka That works fine for me and supports both cases, having validator === ’undefined’ or validator being undefined. I updated this PR with your suggested code change. Thanks.

cooperka commented 7 years ago

Thank you @mzehngut!