[james-elicx]

What

When deploying an ISR/cached app, the deploy command requires a cache adapter to be configured. Previously this check only inspected the Vite config (vinext({ cache })), so apps that wired a cache backend imperatively in their Worker entry were incorrectly blocked.

This PR:

1. Adds backwards-compat detection for Worker-entry cache setters. New exported workerEntryHasCacheHandler(root) scans an existing user-authored Worker entry (worker/index.ts / worker/index.js) for setCacheHandler / setDataCacheHandler / setCdnCacheAdapter calls. The ISR deploy gate now passes when either the Vite config or the Worker entry configures a backend. It mirrors viteConfigHasCacheAdapter's leniency (unreadable entry treated as configured; missing entry returns false).

2. Removes the CDN part from formatMissingCacheAdapterError. The error message no longer suggests the cdnAdapter() import/option and points only at the kvDataAdapter() data cache.

Testing

• Added a workerEntryHasCacheHandler test block covering all three setters, both .ts/.js entries, the no-setter case, and the missing-entry case.
• Added an assertion that the error message no longer references the CDN adapter.
• vp check and the targeted tests/deploy.test.ts cases pass.

[james-elicx]

/bigbonk review

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@1821

npm i https://pkg.pr.new/vinext@1821

commit: 83f8a42

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original
workers-cache	preview	production
web	preview	production

[ask-bonk]

Review: feat(deploy): honor Worker-entry cache setters for ISR deploys

Verified the change builds, lints, type-checks, and the new tests pass (vp check + targeted tests/deploy.test.ts runs green).

Summary

This is a clean, well-scoped backwards-compat fix. The ISR deploy gate now passes when either the Vite config or the Worker entry configures a cache backend, which correctly unblocks older apps that wired setCacheHandler / setDataCacheHandler / setCdnCacheAdapter imperatively in worker/index.{ts,js}. This is logically consistent with #1800 (which deprecated those setters but kept them functional).

What's good

• Candidate paths match repo conventions. worker/index.ts / worker/index.js mirrors hasWorkerEntry (deploy.ts:196-198) and the auto-generated entry path. Worker entries are never .mjs anywhere in the codebase, so omitting .mjs (unlike viteConfigHasCacheAdapter, which checks it for vite configs) is correct.
• Leniency model is intentional and documented. Unreadable entry → true (don't block); missing entry → false (defer to other checks). Because the gate ANDs both checks (!viteConfigHasCacheAdapter(root) && !workerEntryHasCacheHandler(root)), the missing/false asymmetry vs viteConfigHasCacheAdapter is sound — viteConfigHasCacheAdapter already returns true for a missing config, so a missing worker entry returning false doesn't accidentally block anything.
• No dev/prod parity concern. This cache-adapter gate exists only in deploy.ts; there's no duplicate logic in the prod/dev/CLI servers that needs the same fix.
• Tests are thorough and isolated. All three setters, both .ts/.js, no-setter, and missing-entry cases are covered. tmpDir is recreated per test via beforeEach, so the "no Worker entry" case is genuinely clean.

Minor notes (non-blocking)

1. Regex matches commented-out / string-literal calls. \b(?:setCacheHandler|...)\s*\( is a plain text scan, so a commented-out // setCacheHandler(...) would count as "configured" and let a deploy through. Given the explicitly-documented favor-false-positive-over-false-negative model (matching viteConfigHasCacheAdapter), this is an acceptable tradeoff — just flagging it as a known heuristic limitation.

2. CDN removal from the error message. cdnAdapter() / cache.cdn is still a fully-supported, documented option (index.ts:728-733, shims/cdn-cache.ts). Dropping it from formatMissingCacheAdapterError is a reasonable UX simplification (point users at the KV data cache that the auto-scaffolded VINEXT_KV_CACHE binding backs), but confirm this is a deliberate "data-cache-first" messaging choice rather than an implication that cdn is going away. The test now asserts the message no longer references the CDN adapter, which locks in that intent.

Verdict

Approve. Correct, minimal, well-tested backwards-compat fix that matches the deprecation direction of #1800. The two notes above are informational only.

[ask-bonk]

Review posted to PR #1821.

Summary

This PR is a clean, well-scoped backwards-compat fix. I verified it builds, lints, type-checks, and the new tests pass.

Verdict: Approve.

Key findings:

• Correctness verified. The new workerEntryHasCacheHandler candidate paths (worker/index.{ts,js}) match repo conventions exactly; worker entries are never .mjs, so omitting it is correct.
• Leniency model is sound. The intentional missing-entry→false vs unreadable→true asymmetry works correctly because the deploy gate ANDs both cache checks, and viteConfigHasCacheAdapter already returns true for missing configs.
• No dev/prod parity gap — this gate lives only in deploy.ts.
• Tests are thorough and isolated (all three setters, both extensions, no-setter, missing-entry; tmpDir reset per test).

Two non-blocking notes raised:

1. The regex is a plain text scan that would match commented-out calls — acceptable given the documented favor-false-positive model.
2. The CDN reference was removed from the error message even though cdnAdapter()/cache.cdn is still supported — flagged to confirm it's a deliberate data-cache-first messaging choice.

github run