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

Request support for ioredis #2

Closed
seolhw opened this issue Apr 24, 2023 · 16 comments
Closed

Request support for ioredis #2

seolhw opened this issue Apr 24, 2023 · 16 comments
Labels
enhancement New feature or request
Milestone

Comments

@seolhw
Copy link

seolhw commented Apr 24, 2023

No description provided.

@MGTechTeam1
Copy link

Bump +1

1 similar comment
@titk12
Copy link

titk12 commented May 23, 2023

Bump +1

@rajshrimohanks
Copy link

This is much needed. Right now, the only reason why this package depends on node-redis is because the code uses the commandOptions method to set the .xread() method to run in a new Redis connection. This is node-redis specific syntax. Just making this generic would allow ioredis also to be used.

@1finedev
Copy link

Bump +1

@troyflinn
Copy link

Also like to see ioredis support for high availability application deployment utilising redis in sentinel mode. Sentinel not currently supported by node-redis, only ioredis.

@cody-evaluate
Copy link

cody-evaluate commented Oct 6, 2023

Bump +1 for ioredis support

@artyomtrubchik
Copy link

My situation is that I have to use ioredis. +1

@cyxn
Copy link

cyxn commented Jan 19, 2024

+1

darrachequesne added a commit that referenced this issue Feb 13, 2024
@darrachequesne
Copy link
Member

Support for ioredis has been added in 58faa1d, included in release 0.2.0.

@darrachequesne darrachequesne added the enhancement New feature or request label Feb 21, 2024
@darrachequesne darrachequesne added this to the 0.2.0 milestone Feb 21, 2024
@jwetzell
Copy link
Contributor

jwetzell commented Feb 27, 2024

@darrachequesne With the dependency on the commandOptions function from redis am I right that it is still required that projects that are using this library include redis as a dependency whether they are using redis or ioredis? If so why is redis not an actual dependency of this library and not just a dev dependency?

@darrachequesne
Copy link
Member

@jwetzell that's a good question. Not sure how we can get rid of this import. Any idea?

Reference: https://github.com/redis/node-redis/blob/dbf8f59a47573e6a1c75b78e566af8c493015d5d/packages/client/lib/command-options.ts#L7-L10

@jwetzell
Copy link
Contributor

jwetzell commented Mar 11, 2024

@darrachequesne looks like the commandOptions function is exposed on the redisClient itself as well. Should just be able to call redisClient.commandOptions({isolated:true}) which I quickly tested and both ioredis and redis tests pass. It does seem like it is necessary as removing it does make the redis package not happy.

@darrachequesne
Copy link
Member

@jwetzell nice! Does it work with a Redis cluster too?

@jwetzell
Copy link
Contributor

@darrachequesne 😞 it does not... that function doesn't seem to be exposed on the Cluster class from node-redis. That is real annoying.

@thiagon
Copy link

thiagon commented Apr 4, 2024

At least redis should be as a peerDependencies, like socket.io-adapter

But is it really necessary to have a dependency just for this function?
https://github.com/redis/node-redis/blob/dbf8f59a47573e6a1c75b78e566af8c493015d5d/packages/client/lib/command-options.ts#L7-L10

export function commandOptions<T>(options: T): CommandOptions<T> {
    (options as any)[symbol] = true;
    return options as CommandOptions<T>;
}

@jwetzell
Copy link
Contributor

jwetzell commented Apr 4, 2024

At least redis should be as a peerDependencies, like socket.io-adapter

But is it really necessary to have a dependency just for this function? https://github.com/redis/node-redis/blob/dbf8f59a47573e6a1c75b78e566af8c493015d5d/packages/client/lib/command-options.ts#L7-L10

export function commandOptions<T>(options: T): CommandOptions<T> {
    (options as any)[symbol] = true;
    return options as CommandOptions<T>;
}

As far as I can tell for node-redis the first argument needs to pass the isCommandOptions test located in the same file as that function you linked. The first line in the commandOptions function is the deal breaker. The use of the symbol primitive as the key means we can't just create an object that would pass that test without that symbol.

darrachequesne added a commit that referenced this issue May 8, 2024
This import meant the redis package was required even if it was not
used to create a Redis client. It is replaced by a dynamic import,
which is supported since Node.js v13.2.0.

Related:

- #24
- #2
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