feat: add support for redis client

[Antignote]

Adds support for existing Redis client to ISR cache handler

Description

This PR enhances the RedisCacheHandler to accept an existing Redis client instance, providing more flexibility for applications that need to share a Redis connection or have custom Redis configurations.

Changes

• Updated RedisCacheHandlerOptions.redis to accept:

  • An existing Redis client instance (ioredis)
  • A URL string (existing)
  • A RedisOptions object (existing)

• Improved connection lifecycle management: When an existing client is passed, the handler skips adding error listeners to avoid conflicts with the application's own error handling

• Enhanced documentation with examples showing both usage patterns:

  • Creating a new client from URL/config
  • Passing an existing client instance

Benefits

• Connection reuse: Share a single Redis connection across multiple parts of your application
• Custom configuration: Use advanced Redis features (Cluster, Sentinel, etc.) by passing a pre-configured client
• Better control: Manage Redis connection lifecycle independently from the cache handler

Example Usage

import Redis from "ioredis";
import { RedisCacheHandler } from "@mrjasonroy/cache-components-cache-handler/handlers/redis";

const redisClient = new Redis({ host: "localhost", port: 6379 });

new RedisCacheHandler({ redis: redisClient });

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Chore (dependencies, CI, etc.)

Testing

• All existing tests pass (pnpm test)
• Added new tests for new functionality
• Tested with memory cache handler
• Tested with Redis cache handler (if applicable)
• E2E tests pass (pnpm test:e2e)

Checklist

• My code follows the project's code style (pnpm lint passes)
• I have run pnpm format to format my code
• Type checking passes (pnpm typecheck)
• I have updated the documentation accordingly
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

Related Issues

N/A

Additional Context

[gemini-code-assist]

Summary of Changes

Hello @Antignote, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the RedisCacheHandler by introducing the capability to accept an already instantiated ioredis client. This change provides greater flexibility for applications, enabling connection reuse across different parts of an application and supporting advanced Redis configurations like Cluster or Sentinel setups. It also improves control over the Redis connection's lifecycle, allowing applications to manage their Redis instances independently while still leveraging the cache handler's functionality.

Highlights

• Redis Client Flexibility: The RedisCacheHandler now supports passing an existing ioredis client instance directly, alongside existing options for URL strings or RedisOptions objects, providing more control over connection management.
• Connection Lifecycle Management: When an existing Redis client is provided, the handler intelligently skips attaching its own error listeners to prevent conflicts with the application's error handling, ensuring cleaner integration.
• Enhanced Documentation: Updated documentation includes new examples demonstrating how to initialize the RedisCacheHandler with both a new Redis client (via URL/config) and an existing client instance for clarity and ease of use.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a valuable feature by allowing an existing Redis client to be passed to the RedisCacheHandler. This enhances flexibility and promotes connection reuse. The implementation is clean, and the conditional attachment of the error listener is a good practice. The documentation updates are also clear and helpful.

However, I've identified a significant issue with the connection lifecycle management. The close() method will unconditionally close the Redis connection, even if it's a shared client provided externally. This could lead to unexpected disconnections in other parts of an application. My review includes a detailed comment on how to address this by tracking whether the client was created internally.

[Antignote]

@mrjasonroy did you have any chance to have a look at this?

[mrjasonroy]

Review: feat/add support for redis client

Good feature direction — passing an existing client is a real need for connection sharing, Sentinel/Cluster setups, and test mocking. The implementation is close but has two critical bugs and no tests.

Verdict: Request Changes

Critical

• instanceof Redis is fragile and will silently fail across package versions/bundlers (see inline)
• The two instanceof checks at lines 108 and 127 can disagree, creating a worst-case state where a new phantom connection is opened AND no error listener is attached

Blocking

• Zero tests — no handlers/redis.test.ts exists at all. This PR adds lifecycle-sensitive behavior (conditional quit(), conditional error listener) with no coverage.

Questions for the author

1. Why instanceof Redis instead of a duck-type check or the RedisClient interface pattern used in data-cache/redis.ts? Deliberate divergence?
2. Is there a plan to add tests before merge?
3. Should close() be added to the CacheHandler interface? The behavior now differs depending on client ownership — composability matters.

