alibaba / formily

📱🚀 🧩 Cross Device & High Performance Normal Form/Dynamic(JSON Schema) Form/Form Builder -- Support React/React Native/Vue 2/Vue 3
https://formilyjs.org/
MIT License
11.36k stars 1.48k forks source link

[Bug Report] Incorrect mounted Property State in Array Items After Removal #3941

Closed ccpu closed 1 year ago

ccpu commented 1 year ago

Reproduction link

Edit on CodeSandbox

Steps to reproduce

  1. Remove an item from the array.
  2. Observe the mounted property of the remaining array items.

array-problem

What is expected?

After removing an item from the array, the mounted property of the remaining items should remain true.

What is actually happening?

After removing an item from the array, the mounted property of the remaining items incorrectly becomes false.

Package

@formily/core@2.2.29


This issue impacts the accurate tracking of the mounted state of array items, potentially leading to incorrect behaviours or rendering inconsistencies in complex forms.

charlzyx commented 1 year ago

补充一个 jest 测试用例, 很奇怪, 跟 demo 表现并不一致

破案了, 在上述demo中, line: 44-48

  {props.value?.map((item, index) => {
    return (
      <div key={index} style={{ marginBottom: 10, display: "flex" }}>
        <RecursionField name={index} schema={schema.items} />
        <button onClick={() => field.remove(index)}>Delete</button>
      </div>
    );
  })}

这里的数组 key 给了个 index, 事件经过如下

  1. 删除第二条数据 remove(1), 经过下面一系列方法调用, 最终触发了 field.dispose 方法调用链: (packages/core) ArrayField.ts#remove -> internals.ts#spliceArrayState -> internals.ts#patchFieldStates -> internals.ts#destroy -> field?.dispose(); 这几个响应式数据模型操作没有任何问题
  2. 在 react 环境下, mouted 状态是 packages/react/hooks/useAttach.ts -> onMount/onUnmount 来修改的, 而 index 作为列表项的 key 这个 经典问题, 又在这里得到了验证: 第二条数据删除触发了 unMount, 但第三条来补位并没有触发 onMount 这里就不展开了(因为我也不太明白)
  3. 在 jest 环境中, mount 则是通过 内部的 packages/core/src/__test__/shared.ts 简单的手动执行, 所以在 jest 环境中没有复现
    export const attach = <T extends { onMount: () => void }>(target: T): T => {
    target.onMount()
    return target
    }

    所以, demo 中的修复方案就是, 给列表项一个稳定的key


  {props.value?.map((item, index) => {
    return (
-      <div key={index} style={{ marginBottom: 10, display: "flex" }}>
+      <div key={item.key} style={{ marginBottom: 10, display: "flex" }}>
        <RecursionField name={index} schema={schema.items} />
        <button onClick={() => field.remove(index)}>Delete</button>
      </div>
    );
  })}
//...
  field.setValue([
-    { aa: { input: "1" } },
-    { aa: { input: "2" } },
-    { aa: { input: "3" } }
+    { aa: { input: "1" }, key: 1 },
+    { aa: { input: "2" }, key: 2 },
+    { aa: { input: "3" }, key: 3 }
  ]);

https://github.com/alibaba/formily/assets/18055018/c0ab25a4-7cb2-4c1d-84a9-d10226c8a0c7

以下是最初的测试代码和输出结果

