-
-
Notifications
You must be signed in to change notification settings - Fork 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
Allow cache to be async #26
Conversation
Fixes sindresorhus#3, Fixes sindresorhus#14 Signed-off-by: Richie Bendall <[email protected]>
Signed-off-by: Richie Bendall <[email protected]>
Signed-off-by: Richie Bendall <[email protected]>
Signed-off-by: Richie Bendall <[email protected]>
Signed-off-by: Richie Bendall <[email protected]>
Signed-off-by: Richie Bendall <[email protected]>
Signed-off-by: Richie Bendall <[email protected]>
Signed-off-by: Richie Bendall <[email protected]>
…nto async-cache
Co-authored-by: Sindre Sorhus <[email protected]>
CI is failing. |
Co-authored-by: Sindre Sorhus <[email protected]>
Signed-off-by: Richie Bendall <[email protected]>
For future reference, it would have been better if this pull request was split into a separate pull request for each issue it fixed. Massive pull requests are a pain to review. |
Does this introduce any breaking changes other than removal of |
Signed-off-by: Richie Bendall <[email protected]>
|
Signed-off-by: Richie Bendall <[email protected]>
.get()
and .has()
to return a promise
@Richienb Thanks for working on this. This is ready to be released right? There are no more planned breaking changes? |
@sindresorhus Nothing else from me - it's good to go. |
Hallelujah! Thanks for working on this @Richienb |
has: (key: KeyType) => Promise<boolean> | boolean; | ||
get: (key: KeyType) => Promise<ValueType | undefined> | ValueType | undefined; | ||
set: (key: KeyType, value: ValueType) => void; | ||
delete: (key: KeyType) => void; | ||
clear?: () => void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems unreasonable to expect an async cache to have async has
and get
but synchronous set
, delete
and clear
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was made with the intention that set
, delete
and clear
could return promises but we don't care about waiting for them. We could add support for asynchronous functions as a mostly non-breaking change however.
See #3 (comment)
has: (key: KeyType) => Promise<boolean> | boolean; | ||
get: (key: KeyType) => Promise<ValueType | undefined> | ValueType | undefined; | ||
set: (key: KeyType, value: ValueType) => void; | ||
delete: (key: KeyType) => void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cache.delete
is never called. It should either be used or not required by this interface.
Fixes #3, Fixes #14, Closes #19