Skip to content

Replace Vercel KV adapter with Vercel Runtime Cache API #2475

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

Merged
bookernath merged 1 commit into from
Aug 23, 2025
Merged

Replace Vercel KV adapter with Vercel Runtime Cache API #2475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions .changeset/chatty-forks-sniff.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@bigcommerce/catalyst-core": minor
---

Implement Vercel Runtime Cache API as replacement for Vercel KV adapter
101 changes: 0 additions & 101 deletions core/lib/kv/adapters/bc.ts
View file
Open in desktop

This file was deleted.

11 changes: 1 addition & 10 deletions core/lib/kv/adapters/upstash.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,14 @@ import { Redis } from '@upstash/redis';

import { KvAdapter, SetCommandOptions } from '../types';

import { MemoryKvAdapter } from './memory';

const memoryKv = new MemoryKvAdapter();

@migueloller migueloller Aug 28, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How come we removed the memory adapter from the Upstash adapter? This seems unrelated to adding Runtime Cache.

Also, should we have deleted the ./memory.ts file as well?

@bookernath bookernath Aug 28, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Memory KV is now handled centrally outside of the individual adapters in the kv/index.ts.


export class UpstashKvAdapter implements KvAdapter {
private upstashKv = Redis.fromEnv();
private memoryKv = memoryKv;

async mget<Data>(...keys: string[]) {
const memoryValues = await this.memoryKv.mget<Data>(...keys);

return memoryValues.length ? memoryValues : this.upstashKv.mget<Data[]>(keys);
return this.upstashKv.mget<Data[]>(keys);

Copilot AI Jul 10, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] Removing the in-memory fallback means every mget hits the network. Consider documenting this change or reintroducing an optional memory cache for hot keys to avoid potential latency spikes.

Copilot uses AI. Check for mistakes.
}

