-
-
Notifications
You must be signed in to change notification settings - Fork 280
fix: deep clone array for initialValues #377
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
MadCcc
commented
Mar 2, 2022
- Revert "revert: revert initialValues protect (revert: revert initialValues protect #376)"
- fix: deep clone array for initialValues
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/react-component/field-form/D38F1S4YPSn8dBunnv839NvPMogU |
Codecov Report
@@ Coverage Diff @@
## master #377 +/- ##
=======================================
Coverage 99.90% 99.90%
=======================================
Files 14 15 +1
Lines 1035 1051 +16
Branches 221 224 +3
=======================================
+ Hits 1034 1050 +16
Misses 1 1
Continue to review full report at Codecov.
|
| const namePath = entity.getNamePath(); | ||
|
|
||
| const defaultValue = isListField ? undefined : getValue(this.initialValues, namePath); | ||
| const defaultValue = isListField ? undefined : this.getInitialValue(namePath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里使用有深拷贝的方法获取 defaultValue,因为 this.initialValues 里存的是浅拷贝,直接浅拷贝过来会被赋值行为修改原值。
| const recursive = isObject(prevValue) && isObject(value); | ||
| newStore[key] = recursive ? internalSetValues(prevValue, value || {}) : value; | ||
|
|
||
| newStore[key] = recursive ? internalSetValues(prevValue, value || {}) : cloneDeep(value); // Clone deep for arrays |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里的深拷贝是针对 array 的。之前的做法是把 array 看作 object 处理,但是实际上处理逻辑并不一样,数组应该被直接替换,而不是比对每一个元素进行赋值
| this.initialValues = initialValues || {}; | ||
| if (init) { | ||
| this.store = setValues({}, initialValues, this.store); | ||
| this.store = setValues({}, this.store, initialValues); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
修改了一下顺序,防止 initialValues 被覆盖。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
是不是其实 this.store 是没用的?初始化的时候 this.store 应该是空的。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
理论上是的,但我不太确定。
这个方法确实是组件第一次渲染时 init 才会传 true