Skip to content
Open
28 changes: 27 additions & 1 deletion packages/form-core/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,20 @@ export function getBy(obj: unknown, path: string | (string | number)[]): any {
}, obj)
}

/**
* Check if an object is a File.
* @private
*/
export function isFile(obj: any) {
return (
obj &&
typeof obj === 'object' &&
'name' in obj &&
'size' in obj &&
'type' in obj
)
}

/**
* Set a value on an object using a path, including dot notation.
* @private
Expand All @@ -52,7 +66,10 @@ export function setBy(obj: any, _path: any, updater: Updater<any>) {

function doSet(parent?: any): any {
if (!path.length) {
return functionalUpdate(updater, parent)
return functionalUpdate(
isFile(updater) ? { file: updater, uuid: uuid() } : updater,
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't type safe. This wrongly asserts File field values as { file: File | (oldFile: File) => File, uuid: string } instead of

  1. actually running the updater
  2. Preserving the expected field value's structure

parent,
)
}

const key = path.shift()
Expand Down Expand Up @@ -422,6 +439,15 @@ export const isGlobalFormValidationError = (
}

export function evaluate<T>(objA: T, objB: T) {
if (objA instanceof File && objB instanceof File) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the previous comment wasn't addressed, despite the request for another review. This will throw ReferenceErrors in environments where File is not present (such as React Native).

return (
objA.name === objB.name &&
objA.size === objB.size &&
objA.type === objB.type &&
objA.lastModified === objB.lastModified
)
}

if (Object.is(objA, objB)) {
return true
}
Expand Down
33 changes: 32 additions & 1 deletion packages/form-core/tests/FormApi.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, expect, it, vi } from 'vitest'
import { z } from 'zod'
import { FieldApi, FormApi, formEventClient } from '../src/index'
import { FieldApi, FormApi, isFile } from '../src/index'
import { sleep } from './utils'
import type { AnyFieldApi, AnyFormApi } from '../src/index'

Expand Down Expand Up @@ -4096,6 +4096,37 @@ it('should generate a formId if not provided', () => {
expect(form.formId.length).toBeGreaterThan(1)
})

it('should set a file value with uuid when setting a field value with File', () => {
const form = new FormApi({
defaultValues: {
avatar: undefined,
} as { avatar: unknown },
})

form.mount()

const firstFile = new File(['first'], 'first.png', { type: 'image/png' })
form.setFieldValue('avatar', firstFile)

const firstValue = form.state.values.avatar as { file: File; uuid: string }
Copy link
Contributor

Choose a reason for hiding this comment

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

These type assertions are a huge problem. It cannot stay.

Luckily, as long as evaluate properly handles File types, this whole uuid trickery won't be needed. Just make sure to add unit tests for a File comparison with evaluate. I believe the existing tests can be found in utils.spec.ts.


expect(firstValue).toBeDefined()
expect(isFile(firstValue.file)).toBe(true)
expect(firstValue.file.name).toBe('first.png')
expect(typeof firstValue.uuid).toBe('string')
expect(firstValue.uuid.length).toBeGreaterThan(0)

const secondFile = new File(['second'], 'second.png', { type: 'image/png' })
form.setFieldValue('avatar', secondFile)

const secondValue = form.state.values.avatar as { file: File; uuid: string }

expect(secondValue.file.name).toBe('second.png')
expect(typeof secondValue.uuid).toBe('string')
expect(secondValue.uuid.length).toBeGreaterThan(0)
expect(secondValue.uuid).not.toBe(firstValue.uuid)
})

describe('form api event client', () => {
it('should have debug disabled', () => {
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
Expand Down
Loading