Skip to content

Merge pull requests 33 and 28 from mrjasonroy's repo. #34

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.

Sign up for GitHub

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

Closed
jellylamp wants to merge 4 commits into from
Closed

Merge pull requests 33 and 28 from mrjasonroy's repo. #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
883c280
fixed ttl inconsistency between usage of ioredis and node-redis clients
SerMedvid Feb 22, 2026
ad841ce
fix: update factory adapter to translate node-redis style TTL to ioredis
mrjasonroy Feb 24, 2026
e070140
docs: Add setup:e2e script for installing playwright chromium and upd…
bholly24 Mar 9, 2026
d6da04f
Merge branch 'pr-33' into mergePRs
Jun 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ jobs:
run: pnpm build

- name: Install Playwright browsers
run: pnpm --filter e2e-test-app exec playwright install --with-deps chromium
run: pnpm setup:e2e

- name: Build e2e app
env:
Expand Down Expand Up @@ -169,7 +169,7 @@ jobs:
run: pnpm build

- name: Install Playwright browsers
run: pnpm --filter e2e-test-app exec playwright install --with-deps chromium
run: pnpm setup:e2e

- name: Build e2e app with ElastiCache handler
env:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/nextjs-canary-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ jobs:

- name: Install Playwright browsers
if: steps.build.outcome == 'success'
run: pnpm --filter e2e-test-app exec playwright install --with-deps chromium
run: pnpm setup:e2e

- name: Build e2e app
id: build_e2e
Expand Down
14 changes: 9 additions & 5 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,14 @@ Thank you for your interest in contributing! This document provides guidelines a
```bash
pnpm install
```
4. **Create a branch** for your work:
```bash
git checkout -b feature/my-new-feature
```
4. **Setup e2e testing**:
```bash
pnpm setup:e2e
```
5. **Create a branch** for your work:
```bash
git checkout -b feature/my-new-feature
```
Comment on lines +23 to +30

@gemini-code-assist gemini-code-assist Bot Jun 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The markdown code blocks for steps 4 and 5 are not indented, which breaks the list formatting in many markdown renderers. Indenting the code blocks by 3 spaces keeps them nested within their respective list items:

4. **Setup e2e testing**:
   ```bash
   pnpm setup:e2e
  1. Create a branch for your work: 
    git checkout -b feature/my-new-feature


## Development Workflow

Expand Down Expand Up @@ -55,7 +59,7 @@ pnpm test
pnpm test:e2e

# Or run everything at once
pnpm lint && pnpm typecheck && pnpm test
pnpm lint && pnpm typecheck && pnpm test && pnpm test:e2e
```

