Skip to content

fix(FileUpload): ensure native validation works with required #5358

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: v4
Choose a base branch
from
+139 −124
Open

fix(FileUpload): ensure native validation works with required #5358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1374256
fix(FileUpload): ensure native validation works with required
benjamincanac Oct 31, 2025
17005f9
test: update snapshots
benjamincanac Oct 31, 2025
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
20 changes: 11 additions & 9 deletions src/runtime/components/FileUpload.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export interface FileUploadSlots<M extends boolean = false> {

<script setup lang="ts" generic="M extends boolean = false">
import { computed, watch } from 'vue'
import { Primitive } from 'reka-ui'
import { Primitive, VisuallyHidden } from 'reka-ui'
import { createReusableTemplate } from '@vueuse/core'
import { useAppConfig, useLocale } from '#imports'
import { useFormField } from '../composables/useFormField'
Expand Down Expand Up @@ -252,15 +252,17 @@ function removeFile(index?: number) {
}

watch(modelValue, (newValue) => {
const hasModelReset = !Array.isArray(newValue) || !newValue.length
const hasModelReset = props.multiple ? !(newValue as File[])?.length : !newValue

if (hasModelReset && inputRef.value) {
inputRef.value.value = ''
if (hasModelReset && inputRef.value?.$el) {
inputRef.value.$el.value = ''
}
})

defineExpose({
inputRef,
get inputRef() {
return inputRef.value?.$el
},
Comment on lines +263 to +265
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
get inputRef() {
return inputRef.value?.$el
},
inputRef,

The exposed inputRef and dropzoneRef have inconsistent access patterns. inputRef is a getter returning a DOM element directly, while dropzoneRef remains a ref requiring .value access. This breaks backward compatibility and creates an inconsistent API.

View Details

Analysis

FileUpload component has inconsistent ref exposure patterns breaking API consistency

What fails: FileUpload.vue defineExpose exposes inputRef as getter returning DOM element directly, while dropzoneRef remains a ref, creating inconsistent access patterns that break established component API conventions.

How to reproduce:

<script setup>
const fileUpload = ref()
// These access patterns are inconsistent:
fileUpload.value.inputRef.focus()        // Works (direct DOM access)
fileUpload.value.dropzoneRef.value.focus() // Works (ref access)
</script>
<template>
  <UFileUpload ref="fileUpload" />
</template>

Expected behavior: Both refs should be exposed consistently. Example usage in InputKbdExample.vue shows expected pattern: input.value?.inputRef?.focus() which works with Input component but would break with FileUpload's inconsistent exposure.

Result: FileUpload breaks the established pattern used by Input, Textarea, and Select components which all expose refs directly via defineExpose({ inputRef }), while FileUpload uses a getter that bypasses the ref wrapper.

Copy link
Contributor

Choose a reason for hiding this comment

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

@benjamincanac This suggested change makes sense, but it would introduce a breaking change 😬 Seems minor enough to accept it tho?

Copy link
Member Author

@benjamincanac benjamincanac Nov 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

No it is incorrect, the inputRef is now bound on a Vue component (VisuallyHidden) which adds a layer over binding directly to an <input>: https://vuejs.org/guide/essentials/template-refs.html#ref-on-component. We need to return $el to access the actual input as before.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we wrap it inside a computed instead to ensure that it is reactive?

dropzoneRef
})
</script>
Expand Down Expand Up @@ -365,18 +367,18 @@ defineExpose({
<ReuseFilesTemplate v-if="position === 'outside'" />
</slot>

<input
<VisuallyHidden
:id="id"
ref="inputRef"
as="input"
type="file"
feature="fully-hidden"
:name="name"
:accept="accept"
:multiple="(multiple as boolean)"
:required="required"
:disabled="disabled"
v-bind="{ ...$attrs, ...ariaAttrs }"
class="sr-only"
tabindex="-1"
>
/>
</Primitive>
</template>
23 changes: 18 additions & 5 deletions src/runtime/composables/useFileUpload.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ref, computed, unref, onMounted, watch, reactive } from 'vue'
import type { VisuallyHidden } from 'reka-ui'
import { useFileDialog, useDropZone } from '@vueuse/core'
import type { MaybeRef } from '@vueuse/core'

Expand Down Expand Up @@ -45,12 +46,12 @@ export function useFileUpload(options: UseFileUploadOptions) {
dropzone = true,
onUpdate
} = options
const inputRef = ref<HTMLInputElement>()
const inputRef = ref<InstanceType<typeof VisuallyHidden>>()
const dropzoneRef = ref<HTMLDivElement>()

const dataTypes = computed(() => parseAcceptToDataTypes(unref(accept)))

const onDrop = (files: FileList | File[] | null) => {
const onDrop = (files: FileList | File[] | null, fromDropZone = false) => {
if (!files || files.length === 0) {
return
}
Expand All @@ -60,6 +61,18 @@ export function useFileUpload(options: UseFileUploadOptions) {
if (files.length > 1 && !multiple) {
files = [files[0]!]
}

// Sync dropped files to the input element for proper native validation
if (fromDropZone && inputRef.value?.$el) {
try {
const dt = new DataTransfer()
files.forEach(file => dt.items.add(file))
inputRef.value.$el.files = dt.files
} catch (e) {
console.warn('Could not sync files to input element:', e)
}
}

onUpdate(files)
}

Expand All @@ -75,7 +88,7 @@ export function useFileUpload(options: UseFileUploadOptions) {

onMounted(() => {
const { isOverDropZone } = dropzone
? useDropZone(dropzoneRef, { dataTypes: dataTypes.value, onDrop })
? useDropZone(dropzoneRef, { dataTypes: dataTypes.value, onDrop: files => onDrop(files, true) })
: { isOverDropZone: ref(false) }

watch(isOverDropZone, (value) => {
Expand All @@ -85,13 +98,13 @@ export function useFileUpload(options: UseFileUploadOptions) {
const { onChange, open } = useFileDialog({
accept: unref(accept),
multiple,
input: unref(inputRef),
input: unref(inputRef)?.$el,
reset
})

fileDialog.open = open

onChange(fileList => onDrop(fileList))
onChange(fileList => onDrop(fileList, false))
})

return {
Expand Down
Loading
Loading