Skip to content

Add ability to await fireEvent #30

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

Closed
EmilTholin opened this issue Jun 4, 2019 · 7 comments
Closed

Add ability to await fireEvent #30

EmilTholin opened this issue Jun 4, 2019 · 7 comments

Comments

@EmilTholin
Copy link
Contributor

EmilTholin commented Jun 4, 2019

As was outlined in this issue, you sometimes need to wait for the next microtask to run before changes in the DOM have been flushed.

const input = getByTestId('my-input');
fireEvent.input(input, {target: {value: 'some text'}});
// Wait for the next microtask for `value` to be 
// written to a potential bound variable
await tick();
// Check the value of the variable somehow

Maybe we could create a small abstraction over fireEvent from dom-testing-library and return a tick promise so that fireEvent can be awaited directly?

const input = getByTestId('my-input');
await fireEvent.input(input, {target: {value: 'some text'}});
// Check the value of the variable somehow
import {
  getQueriesForElement,
  prettyDOM,
  fireEvent as dtlFireEvent
} from '@testing-library/dom'
import { tick } from 'svelte'
export * from '@testing-library/dom'

export function fireEvent(...args) {
  dtlFireEvent(...args)
  return tick()
}
@alexkrolick
Copy link

alexkrolick commented Jun 4, 2019

Would it make sense to make it a "thenable" instead of a actual promise to avoid dangling promise chains in the tests?

@pngwn
Copy link

pngwn commented Jun 5, 2019

This isn't actually necessary due to how Svelte's updates work. Svelte batches updates and processes them in the next microtask, in order to ensure changes have been flushed to the DOM we can simply await the action that triggered the update like this:

test('clicking the button should change the text', async () => {
  const { getByText } = render(Hello);

  const button = getByText('Change');
  const button2 = getByText('Change Again');
  const h1 = getByText('Hello World');

  await fireEvent.click(button);
  expect(h1.innerHTML).toBe('Hello Everybody');

  await fireEvent.click(button2);
  expect(h1.innerHTML).toBe('Hello Nobody');
});

This also works when performing multiple actions:

test('updating Checkbox inputs should work', async () => {
  const { getByText, getAllByLabelText } = render(Inputs);

  const inputs = getAllByLabelText(/Rice|Beans|Cheese|Guac/);
  const output = getByText(/Value CBG:/);

  await inputs.map(element => fireEvent.click(element));
  expect(output.innerHTML).toBe('Value CBG: Rice,Beans,Cheese,Guac (extra)');
});

I don't think this is a commonly known detail and perhaps warrants documentation, there may be times when this approach fails but a simple await tick() is always an option in such cases. Awaiting events, like in the above examples, should work in the majority of cases.

@alexkrolick
Copy link

I'm fairly certain awaiting something that is not a Promise does not have the effect you are describing - it just returns immediately as if the await wasn't there. I think we exolored this before @kentcdodds?

@kentcdodds
Copy link
Member

I believe so. Either way what you really want to use is findBy and findAllBy queries instead of awaiting the fireEvent API.

@kentcdodds
Copy link
Member

Actually, based on the example above what you probably should be doing is wrapping the assertion inside wait

@pngwn
Copy link

pngwn commented Jun 5, 2019

I'm fairly certain awaiting something that is not a Promise does not have the effect you are describing - it just returns immediately as if the await wasn't there.

This is incorrect. Awaiting a non-promise/ thenable wraps the value in a promise or something to that effect. As I understand it, await fn() will always wait until the microtask queue is flushed regardless of what fn returns. So if fn() adds something to the microtask queue (in this case triggering some state change that adds a flushUpdates task to the queue) then there is no need to fn(); await tick();. Consider it a 'convenience hack', if you will, but it definitely works.

Regardless, the above code snippets are not theoretical examples but taken from actual tests that run and pass as expected, removing await breaks them, naturally. I personally feel that the ergonomics of awaiting the action that causes the update are better than using a helper function to wrap assertions in almost every test.

Examples of the aforementioned tests in action can be found here and here

@kentcdodds
Copy link
Member

Thanks for clarifying @pngwn 👍

@EmilTholin EmilTholin mentioned this issue Aug 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants