Bug: Timeout of `Infinity` causes `waitFor` to reject immediately

[tom-sherman]

Description

This is caused by the use of setTimeout - setTimeout(fn, Infinity) invokes fn immediately.

Expected result

Either:

• Throw an explicit error on infinite timeout values. It shouldn't reject instantly, this leaks implementation details of waitFor
• Remove the timeout

I think the second option is better as it allows for deferring the timeout to the user. I agree that a timeout is the right default but there are definitely scenarios where you want to handle that yourself.

Actual result

Instant rejection of the promise returned by waitFor

Reproduction

https://codesandbox.io/s/stoic-browser-ev9s97?file=/src/index.js

Additional context

No response

[davidkpiano]

I agree with the second option - removing the timeout. Can you make a PR for this? 🙏

[tom-sherman]

On it!

[tom-sherman]

Do we want to error when passed -Infinity? And if so, should this be a synchronous or asynchronous error?

[Andarist]

@tom-sherman from the spec:

If timeout is less than 0, then set timeout to 0.

So I'm OK with just ignoring -Infinity and let the handler to be invoked immediately~ in that case.

[tom-sherman]

Fair enough. Although I think deferring to the spec is a bit leaky, I wouldn't expect users to know that waitFor uses setTimeout under the hood.

[Andarist]

We can add a dev-only warning about negative numbers - that would handle more cases without special-casing -Infinity