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

Broken types #552

Open
TrejGun opened this issue Sep 29, 2024 · 10 comments
Open

Broken types #552

TrejGun opened this issue Sep 29, 2024 · 10 comments
Labels
bug Something isn't working

Comments

@TrejGun
Copy link

TrejGun commented Sep 29, 2024

Describe the bug

image

How to fix

rollback types for

useFactory?: (...args: unknown[]) => RedisModuleOptions | Promise<RedisModuleOptions>;

it should be any not unknown

@TrejGun TrejGun added the bug Something isn't working label Sep 29, 2024
@TrejGun
Copy link
Author

TrejGun commented Sep 29, 2024

I also want to share some feedback:

First of all thanks for your work

While I understand that there was a reason to make all these changes the interface of v9 was better. I especially miss decorators.

I tried to migrate to the new version but the health indicator is not updated and useFactory has wrong typings so I decided to stay on v9 for now

@lluiscab
Copy link

lluiscab commented Oct 1, 2024

Just tried to update to v10 and found the same issue. I'll postpone updating until this issue is resolved.

@ninja-
Copy link

ninja- commented Oct 8, 2024

I decided to ignore the error like that until the fix is there - since I am doing a big version jump and there are lot of related dependencies, I would rather not spend time checking how compatible they are with v9 right now

everything else works :)

    RedisModule.forRootAsync({
      // @ts-expect-error
      useFactory: async (config: ConfigService) => ({
        readyLog: true,
        config: {
          host: config.get('REDIS_HOST'),
          port: +config.get('REDIS_PORT'),
          password: '',
        },
      }) as any,
      imports: [ConfigModule],
      inject: [ConfigService],
    }),

@KlausNie
Copy link

I'm doing something similar to @ninja-

  RedisModule.forRootAsync({
    useFactory: ((configService: ConfigService) => {
      return redisConfig(configService);
    } ) as (...args: unknown[]) => Promise<RedisModuleOptions>,
    inject: [ConfigService],
  }),

I'm just curious what prompted this change?
I'm guessing any is not very restrictive

@MaksimKiselev
Copy link

Same issue

@iamkanguk97
Copy link

image

I got the same issue! Can I get some feedback?

@R11baka
Copy link

R11baka commented Nov 7, 2024

@iamkanguk97 can you provide steps to reproduce ?can you share your code for useFactory ?
because I do not have issue
Screenshot 2024-11-07 at 15 34 32

@liaoliaots
Copy link
Owner

liaoliaots commented Dec 14, 2024

This bug has been fixed(#554), please wait for the next major release: nestjs-redis v11 with new features(Using an existing Redis instance, provide custom loggerContext and loggerTimestamp, some hooks used for Transforming Arguments & Replies, etc..), and new InjectRedis & InjectCluster decorators.

@iamkanguk97
Copy link

iamkanguk97 commented Dec 16, 2024

@R11baka Hello! I'm sorry for the late reply.

export interface RedisModuleAsyncOptions extends Pick<ModuleMetadata, 'imports'> {
    useFactory?: (...args: unknown[]) => RedisModuleOptions | Promise<RedisModuleOptions>;
    useClass?: Type<RedisOptionsFactory>;
    useExisting?: Type<RedisOptionsFactory>;
    inject?: InjectionToken[] | OptionalFactoryDependency[];
    extraProviders?: Provider[];
}
export interface RedisOptionsFactory {
    createRedisOptions: () => RedisModuleOptions | Promise<RedisModuleOptions>;
}

This is my useFactory code. Also, I'm using @liaoliaots/nestjs-redis 10 version and nest version is also 10.
I think you are not using version 10 of nestjs-redis. Was this answered?

For Reference, I temporarily solved it as follows:

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
      // @ts-expect-error
      useFactory: (configService: ConfigService): RedisModuleOptions => {
        const config = getAppConfig(configService);
        const { HOST, PORT, PASSWORD } = config.REDIS;

        return {
          config: {
            host: HOST,
            port: +PORT,
            password: PASSWORD,
          },
          readyLog: true,
          errorLog: true,
        };
      },

Hope you have a good day.
Thanks.

@liaoliaots
Copy link
Owner

liaoliaots commented Dec 17, 2024

This bug has been fixed(#554), please wait for the next major release: nestjs-redis v11 with new features(Using an existing Redis instance, provide custom loggerContext and loggerTimestamp, some hooks used for Transforming Arguments & Replies, etc..), and new InjectRedis & InjectCluster decorators.

And support node-redis v4 (redis & cluster), update redis-health lib.
And write tests to ensure the robustness of the lib.
At that time, version 10 will be marked as deprecated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants