Conversation
AyronK
left a comment
There was a problem hiding this comment.
Hey, great work! I added a couple of comments, cosmetic. One more things, if you had time, would you add this integration to the example project? For instance behind an env variable such as REDIS_TYPE=<"ioredis":"node:redis">. Then we could resolve a different client in the cache handler. Just to make it easy to verify.
Thanks for your review! 🙌 |
Cool, I've just returned from a long xmas break. I'll try to test it this week. |
AyronK
left a comment
There was a problem hiding this comment.
I have fixed the import locally and tried to run the example project. It does not seem to work.
- Instrumentation does not add anything, while with the default
redisevery example is properly initialized on startup - Visiting example pages that should trigger cache save, does not save anything
- In logs I see a lot of undefined errors:
[CacheHandler] Instance created with provided context.
[CacheHandler] Using existing CacheHandler configuration.
[CacheHandler] [method: get] [key: /favicon.ico] Started retrieving value in order.
[CacheHandler] [handler: composite] [method: get] [key: /favicon.ico] Started retrieving value.
[CacheHandler] [method: get] [key: /manifest.webmanifest] Retrieving value not found.
pending revalidates promise finished for: { pathname: '/manifest.webmanifest', search: '' }
[CacheHandler] Using existing CacheHandler configuration.
[CacheHandler] [method: set] [key: /manifest.webmanifest] Started setting value in parallel.
[CacheHandler] [method: get] [key: /favicon.ico] Retrieving value not found.
pending revalidates promise finished for: { pathname: '/favicon.ico', search: '' }
[CacheHandler] Using existing CacheHandler configuration.
[CacheHandler] [method: set] [key: /favicon.ico] Started setting value in parallel.
[CacheHandler] [handler: composite] [method: set] [key: /manifest.webmanifest] Error: TypeError: Cannot read properties of undefined (reading 'options')
[CacheHandler] [handler: composite] [method: set] [key: /favicon.ico] Error: TypeError: Cannot read properties of undefined (reading 'options')
@themitvp please address those issues and test the code before asking me to do so 🙏🏻 It seems you have not even tried it with the example project.
I'll take a look again once you're ready.
@AyronK Just pushed the update. I ran the example project locally to be sure, and everything works perfectly now 🎉 Sorry about the mix-up earlier! |
|
I will move it forward on Monday. |
A draft PR to start collaborating on adding support for ioredis.
Solves #154