formly-js / angular-formly

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

feat(formly-field): Adds parser for keys that contain arrays #709

Closed philjolly closed 7 years ago

philjolly commented 7 years ago

What

This adds a deep assign function to formly-field that allows for keys that contain array indexes to properly create the array if it does not exist. It also adds a flag to formlyConfig.extras to enable this parser lowering the impact on existing installations.

Why

angular.$parse (used in the setter) does not differentiate between object and array when setting values. In the case where a key points to an array that has not already been set in the model it will create an object with a key of the index. This issue has been discussed many times in the angular project with the decision each time that they will not fix it. i.e. https://github.com/angular/angular.js/pull/9850

How

In formly-field the function deepAssign has been added which recursively follows the path provided in the key establishing the proper object/array nodes if they do not exist, there is also a check function added to identify keys that contain array indexes. In formlyConfig.extras a flag called parseKeyArrays (defaulted to false) controls whether or not to utilize the deepAssign function in the setter so that existing implementations will not be affected.

For issue #706

Checklist:

angular.$parse does not (and will not https://github.com/angular/angular.js/pull/9850) properly handle arrays in keys unless they have already been created in the model. This fix/feature adds a separate parser for these circumstances and a formlyConfig.extras flag to control its use. The flag is in place to minimize impact on current functionality of the setter.

This is in support of https://github.com/formly-js/angular-formly/issues/706

codecov-io commented 7 years ago

Current coverage is 95.93% (diff: 100%)

Merging #709 into master will increase coverage by 0.04%

@@             master       #709   diff @@
==========================================
  Files            17         17          
  Lines          1169       1181    +12   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1121       1133    +12   
  Misses           48         48          
  Partials          0          0          

Sunburst

Powered by Codecov. Last update 2073a91...7f8601c

kentcdodds commented 7 years ago

LGTM! I'll let someone from @formly-js/angular-formly-collaborators-read give the thumbs up and someone from @formly-js/angular-formly-collaborators merge the PR.

gillchristian commented 7 years ago

LGTM :+1:

kwypchlo commented 7 years ago

Looks good. I would even suggest to include this as default behavior.

kentcdodds commented 7 years ago

Agreed. We could make that a breaking change in a future release.