Skip to content
This repository was archived by the owner on Apr 30, 2018. It is now read-only.

Conversation

@Rybbow
Copy link
Contributor

@Rybbow Rybbow commented Jan 17, 2017

What

In FormlyFormController in function validateFormControl variable validate was invoked with "undefined" this if the function was called with a promise.

Why

It's a bug.

How

Used a closure and apply() to ensure validate() will be called with formController as this.

For issue #523

Checklist:

Fixes validate being called with undefined this variable scope.

Closes #523

…promise change

Fixes validate being called with undefined this variable scope.

Closes formly-js#523
@GitCop
Copy link

GitCop commented Jan 17, 2017

There were the following issues with your Pull Request

  • Commit: ed1fcfc
  • Your subject line is longer than 72 characters

Guidelines are available at https://github.com/formly-js/angular-formly/blob/master/CONTRIBUTING.md


This message was auto-generated by https://gitcop.com

@codecov-io
Copy link

Current coverage is 95.93% (diff: 100%)

Merging #728 into master will not change coverage

@@             master       #728   diff @@
==========================================
  Files            17         17          
  Lines          1181       1181          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           1133       1133          
  Misses           48         48          
  Partials          0          0          

Powered by Codecov. Last update fc45fb3...ed1fcfc

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine. Thank you.

@kentcdodds
Copy link
Member

Could someone from @angular-formly-collaborators review and merge this?

@kentcdodds
Copy link
Member

Sorry, bad @ mention. Someone from @formly-js/angular-formly-collaborators please review and merge this. Thanks.

promise.then(validate)
promise.then(() => validate.apply(formControl))
} else {
validate()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about this line?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that the problem is that the validate function isn't being called with the right this context. Honestly, I'd prefer the use of bind personally, but I don't care either way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. LGTM

@kentcdodds kentcdodds merged commit 42b47c6 into formly-js:master Jan 20, 2017
aluanhaddad added a commit to aluanhaddad/angular-formly that referenced this pull request Jul 23, 2017
…ange (formly-js#728) (#1)

Fixes validate being called with undefined this variable scope.

Closes formly-js#523
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants