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 4 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
4 changes: 2 additions & 2 deletions src/packages/form/__tests__/form.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,10 @@ test('form onFinishFailed', async () => {
fireEvent.submit(form)
await waitFor(() => {
expect(handleFailed).toBeCalled()
expect(handleFailed).toBeCalledWith({ username: 'NutUI-React' }, [
expect(handleFailed).toBeCalledWith({ username: 'NutUI React Taro' }, [
oasis-cloud marked this conversation as resolved.
Show resolved Hide resolved
{
field: 'username',
fieldValue: 'NutUI-React',
fieldValue: 'NutUI React Taro',
message: 'min 50',
},
])
Expand Down
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(''),
]
}
106 changes: 83 additions & 23 deletions src/utils/merge.ts
Original file line number Diff line number Diff line change
@@ -1,29 +1,89 @@
export function merge(...objects: any[]) {
const result: any = Array.isArray(objects[0]) ? [] : {}

function mergeHelper(obj: any, path: string[] = []) {
for (const [key, value] of Object.entries(obj)) {
const newPath = [...path, key]

if (Array.isArray(value)) {
// Arrays are always overridden
result[key] = [...value]
} else if (typeof value === 'object' && value !== null) {
// Check for circular references
export default main

export function main(clone: boolean, ...items: any[]): any
export function main(...items: any[]): any
export function main(...items: any[]) {
return merge(...items)
}

Check warning on line 7 in src/utils/merge.ts

View check run for this annotation

Codecov / codecov/patch

src/utils/merge.ts#L6-L7

Added lines #L6 - L7 were not covered by tests
oasis-cloud marked this conversation as resolved.
Show resolved Hide resolved

main.clone = clone
main.isPlainObject = isPlainObject
main.recursive = recursive

export function merge(clone: boolean, ...items: any[]): any
export function merge(...items: any[]): any
export function merge(...items: any[]) {
return _merge(items[0] === true, false, items)
}

export function recursive(clone: boolean, ...items: any[]): any
export function recursive(...items: any[]): any
export function recursive(...items: any[]) {
return _merge(items[0] === true, true, items)
}

export function clone<T>(input: T): T {
if (Array.isArray(input)) {
const output = []

for (let index = 0; index < input.length; ++index)
output.push(clone(input[index]))

return output as any
}
if (isPlainObject(input)) {
const output: any = {}

// eslint-disable-next-line guard-for-in
for (const index in input) output[index] = clone((input as any)[index])

return output as any
}
return input
}

export function isPlainObject(input: unknown): input is NonNullable<any> {
if (input === null || typeof input !== 'object') return false
if (Object.getPrototypeOf(input) === null) return true
let ref = input
while (Object.getPrototypeOf(ref) !== null) ref = Object.getPrototypeOf(ref)
return Object.getPrototypeOf(input) === ref
}

function _recursiveMerge(base: any, extend: any) {
if (!isPlainObject(base) || !isPlainObject(extend)) return extend
for (const key in extend) {
if (key === '__proto__' || key === 'constructor' || key === 'prototype')
// eslint-disable-next-line no-continue
continue
base[key] =
isPlainObject(base[key]) && isPlainObject(extend[key])
? _recursiveMerge(base[key], extend[key])

Check warning on line 61 in src/utils/merge.ts

View check run for this annotation

Codecov / codecov/patch

src/utils/merge.ts#L61

Added line #L61 was not covered by tests
: extend[key]
}

return base
}

function _merge(isClone: boolean, isRecursive: boolean, items: any[]) {
let result

if (isClone || !isPlainObject((result = items.shift()))) result = {}
oasis-cloud marked this conversation as resolved.
Show resolved Hide resolved

for (let index = 0; index < items.length; ++index) {
const item = items[index]

// eslint-disable-next-line no-continue
if (!isPlainObject(item)) continue

for (const key in item) {
if (key === '__proto__' || key === 'constructor' || key === 'prototype')
// eslint-disable-next-line no-continue
if (path.some((p: any) => p === value)) continue

// Recursively merge objects
if (!result[key]) result[key] = {}
mergeHelper(value, newPath)
} else {
// Set primitive values
result[key] = value
}
continue
const value = isClone ? clone(item[key]) : item[key]
result[key] = isRecursive ? _recursiveMerge(result[key], value) : value
}
}

objects.filter((obj) => !!obj).forEach((obj) => mergeHelper(obj))

return result
}
Loading