Add support for input file on fireEvent.update

[JeromeDeLeon]

#177

[afontcu]

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()
})

[JeromeDeLeon]

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?

[JeromeDeLeon]

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.

[afontcu]

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 👍

[JeromeDeLeon]

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.

[afontcu]

I'd definitely add the else if, and also a test example on how to input a file! Fancy to do so?

[JeromeDeLeon]

Is this test example not enough? @afontcu

[afontcu]

It is! But as mentioned, I wouldn't make fireEvent.update() smarter when it comes to type="file" inputs. I would just trigger the proper event, and then add a test showcasing how to handle files as in React Testing Library (and probably also a comment suggesting userEvent to do so:

test('fireEvent.update should not crash with input file', async () => {
  const {getByTestId} = render({
    template: `<input 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()
})

[afontcu]

I added a suggestion with the change I mention above 👍

[codecov]

Codecov Report

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.

[afontcu]

Suggested change

	} else if (type === 'file') {
	Object.defineProperty(elem, 'files', {value})
	return fireEvent.change(elem)
	} else if (type === 'file') {
	return fireEvent.change(elem)

(also not 100% sure if we should trigger a change event over an input one? 🤔 haven't checked, so it might be okay)

[JeromeDeLeon]

Seems pretty okay to me since we know we can't use v-model to an input file and the only reasonable event to use in this case is change.

[afontcu]

nice! 🙌

[github-actions]

🎉 This PR is included in version 5.6.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This PR is included in version 6.6.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