Add ioredis to problematic-modules.md #430
Conversation
|
Woah I had no idea we have had that list. that list seems pretty, uhm, problematic (😅)? At least with the wording used, especially in the file name. Honestly I don't think we should have a list unless we're committed to work with package authors to improve the situation. And if we decide to have a list, it should be triaged often and we should have links/examples showing how the library is problematic/breaks async continuity. |
|
Added to the agenda so we can discuss what to do with this list |
Indeed +1. |
|
@mmarchini It's nice that you added it to the agenda, especially since the comments gathered here is more about the existence of the doc than the change. As a user in the enterprise world with hard requirements that in turn forces us to use async_hooks, there's a huge value in having the document as we often won't see the issues before we're in production. Too bad it's not being actively maintained though - I guess it's better not to have it than having it unmaintained. |
|
I agree that we should remove the list. |
| @@ -18,3 +18,4 @@ Module | Embedder API | Note | |||
| [generic-pool](https://github.com/coopernurse/node-pool) | No | Used by a lot of database modules like for instance "pg" | |||
| [mongoose](https://github.com/Automattic/mongoose/) | [#5929](https://github.com/Automattic/mongoose/issues/5929) | Most popular MongoDB ORM/ODM that implements its own user-land queueing. | |||
| [graphql-subscriptions](https://github.com/apollographql/graphql-subscriptions/blob/26ed503566ecfab086e7530f0daf12b60ee3049c/src/pubsub-async-iterator.ts) | No | De-facto standard for working with GraphQL subscriptions. Custom queueing via an async iterator written in TypeScript and compiled down to ES5. | |||
| [ioredis](https://github.com/luin/ioredis) | No | The only commonly used redis client with support for sentinels | |||
There was a problem hiding this comment.
As a question is the other widely used redis package problematic (https://www.npmjs.com/package/redis) as well ?
There was a problem hiding this comment.
@mhdawson I have no idea actually. It doesn't support sentinels, so I can't really try it out at work.
|
As decided in the meeting we'll remove this file and create separate issues for each module that breaks async continuity, so we can work with module maintainers to fix it. |
|
@mmarchini I understand! What does that mean for this PR? Should I close it? |
|
Keep it open, we can close it once the issues are created (will try to get it done this week) |
|
Issues created for all modules in the file (plus this one): https://github.com/nodejs/diagnostics/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc+label%3Aasync-continuity I'll close this PR and remove the file now. @cupofjoakim thank you for bringing it to our attention! |
No description provided.