fix: use flushPromise() instead of waitFor()

[hershelh]

The current fireEvent() uses waitFor() after triggering event handler to ensure that the DOM is updated in a timely manner, passing in an empty function and awaiting it. await waitFor (() => {}) returns a promise that resolves shortly after. Since Vue also uses promises or other microtasks to update the DOM, when the microtask generated by the promise returned by calling waitFor() is executed by the event loop, the microtask of updating the DOM can also be executed. Users only needs to await fireEvent() to ensure the update of the DOM.

This seems very reasonable. But in some cases, such handling can cause some problems, causing the update of the DOM to be performed after the assertion written by the user.

The case is: some component libraries, like vant, encapsulate form that perform form validation before calling the event callback passed by the user when processing the submit event, and their function for validation will use nested promises to do check (usually a combination of promise and Promise.all()), which leads to a problem: when the microtask generated by waitFor() is executed, the microtask of Vue's update DOM has not been executed! So the user's assertion will be executed earlier than the task of updating the DOM, causing the test to fail.

This situation can be reproduced as follows:

const validate = () => {
  return new Promise((resolve) => {
    Promise.resolve().then(() => {
      resolve(1)
    })
  })
}

const submit = () => {
  validate().then(() => {
    // Simulate Vue to generate microtask that update the DOM
    Promise.resolve().then(() => {
      console.log('dom update')
    })
  })
}

const fireEvent = async () => {
  submit() // submit the form
  await Promise.resolve()// simulate `waitFor(() => {})`
}

const myTest = async () => {
  await fireEvent()
  
  console.log('assertion')
}

myTest()

I also provide a reproduction here.

Obviously, the above situation is not in line with the design purpose of fireEvent() and the expectations of users. Users who do not know the reason may wonder whether the implementation of their source code is wrong or whether there is a problem with the test code, but will not think it's a problem with fireEvent() itself, because the docs they've seen tells them that the function is guaranteed to update the dom.

For users, one solution is to use waitFor() to wrap the assertion and wait for the dom to update. This can let the test to pass, but also adds redundant code. It also makes users lose confidence in writing test code, because they can't be sure that the dom will update immediately after they call and await fireEvent().

So I think it is necessary to modify the internal implementation of fireEvent(). I use the flushPromise() exported by vue/test-utils instead of waitFor(). In addition to using promises, flushPromise() calls setImmediate/setTimeout to generate a macroTask that calls the resolve() method. Since the macroTask will be executed after the microTask, this ensures that all microTasks generated after the fireEvent() call, including DOM updates, will be executed before the macroTask. So replacing waitFor() with flushPromises() guarantees that dom updates can be done before the user's assertion.

[hershelh]

@afontcu Hi! Could you please take a look? Thank you!

[afontcu]

@afontcu Hi! Could you please take a look? Thank you!

Hi! I'm afk until October, will take a look then! The PR looks sensible but I think I'll need to take a closer look and probably issue a breaking change.

thank you!

[hershelh]

Hi! I'm afk until October, will take a look then! The PR looks sensible but I think I'll need to take a closer look and probably issue a breaking change.

thank you!

Well, I see. Thank you very much for your reply! I hope it won't take up your time.

[hershelh]

Hi. @afontcu

Excuse me. Do you have time to take a look at this?

[saifahn]

I have a question - why not use await findBy... instead of getBy... in the assertions after firing the event(s)?

[hershelh]

I have a question - why not use await findBy... instead of getBy... in the assertions after firing the event(s)?

I already explained that because the documentation has promised that "await fireEvent() ensures the DOM is properly updated in response to an event in a test", but obviously there are some cases where it can't cover, and that's why we need to use a more safer API to ensure our assertions are called correctly after all micro tasks are completed.