[mrjasonroy]

🔴 Critical: instanceof Redis is fragile and will silently break across package versions.

instanceof fails when the consumer's ioredis version differs from the one bundled here, or when bundlers (Webpack, esbuild) duplicate the module. When it fails, the code falls through to the else branch — calling new Redis(options.redis) with your client object as config. ioredis silently ignores unknown props and opens a new default connection to localhost:6379. No error thrown. You'll just be talking to the wrong Redis.

The sibling data-cache/redis.ts already solves this with a RedisClient interface (duck-typing). Recommend the same approach here:

function isRedisClient(value: unknown): value is Redis {
  return (
    typeof value === 'object' &&
    value !== null &&
    typeof (value as Redis).get === 'function' &&
    typeof (value as Redis).set === 'function' &&
    typeof (value as Redis).quit === 'function'
  );
}

[mrjasonroy]

🔴 This check can disagree with line 108, creating a worst-case scenario.

If instanceof is unreliable (see line 108 comment), both checks can fail simultaneously — meaning: the code creates a new client (line 108 falls through to else) AND skips the error listener here. That's a phantom connection with unhandled errors.

Use this.didCreateClient instead — it's already tracking this and is guaranteed to agree with the actual branching logic:

if (this.didCreateClient) {
  this.redis.on('error', (err) => { ... });
}

This is simpler, correct, and consistent with how close() at line 315 already does it.

[mrjasonroy]

🟡 Nit: should be readonly.

This flag is set once in the constructor and never mutated after. Mark it readonly for clarity and to let the type system enforce that:

private readonly didCreateClient: boolean;

Then assign it explicitly in each constructor branch (e.g. this.didCreateClient = true / false).

[mrjasonroy]

🟡 Foot-gun: if the consumer also forgot to add an error listener, Node.js will crash.

Multiple 'error' listeners on the same EventEmitter are fine — they're called in order. A safer approach: always attach one, but check first so we don't add a duplicate logger:

if (this.redis.listenerCount('error') === 0) {
  this.redis.on('error', (err) => {
    console.error('[RedisCacheHandler] Redis connection error:', err);
  });
}

At minimum, the JSDoc for the shared-client option should document that the caller is responsible for error handling.

[mrjasonroy]

Thanks for this contribution, @Antignote! Accepting an existing Redis client instance is a great feature for connection reuse and advanced configs (Cluster, Sentinel, etc.). The didCreateClient lifecycle pattern is well done.

A few things to address before we can merge:

1. instanceof Redis fragility across package boundaries

If the user's app has a different ioredis version than this package's dependency, instanceof will return false (different constructors from different node_modules copies). A duck-typing check would be more robust:

function isRedisInstance(value: unknown): value is Redis {
  return (
    typeof value === "object" &&
    value !== null &&
    typeof (value as Redis).get === "function" &&
    typeof (value as Redis).set === "function" &&
    typeof (value as Redis).del === "function"
  );
}

Or at minimum, document in the JSDoc that the ioredis version must match.

2. Inconsistent condition for error listener

The error listener guard (line ~125) duplicates the instanceof check:

if (!(options.redis instanceof Redis)) {
  this.redis.on("error", ...);
}

This should use the didCreateClient flag you already have, for consistency and to avoid the instanceof issue:

if (this.didCreateClient) {
  this.redis.on("error", (err) => { ... });
}

3. No tests

Would like to see at least:

• Passing an existing client instance and verifying get/set works
• close() does not call quit() on a shared client
• close() does call quit() on an internally-created client

4. Needs rebase

This branch is from Jan 14 and main has had 14+ commits since. Will need a rebase to merge cleanly.

---

Overall the approach is solid — just needs these cleanups. Let us know if you'd like help with any of them!

[mrjasonroy]

Rebased onto main and pushed a fix commit addressing the review feedback:

1. Duck typing instead of instanceof Redis - instanceof can break across package versions or bundler setups. Now checks for get/set/del methods instead.
2. Consistent didCreateClient usage - Error listener condition now uses this.didCreateClient instead of a second instanceof check.
3. Added 5 tests - Covers existing client detection, error listener behavior, and close() lifecycle management.

All 95 tests pass. Ready to merge once CI is green.