feat(formly-form): formlyField.runExpressions resolve after fields ha…

[pseudo-su]

What

resolve promise from formlyField.runExpressions after fields have been set instead of $timeout promise

Why

$timeout promise will resolve before the async field setters have finished

How

using $q.all to resolve the promise after all the field setter promises are finished

For issue # 523

Checklist:

• [x] Follows the commit message conventions
• [x] Is rebased with master
• [x] Is only one (maybe two) commits

[GitCop]

There were the following issues with your Pull Request

• Commit: 0da9118de99164f5843000badc1efded69399d75
  • 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]

Current coverage is 95.89% (diff: 100%)

Merging #699 into master will increase coverage by 0.01%

@@             master       #699   diff @@
==========================================
  Files            17         17          
  Lines          1164       1169     +5   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1116       1121     +5   
  Misses           48         48          
  Partials          0          0          

Powered by Codecov. Last update a4c54bb...6260cca

[BarryThePenguin]

Do you know if this change solves either of the skipped tests in formly-field.test.js

[pseudo-su]

@BarryThePenguin can't find any xit or xdescribe tests in the formly-field.test.js, am I missing something?

[BarryThePenguin]

Mocha uses test.skip

[pseudo-su]

"validateOnModelChange should run field expressions when form is initialised" test is possibly relevant but the test at the moment seems to be for non-promise situations. before it gets to the relevant part in FormlyFormController.runFieldExpressionProperties the field.runExpressions is undefined.

function runFieldExpressionProperties(field, index) {
    const model = field.model || $scope.model
    const promise = field.runExpressions && field.runExpressions()
    // field.runExpressions === undefined

Do you know when exactly there will be a promise and when there won't?

I'm not actually sure about the specifics which makes it difficult to write/adapt a test case without me spending some more time on it.

[BarryThePenguin]

Hmm... ok, I just thought they would solve those skipped tests. Sounds like it's a little more involved then

[BarryThePenguin]

Looks good. My only concern is there aren't any tests relying directly on this change. Should this be something to worry about?

[pseudo-su]

The relevant code in formly-field and formly-form controller do seem to be covered by some tests at least. Ideally there would be a test for making sure that expressions complete before validation, I'll look into it.

[BarryThePenguin]

Fantastic! That test fails before this PR and passes after, looks good to me

/cc @formly-js/angular-formly-collaborators got any input?

[kentcdodds]

This is great. Thanks for working on it. Any other reviewers wanna pitch in here? Should probably get at least one other.

[kwypchlo]

LGTM

[BarryThePenguin]

Thanks again for your help @psuedoSudo

[kentcdodds]

:clap:

[pseudo-su]

Glad I could help 😃