Skip to content
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

fix(form): 修复 formitem 的值如果是对象会自动重置为空对象的问题 #2952

Merged
merged 6 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
263 changes: 263 additions & 0 deletions src/packages/form/__tests__/merge.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,263 @@
import { merge, clone, recursive, isPlainObject } from '@/utils/merge'

describe('merge', () => {
it('merges two objects', () => {
expect(merge({ a: 1 }, { b: 2 })).toStrictEqual({ a: 1, b: 2 })
})

it('merges nested levels', () => {
expect(merge({ a: 1 }, { b: { c: { d: 2 } } })).toStrictEqual({
a: 1,
b: { c: { d: 2 } },
})
})
it('clones the target', () => {
let input = {
a: 1,
b: {
c: {
d: 2,
e: ['x', 'y', { z: { w: ['k'] } }],
},
},
f: null,
g: undefined,
h: true,
}

const original = {
a: 1,
b: {
c: {
d: 2,
e: ['x', 'y', { z: { w: ['k'] } }],
},
},
f: null,
g: undefined,
h: true,
}

let output = merge(true, input)

input.b.c.d++
;(input.b.c.e[2] as any).z.w = null
;(input as any).h = null

expect(output).toStrictEqual(original)

input = original

output = merge(true, input, { a: 2 })

expect(output.a).toBe(2)
expect(input.a).toBe(1)
})

it('ignores the sources', () => {
const values = createNonPlainObjects()
const $merge = vi.fn().mockImplementation(merge)

for (const value of values) expect($merge(value)).toStrictEqual({})

expect(values.length).toBeGreaterThan(0)
expect($merge).toBeCalledTimes(values.length)
expect(
merge(...values, [0, 1, 2], ...values, { a: 1 }, ...values, {
b: 2,
})
).toStrictEqual({ a: 1, b: 2 })
})

it('does not merge non plain objects', () => {
const values = createNonPlainObjects()
expect(values.length).toBeGreaterThan(0)
const input: any = {}

for (const [index, value] of Object.entries(values)) {
input[`value${index}`] = value
}

const output = merge({}, input)

for (const [index] of Object.entries(values)) {
const key = `value${index}`
const inputValue = input[key]
const outputValue = output[key]

// eslint-disable-next-line no-restricted-globals
if (typeof outputValue === 'number' && isNaN(outputValue)) {
// eslint-disable-next-line no-restricted-globals
expect(isNaN(inputValue), key).toBeTruthy()
Comment on lines +89 to +91
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

使用 Number.isNaN 替换不安全的全局 isNaN

为了避免类型强制转换带来的潜在问题,建议使用 Number.isNaN

-      if (typeof outputValue === 'number' && isNaN(outputValue)) {
+      if (typeof outputValue === 'number' && Number.isNaN(outputValue)) {
-        expect(isNaN(inputValue), key).toBeTruthy()
+        expect(Number.isNaN(inputValue), key).toBeTruthy()

// 同样在其他位置也需要修改:
-      if (typeof cloned === 'number' && isNaN(cloned)) {
+      if (typeof cloned === 'number' && Number.isNaN(cloned)) {
-        expect(isNaN(value)).toBeTruthy()
+        expect(Number.isNaN(value)).toBeTruthy()

Also applies to: 132-134

🧰 Tools
🪛 Biome (1.9.4)

[error] 89-89: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

(lint/suspicious/noGlobalIsNan)


[error] 91-91: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

(lint/suspicious/noGlobalIsNan)

} else {
expect(inputValue === outputValue, key).toBeTruthy()
}
}
})

it('is safe', () => {
expect(
merge({}, JSON.parse('{"__proto__": {"evil": true}}'))
).toStrictEqual({})
expect(({} as any).evil).toBeUndefined()
})
})

describe('clone', () => {
it('clones the input', () => {
const object1 = { a: 1, b: { c: 2 } }
const object2 = clone(object1)

expect(object1).toStrictEqual(object2)
expect(object1 === object2).toBeFalsy()
expect(object1.b === object2.b).toBeFalsy()
})

it('clones each item of the array', () => {
const object1 = [{ a: 1, b: { c: 2 } }]
const object2 = clone(object1)

expect(object1).toStrictEqual(object2)
expect(object1 === object2).toBeFalsy()
expect(object1[0] === object2[0]).toBeFalsy()
expect(object1[0].b === object2[0].b).toBeFalsy()
})

it('returns the same input', () => {
const values = createNonPlainObjects()
const $clone = vi.fn().mockImplementation(clone)
for (const value of values) {
const cloned = $clone(value)
// eslint-disable-next-line no-restricted-globals
if (typeof cloned === 'number' && isNaN(cloned)) {
// eslint-disable-next-line no-restricted-globals
expect(isNaN(value)).toBeTruthy()
} else if (Array.isArray(cloned)) {
expect(Array.isArray(value)).toBeTruthy()
} else {
expect(cloned === value).toBeTruthy()
}
}
expect(values.length).toBeGreaterThan(0)
expect($clone).toBeCalledTimes(values.length)
})
})

describe('recursive', () => {
it('merges recursively', () => {
expect(recursive({ a: { b: 1 } }, { a: { c: 1 } })).toStrictEqual({
a: { b: 1, c: 1 },
})

expect(recursive({ a: { b: 1, c: 1 } }, { a: { b: 2 } })).toStrictEqual({
a: { b: 2, c: 1 },
})

expect(
recursive({ a: { b: [1, 2, 3], c: 1 } }, { a: { b: ['a'] } })
).toStrictEqual({ a: { b: ['a'], c: 1 } })

expect(
recursive({ a: { b: { b: 2 }, c: 1 } }, { a: { b: 2 } })
).toStrictEqual({
a: { b: 2, c: 1 },
})
})

it('clones recursively', () => {
const test1 = { a: { b: 1 } }

expect(recursive(true, test1, { a: { c: 1 } })).toStrictEqual({
a: { b: 1, c: 1 },
})

expect(test1).toStrictEqual({ a: { b: 1 } })

const test2 = { a: { b: 1, c: 1 } }

expect(recursive(true, test2, { a: { b: 2 } })).toStrictEqual({
a: { b: 2, c: 1 },
})

expect(test2).toStrictEqual({ a: { b: 1, c: 1 } })

const test3 = { a: { b: [1, 2, 3], c: 1 } }

expect(recursive(true, test3, { a: { b: ['a'] } })).toStrictEqual({
a: { b: ['a'], c: 1 },
})

expect(test3).toStrictEqual({ a: { b: [1, 2, 3], c: 1 } })

const test4 = { a: { b: { b: 2 }, c: 1 } }

expect(recursive(true, test4, { a: { b: 2 } })).toStrictEqual({
a: { b: 2, c: 1 },
})

expect(test4).toStrictEqual({ a: { b: { b: 2 }, c: 1 } })
})

it('does not merge non plain objects', () => {
const object = recursive({ map: { length: 1 } }, { map: new Map() })
expect(object.map).toBeInstanceOf(Map)
})

it('is safe', () => {
const payload = '{"__proto__": {"a": true}}'
expect(recursive({}, JSON.parse(payload))).toStrictEqual({})
expect(({} as any).a).toBeUndefined()
expect(recursive({ deep: {} }, JSON.parse(payload))).toStrictEqual({
deep: {},
})
expect(({} as any).b).toBeUndefined()
})
})

describe('isPlainObject', () => {
it('returns true', () => {
expect(isPlainObject({})).toBeTruthy()
expect(isPlainObject({ v: 1 })).toBeTruthy()
expect(isPlainObject(Object.create(null))).toBeTruthy()
expect(isPlainObject({})).toBeTruthy()
})
it('returns false', () => {
const values = createNonPlainObjects()
const $isPlainObject = vi.fn().mockImplementation(isPlainObject)
for (const value of values) expect($isPlainObject(value)).toBeFalsy()
expect(values.length).toBeGreaterThan(0)
expect($isPlainObject).toBeCalledTimes(values.length)
})
})

function createNonPlainObjects(): any[] {
class SubObject extends Object {}

return [
null,
undefined,
1,
'',
'str',
[],
[1],
() => {},
function () {},
true,
false,
NaN,
Infinity,
class {},
new (class {})(),
new Map(),
new Set(),
new Date(),
[],
new Date(),
/./,
/./,
SubObject,
new SubObject(),
Symbol(''),
]
}
4 changes: 2 additions & 2 deletions src/packages/form/useform.taro.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useRef } from 'react'
import Schema from 'async-validator'
import { merge } from '@/utils/merge'
import { merge, recursive } from '@/utils/merge'
import {
Callbacks,
FormFieldEntity,
Expand Down Expand Up @@ -98,7 +98,7 @@ class FormStore {
* @param newStore { [name]: newValue }
*/
setFieldsValue = (newStore: any) => {
const nextStore = merge(this.store, newStore)
const nextStore = recursive(true, this.store, newStore)
this.updateStore(nextStore)
this.fieldEntities.forEach((entity: FormFieldEntity) => {
const { name } = entity.props
Expand Down
6 changes: 4 additions & 2 deletions src/packages/form/useform.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useRef } from 'react'
import Schema from 'async-validator'
import { merge } from '@/utils/merge'
import { merge, recursive } from '@/utils/merge'
import {
Callbacks,
FormFieldEntity,
Expand Down Expand Up @@ -98,7 +98,7 @@ class FormStore {
* @param newStore { [name]: newValue }
*/
setFieldsValue = (newStore: any) => {
const nextStore = merge(this.store, newStore)
const nextStore = recursive(true, this.store, newStore)
this.updateStore(nextStore)
this.fieldEntities.forEach((entity: FormFieldEntity) => {
const { name } = entity.props
Expand Down Expand Up @@ -202,7 +202,9 @@ class FormStore {

resetFields = () => {
this.errors.length = 0
console.log('xxx', this.initialValues)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

移除调试日志

建议移除这些控制台日志,因为它们不应该出现在生产代码中。

  resetFields = () => {
    this.errors.length = 0
-   console.log('xxx', this.initialValues)
    const nextStore = merge({}, this.initialValues)
-   console.log(nextStore)
    this.updateStore(nextStore)
    this.fieldEntities.forEach((entity: FormFieldEntity) => {

Also applies to: 207-207

const nextStore = merge({}, this.initialValues)
console.log(nextStore)
this.updateStore(nextStore)
this.fieldEntities.forEach((entity: FormFieldEntity) => {
entity.onStoreChange('reset')
Expand Down
Loading
Loading