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

Docs: timeout parameter for check() #868

Open
thernstig opened this issue Apr 27, 2023 · 4 comments
Open

Docs: timeout parameter for check() #868

thernstig opened this issue Apr 27, 2023 · 4 comments

Comments

@thernstig
Copy link

What is the problem?

https://makenowjust-labs.github.io/recheck/docs/usage/as-javascript-library/ mentions:

There is the timeout parameter to specify timeout seconds. Please use this instead of the manual way.

There are two problems here:

  1. It says seconds while I believe it should say milliseconds
  2. The text "Please use this instead of the manual way." is ambiguous. Does it mean to use an AbortController instead of the timeout parameter? If so the text should be re-phrased. If not, it is hard to understand what the sentence is trying to say.
@makenowjust
Copy link
Contributor

Since timeout was added before AbortController, "manual way" may have been imagined as a lazier way (such as using setTimeout) than using AbortController. I don't remember exactly.

If you have a better idea, a Pull Request is welcome.

@thernstig
Copy link
Author

thernstig commented Apr 27, 2023

@makenowjust I could make a PR, but I am still not sure what the text is trying to say. Your reply did not help enough.

Does the note try to say this?

Does it mean to use an AbortController instead of the timeout parameter?

Or did you mean to use timeout or AbortController, instead of if someone tried setTimeout?

I want to be clear or not if you are just recommending to use either timeout or AbortController, but not setTimeout.

@makenowjust
Copy link
Contributor

@thernstig I list some notes on timeout and AbortController.

  • While timeout works for synchronous and asynchronous executions, AbortController works only for asynchronous execution.
  • If a timeout is implemented using only setTimeout without AbortController and timeout, check may continue to consume CPU resources.
  • Usually, only setting timeout is sufficient to implement a timeout to check. If you want to allow users to cancel heavy check, AbortController is helpful.

If you have any other questions, please feel free to ask.

@thernstig
Copy link
Author

I will try to send a PR in the coming days, so I also have contributed something back to this project since I am using it :)

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

2 participants