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

Further cache improvements (async cache key generation) #875

Closed
1 task done
MiniDigger opened this issue Jan 25, 2023 · 3 comments
Closed
1 task done

Further cache improvements (async cache key generation) #875

MiniDigger opened this issue Jan 25, 2023 · 3 comments
Labels
enhancement New feature or request

Comments

@MiniDigger
Copy link
Contributor

Describe the feature

Additionally to #874, I have usecases for async cache key generation and null cache keys (that should lead to a cache bypass)

Basically, my caching key is generated by a 3rd party service, so my getKey method makes a network call (so returns a promise). Additionally, if that network call fails, I don't want the default cache key generation to happen, so my getKey method needs a way to tell nitro that.

what I would propose is adding an awaits here
https://github.com/unjs/nitro/blob/v2.0.0/src/runtime/cache.ts#L120
https://github.com/unjs/nitro/blob/v2.0.0/src/runtime/cache.ts#L161
and making this async
https://github.com/unjs/nitro/blob/v2.0.0/src/runtime/cache.ts#L160

how to handle the second part of my request I don't know yet. Maybe returning false from getKey and checking for that here and here?
https://github.com/unjs/nitro/blob/v2.0.0/src/runtime/cache.ts#L162
https://github.com/unjs/nitro/blob/v2.0.0/src/runtime/cache.ts#L120
(if key == false, return fn(...args) similar to https://github.com/unjs/nitro/pull/874/files#diff-b6cbd58c14e4265d5cee3c13fc5bdaff3df92a7958f2a6c12c4a471ab49efe27R121)

if that sounds like something you want, I can make that PR

Additional information

  • Would you be willing to help implement this feature?
@pi0 pi0 added enhancement New feature or request and removed pending triage labels Jan 25, 2023
@pi0
Copy link
Member

pi0 commented Jan 25, 2023

Async key support seems a good idea 👍🏼 By returning a falsy, i think fallback to default key is safer. We have explicit shouldByPassCache option now :)

@MiniDigger
Copy link
Contributor Author

PR'd the first part, idk what to do with the second, maybe I need to rethink my logic a bit, but I basically want to handle the case where the getKey promise is rejected? its not too important to me rn, so am fine with this issue being closed by the PR if nobody has an idea.

@pi0
Copy link
Member

pi0 commented Jan 25, 2023

Error handling would be good idea indeed however also applies to current sync support.

Thanks for PR!

@pi0 pi0 closed this as completed Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants