formly-js / angular-formly

JavaScript powered forms for AngularJS
http://docs.angular-formly.com
MIT License
2.23k stars 405 forks source link

fix(model): keep track of correct model reference #657

Closed kwypchlo closed 8 years ago

kwypchlo commented 8 years ago

What

Fix for issue #656

Why

I did not take into account such basic scenario - no test case was failing and it all seemed to work on my app

How

field object does not contain model property that is the model itself for the basic field usage where the model is $scope.model. I assumed we could then assign this $scope.model as a property to field object and operate only on this property. This would make it consistent with the scenarios when field.model was passed as a string and resolved to an object (with a watcher) or assigned as an custom object. This was a mistake because when scope.model reference changed, field.model referenced old model object. To make it work I would have to put a watcher on scope.model and update field.model accordingly but this would add a lot of unnecessary watchers so I've reverted to the original solution where we use construction field.model || scope.model which works fine now.

One test case added, one test case fixed.

For issue #656

Checklist:

closes #656

codecov-io commented 8 years ago

Current coverage is 96.03%

Merging #657 into master will decrease coverage by -0.01% as of 84d5d97

@@            master    #657   diff @@
======================================
  Files           16      16       
  Stmts         1163    1161     -2
  Branches         0       0       
  Methods          0       0       
======================================
- Hit           1117    1115     -2
  Partial          0       0       
  Missed          46      46       

Review entire Coverage Diff as of 84d5d97


Uncovered Suggestions

  1. +0.35% via ...alidationMessages.js#25...28
  2. +0.26% via src/test.utils.js#38...40
  3. +0.26% via ...s/formlyUsability.js#18...20
  4. See 7 more...

Powered by Codecov. Updated on successful CI builds.

BarryThePenguin commented 8 years ago

Ok, looks good