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

Importing @keyv/redis causes TypeScript error: Cannot find name 'Value'. #512

Closed
rasmuslp opened this issue Oct 7, 2022 · 5 comments · Fixed by #523
Closed

Importing @keyv/redis causes TypeScript error: Cannot find name 'Value'. #512

rasmuslp opened this issue Oct 7, 2022 · 5 comments · Fixed by #523
Labels

Comments

@rasmuslp
Copy link

rasmuslp commented Oct 7, 2022

Describe the bug
Simply loading @keyv/redis in a TS project causes transpilation error:

> tsc

node_modules/@keyv/redis/src/index.d.ts:6:63 - error TS2304: Cannot find name 'Value'.

6 declare class KeyvRedis extends EventEmitter implements Store<Value> {
                                                                ~~~~~

node_modules/@keyv/redis/src/index.d.ts:13:28 - error TS2304: Cannot find name 'Value'.

13  get(key: string): Promise<Value>;
                              ~~~~~

node_modules/@keyv/redis/src/index.d.ts:16:22 - error TS2304: Cannot find name 'Value'.

16  ): Array<StoredData<Value>> | Promise<Array<StoredData<Value>>> | undefined;
                        ~~~~~

node_modules/@keyv/redis/src/index.d.ts:16:57 - error TS2304: Cannot find name 'Value'.

16  ): Array<StoredData<Value>> | Promise<Array<StoredData<Value>>> | undefined;
                                                           ~~~~~

node_modules/@keyv/redis/src/index.d.ts:17:26 - error TS2304: Cannot find name 'Value'.

17  set(key: string, value: Value, ttl?: number): any;
                            ~~~~~


Found 5 errors in the same file, starting at: node_modules/@keyv/redis/src/index.d.ts:6

How To Reproduce (best to provide workable code or tests!)
Im upgrading from

  • "@keyv/redis": "2.3.7",
  • "keyv": "4.3.2"
    but experience the above error when building with TypeScript 4.8.4

Upgrading to

  • "@keyv/redis": "2.5.1",
  • "keyv": "4.5.0"

the following examples causes the above compilation errors.

RedisCache.ts - Note that this is a dumbed down snippet from my project.

import Keyv from 'keyv';
import KeyvRedis from '@keyv/redis';

import { ICache } from './ICache';

export type RedisCacheOptions = {
	namespace: string,
	ttl: number
};

export class RedisCache<T> implements ICache<T> {
	private readonly keyv: Keyv;

	constructor(options: RedisCacheOptions) {
		this.keyv = new Keyv({
			namespace: options.namespace,
			ttl: options.ttl
		});
	}

	public async clear(): Promise<void> {
		return this.keyv.clear();
	}

	public async delete(key: string): Promise<void> {
		await this.keyv.delete(key);

		return undefined;
	}

	public async get(key: string): Promise<any | undefined> {
		return this.keyv.get(key);
	}

	public async set(key: string, value: any, ttl?: number): Promise<void> {
		await this.keyv.set(key, value, ttl);

		return undefined;
	}
}

Looking at the content of https://github.com/jaredwray/keyv/blob/f060943bd21b4de17b5b7be956debad86d6633d1/packages/redis/src/index.d.ts I don't see where Value should come from. Also, none of the other apaters' typedefinitions forward or define this themselves. It's only defined as any in the typedefinitions of keyv itself.
Should the adapters themselves not let this be customisable? Something like

declare class KeyvRedis<Value = any> extends EventEmitter implements Store<Value> {}
@rasmuslp rasmuslp added the bug label Oct 7, 2022
@rasmuslp
Copy link
Author

rasmuslp commented Oct 7, 2022

"@keyv/redis": "2.3.8" also works fine.

@jaredwray
Copy link
Owner

@rasmuslp - we are looking into this and most likely will have a fix soon.

@jaredwray
Copy link
Owner

It looks like this pull request will fix this issue and we will be merging today and releasing in the next couple days. #513

Let us know if you can validate it. Closing this issue at this time since it is being merged.

@jaytist jaytist mentioned this issue Oct 17, 2022
3 tasks
@rasmuslp
Copy link
Author

Works just fine 👍

@jaredwray
Copy link
Owner

@rasmuslp thank you for the reply. Really appreciate it.

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

Successfully merging a pull request may close this issue.

2 participants