Skip to content
This repository has been archived by the owner on Jan 15, 2021. It is now read-only.

Improve timeout util #1645

Merged
merged 7 commits into from
Nov 30, 2020
Merged

Improve timeout util #1645

merged 7 commits into from
Nov 30, 2020

Conversation

Velenir
Copy link
Contributor

@Velenir Velenir commented Nov 30, 2020

  • Fixes types
  • Simplifies timeout function body to reuse already available utlils/delay
  • Fixes more types

You can see the usefulness of Promise<never> here

export function timeout<T>(params: TimeoutParams<T>): Promise<T> {
export function timeout(params: TimeoutParams<undefined>): Promise<never> // never means function throws
export function timeout<T>(params: TimeoutParams<T extends undefined ? never : T>): Promise<T>
export async function timeout<T>(params: TimeoutParams<T>): Promise<T | never> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be renamed to promisedTimeout or sth, to indiciate it is awaitable in nature or actually i dont care

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, I also don't really care

@ghost
Copy link

ghost commented Nov 30, 2020

Travis automatic deployment:

@W3stside
Copy link
Contributor

W3stside commented Nov 30, 2020 via email

@W3stside W3stside self-requested a review November 30, 2020 10:27
@Velenir Velenir merged commit aafc2ae into timeout-fetching-tokens Nov 30, 2020
@Velenir Velenir deleted the improve_timeout_util branch November 30, 2020 11:11
Velenir added a commit that referenced this pull request Nov 30, 2020
* Add timeout for fetching tokens

* Improve timeout util (#1645)

* simplify type without casting

* default delay() return to void

* simplify timeout function logic

* improve timeout types

* use new timeout with automatic rejected Promise detection

* construct cacheKey once only

* throw proper Error

Co-authored-by: Velenir <[email protected]>
@anxolin
Copy link
Contributor

anxolin commented Dec 2, 2020

Cool, TS 🧙‍♂️

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants