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

Error: ioredis dependency not found #15

Open
JDunbarBH opened this issue Jun 8, 2021 · 4 comments · May be fixed by #17
Open

Error: ioredis dependency not found #15

JDunbarBH opened this issue Jun 8, 2021 · 4 comments · May be fixed by #17

Comments

@JDunbarBH
Copy link

Hello,
Before my question - thanks for taking time to publish and maintain this library. NPM has a large number of rate limiting libraries, and I've tested quite a few. Very few handle both request rate and concurrency as cleanly, which should always go hand in hand since most servers will either 500 when overloaded or expressly prevent too high a concurrency. We're using your package to limit requests to a 3rd party api, for both proxied client uses (portioned using your redis manager) and for maintenance tasks (which utilize the full rates) - except for the below issue, so far its been good.

While integrating your package, we found an immediate problem like to how you're using package dependencies as runtime types to detect the redis client used. We do not use ioredis, but have been forced to install this ourselves as a dependency in our node project as it will fail to compile w/o doing so.

offending code, '~/build/src/quota/redisQuotaManager.ts' lines 1-5:

import { Quota } from './quota';
import { QuotaManager } from './quotaManager';
import { RedisClient } from 'redis';
import * as IORedis from 'ioredis';  // <= fails here w/o manually running 'npm i --save ioredis'
declare type RedisCompatibleClient = RedisClient | IORedis.Redis | IORedis.Cluster;

From the above and its usages, it seems like you're using packages directly in a union type as a means of feature detection. If I may make a suggestion, maybe consider just creating a generic type definition with the methods you require, and inspect/validate that the unknown/provided redis-client has them instead. Using the packages directly this way seems a bit heavy... even if these were say bundled w/ node or some other ubiquitous dependency. Regardless of how its solved, I think the redis client dependencies are an issue that should be addressed.

Thanks again for all your efforts!
Jared D.

@natesilva
Copy link
Owner

natesilva commented Jun 8, 2021

I agree that we should be Redis-client agnostic. This is something I’ve thought about, but I haven’t had the bandwidth to try. We should accept any client that conforms to the de-facto standard callback interface given in node-redis, or the equivalent Promise interface. That’s basically every client.

I tried this in a side project. Maybe that could be adapted. Or I would be open to another approach as well.

@remorses
Copy link

You can use the following syntax to remove the ioredis dependency

type Redis = import('ioredis').Redis

@remorses remorses linked a pull request Oct 19, 2021 that will close this issue
@zipzapzanigan
Copy link

zipzapzanigan commented May 7, 2022

I hit this same issue when compiling using tsc:

$ shx rm -rf dist && tsc -b
node_modules/p-ratelimit/build/src/quota/redisQuotaManager.d.ts:3:29 - error TS2307: Cannot find module 'redis' or its corresponding type declarations.

3 import { RedisClient } from 'redis';
                              ~~~~~~~

node_modules/p-ratelimit/build/src/quota/redisQuotaManager.d.ts:4:26 - error TS2307: Cannot find module 'ioredis' or its corresponding type declarations.

4 import * as IORedis from 'ioredis';
                           ~~~~~~~~~

installing redis, and ioredis, and @types/redis doesn't make this error go away...
I found this bug that seems related, but has no accepted solutions:

https://stackoverflow.com/questions/70251426/module-redis-has-no-exported-member-redisclient

The one solution of installing redis@3 diddn't work for me, same issue was apparent.

my current short term solution is to comment out RedisQuoteManager from exports in index.d.ts:

export { pRateLimit } from './rateLimit';
export { Quota } from './quota/quota';
export { QuotaManager } from './quota/quotaManager';
// export { RedisQuotaManager } from './quota/redisQuotaManager';
export { RateLimitTimeoutError } from './rateLimitTimeoutError';

@Tanv33
Copy link

Tanv33 commented Nov 17, 2023

I hit this same issue when compiling using tsc:

$ shx rm -rf dist && tsc -b
node_modules/p-ratelimit/build/src/quota/redisQuotaManager.d.ts:3:29 - error TS2307: Cannot find module 'redis' or its corresponding type declarations.

3 import { RedisClient } from 'redis';
                              ~~~~~~~

node_modules/p-ratelimit/build/src/quota/redisQuotaManager.d.ts:4:26 - error TS2307: Cannot find module 'ioredis' or its corresponding type declarations.

4 import * as IORedis from 'ioredis';
                           ~~~~~~~~~

installing redis, and ioredis, and @types/redis doesn't make this error go away... I found this bug that seems related, but has no accepted solutions:

https://stackoverflow.com/questions/70251426/module-redis-has-no-exported-member-redisclient

The one solution of installing redis@3 diddn't work for me, same issue was apparent.

my current short term solution is to comment out RedisQuoteManager from exports in index.d.ts:

export { pRateLimit } from './rateLimit';
export { Quota } from './quota/quota';
export { QuotaManager } from './quota/quotaManager';
// export { RedisQuotaManager } from './quota/redisQuotaManager';
export { RateLimitTimeoutError } from './rateLimitTimeoutError';

Ran into the same compilation snag you've been discussing. Found a neat fix:

Instead of:
import { pRateLimit } from 'p-ratelimit';

Try this:
import { pRateLimit } from 'p-ratelimit/build/src/rateLimit';

Why it's cool:

Bypasses node_modules tweaks.
Gets straight to the pRateLimit goodness.
Worked like a charm for me. Hope it does for you too! Happy coding! ♥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants