-
-
Notifications
You must be signed in to change notification settings - Fork 576
fix(form-core): π File input field - Picking a different file does not trigger rerender (deep equality issue with File/Blob objects) #1939
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
base: main
Are you sure you want to change the base?
Changes from all commits
f461a64
52899a3
e52c1a6
9145eca
45c3e2a
df74f02
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
|
@@ -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, | ||
| parent, | ||
| ) | ||
| } | ||
|
|
||
| const key = path.shift() | ||
|
|
@@ -422,6 +439,15 @@ export const isGlobalFormValidationError = ( | |
| } | ||
|
|
||
| export function evaluate<T>(objA: T, objB: T) { | ||
| if (objA instanceof File && objB instanceof File) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| return ( | ||
| objA.name === objB.name && | ||
| objA.size === objB.size && | ||
| objA.type === objB.type && | ||
| objA.lastModified === objB.lastModified | ||
| ) | ||
| } | ||
|
|
||
| if (Object.is(objA, objB)) { | ||
| return true | ||
| } | ||
|
|
||
| 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' | ||
|
|
||
|
|
@@ -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 } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
|
||
| 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(() => {}) | ||
|
|
||
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 isn't type safe. This wrongly asserts File field values as
{ file: File | (oldFile: File) => File, uuid: string }instead of