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

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

Merged
merged 1 commit into from
Sep 6, 2016

Conversation

philjolly
Copy link
Contributor

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. angular/angular.js#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 angular/angular.js#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 #706

angular. does not (and will not angular/angular.js#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 formly-js#706
@codecov-io
Copy link

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
Copy link
Member

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
Copy link

LGTM 👍

@kwypchlo
Copy link
Member

kwypchlo commented Sep 6, 2016

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

@kentcdodds
Copy link
Member

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

@kentcdodds kentcdodds merged commit fc45fb3 into formly-js:master Sep 6, 2016
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