getValidationMessages bug (dynamic scope)

[idolize]

Since 5.x migrated from using the ES5 mixin approach (where class methods were auto-bound by React) to using the ES6 class approach, there is no longer any autobinding of the this variable.

As a result, methods like getValidationMessages which attempt to read this.state are not guaranteed to be reading the state of the Validation wrapper component (because this is no longer bound), and can return an undefined result. At this point the factory just turns it into an empty Array, which leads to very hard to track down errors.

[jurassix]

This may be an issue of poor documentation?

In 5.x we no longer reach into your component and pull out this.state. Instead we enforce the getValidationData api which returns data in any form the validator.

If you data is stored in state and your using ES6 classes you need to bind the this context manually in the constructor.

pulled from docs and updated a bit

class Example extends Component {

  constructor(props) {
    super(props);
    this.validatorTypes = {
      username: Joi.string().alphanum().min(3).max(30).required().label('Username'),
      password: Joi.string().regex(/[a-zA-Z0-9]{3,30}/).label('Password')
    };
    this.getValidatorData = this.getValidatorData.bind(this);
    this.renderHelpText = this.renderHelpText.bind(this);
    this.getClasses = this.getClasses.bind(this);
    this.onSubmit = this.onSubmit.bind(this);
    this.state = {};
  }

  getValidatorData() {
    return this.state;
  }
...

Can you provide an example of how you are approaching validation?

[idolize]

Sure, I get that.

But what I'm saying is that you pass the getValidationMessages method down as a prop, but when the user calls getValidationMessages it is not guaranteed that the caller will have the same binding for this, so this line here may not return what you expect it to return (it may return undefined because the caller may have no state.errors)

[jurassix]

I have explicitly bound the function in the Validation constructor here

I'll need to put some tests together to see this break down.

[jurassix]

If you can put together a simple example that exploits this issue - the same way you ran into it - it will help. In the meantime I'll experiment with different scenarios and see under what conditions the original binding is being bound to a new context.

[idolize]

Ah, thanks! I think I may have been seeing an unrelated issue and attributing it to this. My fault! I'll close the issue unless you can reproduce, because (after digging a little further) it seems to be working for me.

[jurassix]

Great glad it was a false alarm! If there is anything we can do to increase errors or debugging of wrapped components please open more issues or PR's.