3941.spec.ts ```ts import { createForm, isArrayField, isDataField, onFieldInputValueChange, } from '../' import { attach } from './shared' test('test 3941', () => { let record = '' const form = attach( createForm({ effects: () => { onFieldInputValueChange('*', (field) => { if (isDataField(field) && !isArrayField(field)) { record = [field.path, 'mounted', field.mounted.toString()].join(' ') } }) }, }) ) const array = attach( form.createArrayField({ name: 'array', }) ) const ObjectField1 = attach( form.createObjectField({ name: 'aa', basePath: 'array.0', }) ) const Field1 = attach( form.createField({ name: 'input', basePath: 'array.0.aa', }) ) const ObjectField2 = attach( form.createObjectField({ name: 'aa', basePath: 'array.1', }) ) const Field2 = attach( form.createField({ name: 'input', basePath: 'array.1.aa', }) ) const ObjectField3 = attach( form.createObjectField({ name: 'aa', basePath: 'array.2', }) ) const Field3 = attach( form.createField({ name: 'input', basePath: 'array.2.aa', }) ) array.setValue([ { aa: { input: '1' } }, { aa: { input: '2' } }, { aa: { input: '3' } }, ]) console.log('before1:', Field1.address.entire, Field1.value) console.log('before2:', Field2.address.entire, Field2.value) console.log('before3:', Field3.address.entire, Field3.value) array.remove(1) console.log('after1:', Field1.address.entire, Field1.value) console.log('after2:', Field2.address.entire, Field2.value) console.log('after3:', Field3.address.entire, Field3.value) expect(Field2.value).toBe('3') expect(Field2.mounted).toBe(true) expect(ObjectField2.mounted).toBe(true) Field2.onInput('22') expect(record).toBe('array.1.aa.input mounted true') expect(Field2.value).toBe('22') }) ```
输出 ```log PASS packages/core/src/__tests__/3941.spec.ts ✓ test 3941 (24 ms) console.log before1: array.0.aa.input 1 at Object. (packages/core/src/__tests__/3941.spec.ts:71:11) console.log before2: array.1.aa.input 2 at Object. (packages/core/src/__tests__/3941.spec.ts:72:11) console.log before3: array.2.aa.input 3 at Object. (packages/core/src/__tests__/3941.spec.ts:73:11) console.log after1: array.0.aa.input 1 at Object. (packages/core/src/__tests__/3941.spec.ts:77:11) console.log after2: array.1.aa.input 3 at Object. (packages/core/src/__tests__/3941.spec.ts:78:11) console.log after3: array.1.aa.input 3 at Object. (packages/core/src/__tests__/3941.spec.ts:79:11) --------------------|---------|----------|---------|---------|-------------------------------------------------------------------------------------------------------------- File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s --------------------|---------|----------|---------|---------|-------------------------------------------------------------------------------------------------------------- All files | 56.5 | 27.58 | 41.81 | 57.52 | src | 100 | 100 | 100 | 100 | index.ts | 100 | 100 | 100 | 100 | types.ts | 100 | 100 | 100 | 100 | src/effects | 65.97 | 10 | 22.72 | 68.81 | index.ts | 100 | 100 | 100 | 100 | onFieldEffects.ts | 59.64 | 11.11 | 28.57 | 62.96 | 98-104,112-115,134-152 onFormEffects.ts | 73.68 | 0 | 12.5 | 75.67 | 10-12,73-80 src/models | 57.42 | 30.33 | 41.35 | 59.08 | ArrayField.ts | 39.02 | 30.76 | 33.33 | 42.85 | 34,44-49,54-62,67-79,96-99,104-113,118-126,131-132,136-137 BaseField.ts | 53.67 | 33.72 | 40.38 | 53.04 | ...4,210-213,219-222,235,239,243,247,254-259,266-268,276-281,288-290,295,299,315-317,321,340,344,348-350,356 Field.ts | 67.71 | 25.97 | 45.61 | 72.41 | ...9,323,342-345,362-378,386,394,398,402,406,414,421-422,442,446,450,454,468,489-494,498-502,506,510,514,518 Form.ts | 59.56 | 28.91 | 36.7 | 62.3 | ...9,447,451,455,459,463,477,481-483,492-494,503-505,522-533,545,549,563,568-575,592,596,600-601,606,612,616 Graph.ts | 31.42 | 0 | 28.57 | 29.41 | 23-28,32-55 Heart.ts | 70.83 | 42.1 | 69.23 | 69.04 | 22-28,36,49,58-59,70-72 LifeCycle.ts | 77.77 | 50 | 83.33 | 73.91 | 20,27-31 ObjectField.ts | 71.42 | 50 | 55.55 | 69.23 | 29,38-40,44-46,50 Query.ts | 55.35 | 33.33 | 29.41 | 56.86 | 11-14,23,25,44-49,68-69,76-77,85-88,95-97,102,106,110 VoidField.ts | 22.91 | 0 | 20 | 22.22 | 24-43,47-63,67-68,111-117 index.ts | 100 | 100 | 100 | 100 | src/shared | 51.51 | 25.61 | 44.78 | 50.59 | checkers.ts | 64.4 | 4.41 | 40 | 60.52 | 28,44-45,49-50,54-55,59-60,68,74-75,79-80,84 constants.ts | 100 | 100 | 100 | 100 | effective.ts | 87.8 | 60 | 87.5 | 86.48 | 23,38-49 externals.ts | 100 | 100 | 19.23 | 100 | internals.ts | 45.32 | 28.57 | 48.24 | 43.92 | ...0-742,756,767-788,797-810,816-880,886-908,918-930,944,951-969,978-1001,1048,1055-1060,1087-1092,1098-1103 --------------------|---------|----------|---------|---------|-------------------------------------------------------------------------------------------------------------- Test Suites: 1 passed, 1 total Tests: 1 passed, 1 total Snapshots: 0 total Time: 2.533 s, estimated 3 s Ran all test suites matching /packages\/core\/src\/__tests__\/3941.spec.ts/i. ```

综上, 这应该是使用的问题, 而不是 formily 的bug

另一个思路是, 因为这个attach也好, useAttach 也好, 只影响的 mounted/unmounted 这两个字段, 因此

packages/core/src/shared/internals.ts#patchFieldStates 方法中的 update 分支语句中, 强行修正 traget.mounted = true;traget.unmounted = false; (可以根据是否之前位置是否删除再加一些优化判断)

但这只是 react 的会遇到的问题, 不是core的问题, 我个人是不赞同进行这样的修复, 而是尊重 react 的约定, 使用稳定的 key