### Commit Messages
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
"docker:up": "docker compose up -d",
"docker:down": "docker compose down",
"docker:logs": "docker compose logs -f redis",
"clean": "turbo clean && rm -rf node_modules"
"clean": "turbo clean && rm -rf node_modules",
"setup:e2e": "pnpm --filter e2e-test-app exec playwright install --with-deps chromium"
},
"devDependencies": {
"@biomejs/biome": "^1.9.4",
Expand Down
11 changes: 7 additions & 4 deletions packages/cache-handler/src/data-cache/factory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,17 @@ function loadIoredis(type: string): typeof import("ioredis").default {
}

/**
* Create adapter for ioredis (lowercase methods) to match RedisClient interface (camelCase)
* Create adapter for ioredis (lowercase methods) to match RedisClient interface (camelCase).
* Translates node-redis-style SET options `{ EX: seconds }` to ioredis positional args.
*/
function createRedisAdapter(redis: import("ioredis").default): RedisClient {
return {
get: (key) => redis.get(key),
set: (key, value, exFlag?, ttl?) => {
if (exFlag === "EX" && typeof ttl === "number") {
return redis.set(key, value, "EX", ttl) as Promise<unknown>;
set: (key, value, ...args) => {
// node-redis style: set(key, value, { EX: seconds })
const opts = args[0] as Record<string, unknown> | undefined;
if (opts && typeof opts === "object" && typeof opts.EX === "number") {
return redis.set(key, value, "EX", opts.EX) as Promise<unknown>;
}
return redis.set(key, value) as Promise<unknown>;
},
Expand Down
17 changes: 9 additions & 8 deletions packages/cache-handler/src/data-cache/redis.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ class FakeRedis {
this.setCalls.push({ key, args });

let expireAt: number | undefined;
// ioredis style: set(key, value, "EX", seconds)
if (args[0] === "EX" && typeof args[1] === "number") {
expireAt = Date.now() + args[1] * 1000;
// node-redis style: set(key, value, { EX: seconds })
const opts = args[0] as { EX?: number } | undefined;
if (opts && typeof opts === "object" && typeof opts.EX === "number") {
expireAt = Date.now() + opts.EX * 1000;
}

this.store.set(key, { value, expireAt });
Expand Down Expand Up @@ -147,7 +148,7 @@ describe("RedisDataCacheHandler", () => {
expect(redis.setCalls).toHaveLength(1);
expect(redis.setCalls[0]).toMatchObject({
key: "nextjs:data-cache:cache-key",
args: ["EX", 120],
args: [{ EX: 120 }],
});

const result = await handler.get("cache-key", []);
Expand Down Expand Up @@ -231,7 +232,7 @@ describe("RedisDataCacheHandler", () => {
expect(redis.delCalls).toContainEqual(["nextjs:data-cache:invalidate-key"]);
});

test("sets TTL correctly with ioredis style args (fixes #16)", async () => {
test("sets TTL correctly with node-redis style options (fixes #16)", async () => {
vi.useFakeTimers();
vi.setSystemTime(BASE_TIME);

Expand All @@ -241,11 +242,11 @@ describe("RedisDataCacheHandler", () => {
const entry = createEntry("ttl-test", { expire: 60, revalidate: 30 });
await handler.set("ttl-key", Promise.resolve(entry));

// Verify the set call used ioredis style: "EX", seconds
// Verify the set call used node-redis style: { EX: seconds }
expect(redis.setCalls).toHaveLength(1);
const setCall = redis.setCalls[0];
expect(setCall.key).toBe("nextjs:data-cache:ttl-key");
expect(setCall.args).toEqual(["EX", 60]);
expect(setCall.args).toEqual([{ EX: 60 }]);
});

test("TTL causes entry to expire after specified time", async () => {
Expand Down Expand Up @@ -286,6 +287,6 @@ describe("RedisDataCacheHandler", () => {
await handler.set("default-ttl-key", Promise.resolve(entry));

expect(redis.setCalls).toHaveLength(1);
expect(redis.setCalls[0].args).toEqual(["EX", 3600]);
expect(redis.setCalls[0].args).toEqual([{ EX: 3600 }]);
});
});
8 changes: 4 additions & 4 deletions packages/cache-handler/src/data-cache/redis.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type { DataCacheEntry, DataCacheHandler } from "./types.js";

export interface RedisDataCacheHandlerOptions {
/**
* Redis client instance (ioredis compatible)
* Redis client instance (node-redis)
*/
redis: RedisClient;

Expand Down Expand Up @@ -41,8 +41,8 @@ export interface RedisDataCacheHandlerOptions {
}

/**
* Redis client interface (ioredis compatible)
* Uses ioredis-style SET with "EX" positional args: set(key, value, "EX", seconds)
* Redis client interface (node-redis compatible)
* Uses node-redis-style SET with options object: set(key, value, { EX: seconds })
*/
export interface RedisClient {
get(key: string): Promise<string | null>;
Expand Down Expand Up @@ -275,7 +275,7 @@ export function createRedisDataCacheHandler(
const ttl = entry.expire < 4294967294 ? entry.expire : defaultTTL;

// Store in Redis with TTL
await redis.set(key, JSON.stringify(serialized), "EX", Math.ceil(ttl));
await redis.set(key, JSON.stringify(serialized), { EX: Math.ceil(ttl) });
Comment on lines 277 to +278

@gemini-code-assist gemini-code-assist Bot Jun 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If ttl is calculated to be 0 or negative (for example, if entry.expire is 0 or negative), passing it to Redis EX option will result in a Redis error (ERR invalid expire time in set). Adding a defensive check to skip caching when ttl <= 0 prevents unnecessary Redis errors.

        if (ttl <= 0) {
          log?.("set", cacheKey, "skipped due to non-positive TTL", { ttl });
          return;
        }

        // Store in Redis with TTL
        await redis.set(key, JSON.stringify(serialized), { EX: Math.ceil(ttl) });


log?.("set", cacheKey, "done", { ttl });
} catch (error) {
Expand Down