async set<Data>(key: string, value: Data, opts?: SetCommandOptions) {
await this.memoryKv.set(key, value, opts);

const response = await this.upstashKv.set(key, value, opts);

if (response === 'OK') {
Expand Down
78 changes: 78 additions & 0 deletions core/lib/kv/adapters/vercel-runtime-cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import { getCache } from '@vercel/functions';

import { KvAdapter, SetCommandOptions } from '../types';

export class RuntimeCacheAdapter implements KvAdapter {
private cache = getCache();

async mget<Data>(...keys: string[]): Promise<Array<Data | null>> {
this.logger(
`MGET - Keys: ${keys.toString()} - Source: RUNTIME_CACHE - Fetching ${keys.length} keys`,
);

try {
const values = await Promise.all(
keys.map(async (key) => {
try {
// Use the Runtime Cache API
const cachedValue = await this.cache.get(key);

if (cachedValue !== null) {
this.logger(`RUNTIME_CACHE GET - Key: ${key} - Found: true`);

return cachedValue;
}

this.logger(`RUNTIME_CACHE GET - Key: ${key} - Found: false`);

return null;
} catch (error) {
this.logger(
`RUNTIME_CACHE GET ERROR - Key: ${key} - Error: ${error instanceof Error ? error.message : 'Unknown error'}`,
);

return null;
}
}),
);

// eslint-disable-next-line @typescript-eslint/consistent-type-assertions
return values as Array<Data | null>;
Comment on lines +39 to +40

Copilot AI Aug 23, 2025

Copy link

Choose a reason for hiding this comment

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

The type assertion as Array<Data | null> bypasses TypeScript's type checking. Consider using a type guard or proper typing to ensure type safety instead of disabling the linting rule.

Suggested change
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions
return values as Array<Data | null>;
// Return values directly, as they are Array<Data | null>
return values;

Copilot uses AI. Check for mistakes.
} catch (error) {
this.logger(
`RUNTIME_CACHE ACCESS ERROR - Returning null values: ${error instanceof Error ? error.message : 'Unknown error'}`,
);

// Return null for all keys if Runtime Cache is unavailable
return keys.map(() => null);

@migueloller migueloller Aug 28, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the idea here that returning null behaves as if the cache entries don't exist?

@bookernath bookernath Aug 28, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, I want the application to treat this as a cache miss and gracefully continue to load the page even if the cache is degraded. Memory KV will still provide some offload in most scenarios.

}
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
async set<Data>(key: string, value: Data, _opts?: SetCommandOptions): Promise<Data | null> {
this.logger(`SET - Key: ${key} - Setting in runtime cache`);

try {
await this.cache.set(key, value);
this.logger(`RUNTIME_CACHE SET - Key: ${key} - Success`);
} catch (error) {
this.logger(
`RUNTIME_CACHE SET ERROR - Key: ${key} - Error: ${error instanceof Error ? error.message : 'Unknown error'}`,
);
}

return value;
}

private logger(message: string) {
// Check if logging is enabled using the same logic as the main KV class
const loggingEnabled =
(process.env.NODE_ENV !== 'production' && process.env.KV_LOGGER !== 'false') ||
process.env.KV_LOGGER === 'true';

if (loggingEnabled) {
// eslint-disable-next-line no-console
console.log(`[BigCommerce] Runtime Cache ${message}`);
}
}
}
30 changes: 0 additions & 30 deletions core/lib/kv/adapters/vercel.ts
View file
Open in desktop

This file was deleted.

13 changes: 4 additions & 9 deletions core/lib/kv/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,11 @@ class KV<Adapter extends KvAdapter> implements KvAdapter {
}

async function createKVAdapter() {

Copilot AI Jul 10, 2025

Copy link

Choose a reason for hiding this comment

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

The createKVAdapter function only handles Vercel and Upstash cases and doesn’t throw or return a default adapter if no env vars match. Consider adding a clear fallback or throwing an explicit error to avoid returning undefined.

Copilot uses AI. Check for mistakes.
if (process.env.BC_KV_REST_API_URL && process.env.BC_KV_REST_API_TOKEN) {
const { BcKvAdapter } = await import('./adapters/bc');
// Prioritize Runtime Cache for Vercel environments
if (process.env.VERCEL === '1') {
const { RuntimeCacheAdapter } = await import('./adapters/vercel-runtime-cache');

return new BcKvAdapter();
}

if (process.env.KV_REST_API_URL && process.env.KV_REST_API_TOKEN) {
const { VercelKvAdapter } = await import('./adapters/vercel');

return new VercelKvAdapter();
return new RuntimeCacheAdapter();
}

if (process.env.UPSTASH_REDIS_REST_URL && process.env.UPSTASH_REDIS_REST_TOKEN) {
Expand Down
4 changes: 2 additions & 2 deletions core/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"@t3-oss/env-core": "^0.13.6",
"@upstash/redis": "^1.35.0",
"@vercel/analytics": "^1.5.0",
"@vercel/kv": "^3.0.0",
"@vercel/functions": "^2.2.12",
"@vercel/speed-insights": "^1.2.0",
"clsx": "^2.1.1",
"content-security-policy-builder": "^2.3.0",
Expand All @@ -50,7 +50,7 @@
"lodash.debounce": "^4.0.8",
"lru-cache": "^11.1.0",
"lucide-react": "^0.474.0",
"next": "15.4.2-canary.10",
"next": "15.5.1-canary.4",

@migueloller migueloller Aug 28, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Was upgrading Next.js required to leverage the new Runtime Cache?

@bookernath bookernath Aug 28, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, this was initially desirable when I thought we would move to Node.js middleware as part of this change, so I wanted to be on a version where that feature was considered "stable". Upon review I probably should have decoupled the version bump from this PR once it was clear it was not necessary.

@migueloller migueloller Aug 28, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, I would suggested we do that if we hadn't merged already. Thanks for the explanation 🙏🏻

"next-auth": "5.0.0-beta.25",
"next-intl": "^4.1.0",
"nuqs": "^2.4.3",
Expand Down
Loading
Loading