Skip to content
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

Promise handling in waitFor #686

Open
mpeyper opened this issue Aug 31, 2021 · 1 comment
Open

Promise handling in waitFor #686

mpeyper opened this issue Aug 31, 2021 · 1 comment
Labels
enhancement New feature or request request for comment Discussion about changes to the library

Comments

@mpeyper
Copy link
Member

mpeyper commented Aug 31, 2021

Describe the feature you'd like:

This feature has been mentioned previously in #631 and in discord

The ability to use a Promise with waitFor to wrap async behaviour in act . This is inline with waitFor from @testing-library/dom, but I'm also thinking it would be great to support all of the following:

// currently supported

await waitFor(() => {
  expect(actual).toBe(expected)
})

await waitFor(() => booleanValue)

// proposed new values supported by `@testing-library/dom`

await waitFor(new Promise<void>(resolve => setTimeout(resolve, 100)))

// proposed new values not supported by `@testing-library/dom`

await waitFor( new Promise<boolean>(resolve => setTimeout(() => resolve(true), 100))) // would repeat if resolved to `false`

await waitFor(() => new Promise<void>(resolve => setTimeout(resolve, 100)))

await waitFor(async () => {
  await new Promise<void>(resolve => setTimeout(resolve, 100))
})

await waitFor(() => new Promise<boolean>(resolve => setTimeout(() => resolve(true), 100))) // would repeat if resolved to `false`

await waitFor(async () => {
  await new Promise<void>(resolve => setTimeout(resolve, 100))
  return true // would repeat if returning to `false`
})

Essentially the waitFor would look something like this:

function waitFor(callback: (() => boolean | void | Promise<boolean | void>) | Promise<boolean | void>, options?: WaitOptions) {
 // ...
}

The reason I want to support returning promises from the callback as well as instead of the callback is so that we can ensure any state updates that happen before the first await in a async function occur within the act block, e.g.

function useAuth() {
  const [user, setUser] = useState<User | null>(null)
  const [isAuthenticated, setIsAuthenticated] = useState(false)

  const signIn = async ({ email, password }:{ email: string; password: string; }) => {
    const user = await login(email, password)

    setUser(user)
    setIsAuthenticated(true)
  }
  
  const signOut = async () => {
    setUser(null)
    setIsAuthenticated(false)

    await logout(user)
  }

  return { user, isAuthenticated, signIn, signOut }
}
test('example', async () => {
  const { result, waitFor } = renderHook(() => useAuth());

  await waitFor(result.current.signIn({ email: '[email protected]', password: 'valid-password' }))

  expect(result.current.user).toBeDefined();
  expect(result.current.isAuthenticated).toEqual(true);

  // If we just pass `await waitFor(result.current.signOut())` here, there is an `act` warning because
  // `setUser` and `setIsAuthenticated` are called synchronously before the promise is returned
  await waitFor(() => result.current.signOut())

  expect(result.current.user).toEqual(null);
  expect(result.current.isAuthenticated).toEqual(false);
})

Suggested implementation:

No idea yet. It's probably more complicated than I think it is right now.

Describe alternatives you've considered:

None.

Teachability, Documentation, Adoption, Migration Strategy:

Updating the API docs and possibly adding more examples in the async hooks section. There is also another ticket to take a more holistic look at the async docs already (#625).

@mpeyper mpeyper added enhancement New feature or request request for comment Discussion about changes to the library labels Aug 31, 2021
@joshuaellis
Copy link
Member

I think this looks like a really great addition. I can definitely see the use cases. And yes, It would be a good time to look at updating those async docs too. I might try and have a play myself if I have some free time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request request for comment Discussion about changes to the library
Projects
None yet
Development

No branches or pull requests

2 participants