formly-js / angular-formly

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

fix(models): update field model reference for fields with nested string models #653

Closed kwypchlo closed 8 years ago

kwypchlo commented 8 years ago

What

String field models were resolved just once on init and when the outer model ($scope.model) is totally overridden, the field's model still references the old model.

How

I have added a shallow reference watcher to every field that has string model. It checks if the reference changed and updates field.model accordingly. I had to add 5th parameter to evalCloseToFormlyExpression because I didn't want to override the field object on each watcher. This might seem a bit dirty :+1: Test case for the fix added.

Checklist:

codecov-io commented 8 years ago

Current coverage is 96.04%

Merging #653 into master will increase coverage by +0.01% as of 0280668

@@            master    #653   diff @@
======================================
  Files           16      16       
  Stmts         1160    1163     +3
  Branches         0       0       
  Methods          0       0       
======================================
+ Hit           1114    1117     +3
  Partial          0       0       
  Missed          46      46       

Review entire Coverage Diff as of 0280668


Uncovered Suggestions

  1. +0.34% 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.

kentcdodds commented 8 years ago

LGTM, could I have someone else review and merge this? Ping @formly-js/angular-formly-collaborators

BarryThePenguin commented 8 years ago

I guess the only performance hit we're going to see is for people using nested objects. Looks fine, LGTM