Closed JeromeDeLeon closed 3 years ago
Hi! Can't fix it right now, but you can use userEvent.upload(). It works fine with your example.
That being said, I think the problem is that we should avoid setting value
attribute when type is file
(source):
case 'INPUT': {
if (['checkbox', 'radio'].includes(type)) {
elem.checked = true
return fireEvent.change(elem)
} else if (type === 'file') {
return fireEvent.input(elem)
} else {
elem.value = value
return fireEvent.input(elem)
}
}
With that, the following test passes (copied from the test you linked in the issue):
test('fireEvent.update should not crash with input file', async () => {
const {getByTestId} = render({
render(h) {
return h('input', {
attrs: {
type: 'file',
'data-testid': 'test-update',
},
})
},
})
const file = new File(['(ββ‘_β‘)'], 'chucknorris.png', {
type: 'image/png',
})
const inputEl = getByTestId('test-update')
Object.defineProperty(inputEl, 'files', {
value: [file],
})
await fireEvent.update(inputEl)
expect(console.warn).not.toHaveBeenCalled()
})
if (['checkbox', 'radio'].includes(type)) {
elem.checked = true
return fireEvent.change(elem)
} else if (type === 'file') {
Object.defineProperty(elem, 'files', { value })
return fireEvent.change(elem)
} else {
elem.value = value
return fireEvent.input(elem)
}
Why can't it be like this? instead of doing manual population of files property every time we use input file?
Yeah, it works fine with upload
and works just fine even with using just fireEvent.change
manually, the only reason I want it by default is to avoid the warning that's all.
if (['checkbox', 'radio'].includes(type)) { elem.checked = true return fireEvent.change(elem) } else if (type === 'file') { Object.defineProperty(elem, 'files', { value }) return fireEvent.change(elem) } else { elem.value = value return fireEvent.input(elem) }
Why can't it be like this? instead of doing manual population of files property every time we use input file?
That would make sense. However I'm not sure if we should step into userEvent
realm with fireEvent
. The only reason fireEvent.update
exists is to overcome some limitations when it comes to async DOM updates. Now, handling <input type="file">
differenty is probably unexpected, and better handled by userEvent
.
I'm ok with adding the else if
case to avoid the warning when adding files, though π
Yeah, you're right with the purpose of fireEvent.update
. It should only be used when we're using the v-model
or value/ (change/input)
case and for the case of the input file, we're not allowed to set up a value so the ability to turn off the warning is fine.
I'd definitely add the else if
, and also a test example on how to input a file! Fancy to do so?
Merging #179 (59e3946) into master (a051013) will not change coverage. The diff coverage is
100.00%
.
@@ Coverage Diff @@
## master #179 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 98 100 +2
Branches 34 35 +1
=========================================
+ Hits 98 100 +2
Impacted Files | Coverage Ξ | |
---|---|---|
src/vue-testing-library.js | 100.00% <100.00%> (ΓΈ) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Ξ = absolute <relative> (impact)
,ΓΈ = not affected
,? = missing data
Powered by Codecov. Last update a051013...59e3946. Read the comment docs.
nice! π
:tada: This PR is included in version 5.6.0 :tada:
The release is available on:
Your semantic-release bot :package::rocket:
:tada: This PR is included in version 6.6.0 :tada:
The release is available on:
Your semantic-release bot :package::rocket:
https://github.com/testing-library/vue-testing-library/issues/177