Moleculer: Redis Cacher with TypeScript

[slinkardbrandon]

Expected Behavior

Ioredis should not be a required dependency of developers who choose to use Typescript with their implementation of moleculer.

Current Behavior

A change was made fairly recently that has started failing our builds in our CI. The index.d.ts file is importing Ioredis and attempting to use it as a dependency even though ioredis is marked as a dev dependency in the package.json. I've attached a screenshot of the error that I'm receiving below.

Steps to Reproduce

Please provide detailed steps for reproducing the issue.

1. Use typescript with moleculer
2. Do not install ioredis
3. run tsc

Context

• Moleculer version: 0.13.6
• NodeJS version: 10.14.1
• Operating System: MacOS

Doesn't implementing ioredis as a direct dependency of moleculer break modularity? I don't need redis in one of my projects and making the assumption that everyone is leads to unnecessary bloat. Any thoughts or suggestions would be nice, thanks!

[icebob]

It came with https://github.com/moleculerjs/moleculer/pull/459
@shawnmcknight could you solve it?

[shawnmcknight]

You definitely don't need the full ioredis module, but since broker.cacher can definitely be of type ioredis, the type definition needs to be accessible, if defining the type is desired.

I'm not totally sure what best practices would be in a situation like this since generally speaking, the dependency would be required and thus including the type definition as a dependency would be perfectly reasonable. However, in this case, as @slinkardbrandon mentions, ioredis is optional and thus it could be argued that the type is optional too, but #459 basically makes the type not-optional. #474 would solve the issue temporarily by ensuring that the type is always available, but I'm uncertain if that is the best practice.

Just brainstorming here. Here's the ideas I have as to how it could be solved:

1. Do what #474 is doing and accept that there will be additional @types installed which might be unneeded.
2. Reverse #459 and just define client as a property on cacher which is a GenericObject. You wouldn't get any types out of ioredis, but maybe that's satisfactory.
3. Adjust ServiceBroker to accept a type variable denoting the type of the cacher and default it to GenericObject. Consumers wishing to get type definitions of ioredis could then provide that as the type to their broker instance. I'm thinking every other type definition which composes ServiceBroker would need this as well. (e.g. Context, Service, etc.).

I don't have a ton of TypeScript experience, particularly for this use-case where a module is optional, so I'm sure there's another solution out there I'm not thinking of. I can pitch in to help with a solve with whatever direction make sense though.

[keawade]

I've opened #476 with a potential fix.

The PR adds client as an optional property typed to any on the cacher for visibility. This will require TS users to check for the property or cast the type to Redis.Redis themselves (recommended implementation).

[icebob]

Published: https://github.com/moleculerjs/moleculer/releases/tag/v0.13.7