fix: [EXT-3586] add default cache #41
Conversation
src/keys/get-management-token.ts
Outdated
| @@ -16,6 +16,8 @@ export interface GetManagementTokenOptions { | |||
| reuseToken?: boolean | |||
| } | |||
|
|
|||
| const defaultCache = new NodeCache() | |||
There was a problem hiding this comment.
What about
let defaultCache: NodeCache;
export const getManagementToken = (privateKey: string, opts: GetManagementTokenOptions) => {
if (opts.reuseToken) {
defaultCache = new NodeCache();
}
...
So we don't over-allocate if it's not needed. We could also cleanup createGetManagementToken at that point
There was a problem hiding this comment.
Yes that makese sense. I changed it in that way and then removed the assignment in the createGetManagementToken as this is not needed anymore. Let me know if that is what you meant with cleanup :D
| opts | ||
| ) | ||
| if (opts.reuseToken || opts.reuseToken === undefined) { | ||
| defaultCache = new NodeCache() |
There was a problem hiding this comment.
I am a bit confused by this. Doesn't it mean that we are initialising the cache each time you a user tries gets a token?
There was a problem hiding this comment.
Yes you are right, I added a check that this only happens once
| if ((opts.reuseToken || opts.reuseToken === undefined) && !defaultCache) { | ||
| defaultCache = new NodeCache() | ||
| } | ||
| return createGetManagementToken( |
There was a problem hiding this comment.
I guess we should now ask why this function even exists. We could just inline the createGetManagementToken into this function, and it probably makes the code slightly easier to follow. I don't think we need to do it in this PR though.
## [2.0.1](v2.0.0...v2.0.1) (2022-03-11) ### Bug Fixes * [EXT-3586] add default cache ([#41](#41)) ([420cdfb](420cdfb))
|
🎉 This PR is included in version 2.0.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
ghost
added
the
fixes #40