fix(use-cache): support nested cache functions passed as props

[james-elicx]

Failing test: test/e2e/app-dir/use-cache-with-server-function-props — "should be able to use nested cache functions as props".

Change in packages/vinext/src/index.ts.

Recovered from a cut-off worker; CI + bonk to verify. (No unit test added by the worker — may add per bonk.)

[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

[james-elicx]

/bigbonk review for issues

[ask-bonk]

@james-elicx Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

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

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

commit: 9f4b6ee

[ask-bonk]

Review: fix(use-cache): support nested cache functions passed as props

I dug into the @vitejs/plugin-rsc runtime to verify whether this approach can actually round-trip a server reference. It can't in production builds, and likely not in dev either. The fix targets the right problem but uses the wrong reference id, and never registers the module in the manifest the plugin resolves from. The linked Next.js test (/nested-cache) clicks a form button, which is a full client→server action round-trip — so resolution must work, not just serialization.

Blocking: the id passed to registerServerReference is not a resolvable reference key

The PR emits:

registerServerReference(cachedFnExpr, id, name)

where id is the raw module id Vite hands to the transform hook (an absolute file path, possibly with a query). But the plugin's own "use server" transform does not use the raw id — it uses a normalized reference key:

// node_modules/@vitejs/plugin-rsc/dist/plugin-*.js
runtime: (value, name) =>
  `$$ReactServer.registerServerReference(${value}, ${JSON.stringify(getNormalizedId())}, ${JSON.stringify(name)})`
// getNormalizedId():
//   build: hashString(manager.toRelativeId(id))
//   dev:   normalizeViteImportAnalysisUrl(server.environments[server], id)

When the client submits the form action, the id is posted back and resolved via loadServerAction(id):

// dist/core/rsc.js
async function loadServerAction(id) {
  const [file, name] = id.split("#");
  return (await requireModule(file))[name];
}

In a production build, requireModule/setRequireModule.load resolves through the generated virtual:vite-rsc/server-references manifest, which is keyed only by meta.referenceKey (the normalized/hashed id):

// dist/rsc.js
const import_ = serverReferences[id];
if (!import_) throw new Error(`server reference not found '${id}'`);

The raw path this PR passes will not be a key in that manifest, so clicking the button in production throws server reference not found '<id>'. vinext already maps exactly that error to a Next.js 404 (server/server-action-not-found.ts:77), so the test's expect(...).toMatch(isoDateRegExp) would fail.

Blocking: the module is never added to serverReferenceMetaMap

Even if the id were correct, the build-time server-references manifest is populated exclusively by the plugin's own "use server" transform writing manager.serverReferenceMetaMap[id]:

// dist/plugin-*.js — createVirtualPlugin("vite-rsc/server-references")
for (const meta of Object.values(manager.serverReferenceMetaMap)) {
  const key = JSON.stringify(meta.referenceKey);
  const id  = JSON.stringify(meta.importId);
  ...
}

vinext calling registerServerReference at runtime attaches $$typeof/$$id metadata to the wrapper, but it does not add an entry to serverReferenceMetaMap. So there is no manifest entry to import from in production — independent of the id-format issue above. Both must be solved for the round-trip to work in a build.

Dev is also suspect, not just prod

In dev the reference key is normalizeViteImportAnalysisUrl(...), not the raw transform id. So the registered id and the id the consumer resolves are different shapes, meaning the round-trip likely fails in dev too. If the preview deploy appears to "work," please verify the actual /nested-cache button click renders a date/random rather than 404 — serialization succeeding is not the same as the action resolving.

Direction worth investigating

The robust fix is to make these inline cache functions go through the plugin's own server-reference machinery (so they land in serverReferenceMetaMap with the correct referenceKey), rather than calling registerServerReference manually with a hand-rolled id. Two avenues:

• Let @vitejs/plugin-rsc's transformServerActionServer (or its useServer transform) also process these hoisted cached functions so the manifest entry + normalized key are produced by the plugin, then layer the cache wrapper on top.
• If vinext must register manually, it needs to (a) compute the same normalized reference key the plugin uses for both dev and build, and (b) ensure the module is included in the server-references manifest. That's effectively reimplementing the plugin's bookkeeping and is fragile.

Process gaps

• No test added. This is required per AGENTS.md ("Add tests first"), and the linked Next.js test is a behavioral round-trip that a unit test alone can't fully cover. Please port the /nested-cache fixture (page + client form) into tests/fixtures/app-basic and add an E2E or integration test that asserts the date/random actually render after the click — not just that the page serializes. Without it, a green preview can mask a prod-only 404.
• Branch is behind main. It's missing several merged commits; please rebase so CI exercises the change against current main.

Minor

• index.ts:4227-4232 — the IIFE that resolves @vitejs/plugin-rsc/react/rsc and the isRscEnv gating are fine, but if the manual approach is kept, hoist the resolution so it isn't recomputed per matched module, and confirm the resolved file URL matches what cache-runtime.ts already imports (@vitejs/plugin-rsc/react/rsc) to avoid loading two copies in separate module instances.

Summary

The serialization intent is correct, but as written the cached function cannot be resolved back on submission: the reference id isn't the plugin's normalized key, and the module is never registered in the build manifest. This will surface as a production 404 on form submit (the exact path vinext maps via server-action-not-found.ts). Recommend routing through the plugin's server-reference machinery and adding a round-trip test before merge.

[ask-bonk]

Review posted to PR #1871.

Summary

I reviewed the index.ts change that registers inline "use cache" functions as server references in the RSC environment. By tracing the @vitejs/plugin-rsc runtime, I found two blocking correctness issues:

1. Wrong reference id. The PR passes the raw transform id (absolute file path) to registerServerReference. The plugin resolves server references by a normalized key (hashString(toRelativeId(id)) in build, normalizeViteImportAnalysisUrl(...) in dev). On form submit, loadServerAction(id) won't find the raw path → server reference not found '<id>'.

2. Module never added to the manifest. The build-time virtual:vite-rsc/server-references manifest is populated exclusively from manager.serverReferenceMetaMap, which only the plugin's own "use server" transform writes. Manually calling registerServerReference attaches $$typeof/$$id metadata but doesn't create a manifest entry, so production builds can't import the action.

The linked Next.js test (/nested-cache) does a real client→server round-trip (button click → form action), so resolution must work, not just serialization. In production this would surface as a 404 — the exact error vinext already maps in server/server-action-not-found.ts:77. Dev is also suspect since the registered id shape differs from what the consumer resolves.

I also flagged process gaps: no test was added (required by AGENTS.md, and the behavior needs a round-trip test), and the branch is behind main and should be rebased. I recommended routing these cached functions through the plugin's own server-reference machinery rather than hand-rolling the id and registration.

github run

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(use-cache): support nested cache functions passed as props

This revision directly addresses both blockers from the previous review: it now derives the normalised reference key (instead of the raw absolute id) and registers the module in serverReferenceMetaMap so the build-time virtual:vite-rsc/server-references manifest includes it. I traced the @vitejs/plugin-rsc runtime again and the overall mechanics now line up:

• Dev: loadServerAction(id) validates against manager.serverReferenceMetaMap (plugin-BK29Va7z.js:616) then import(file). The PR registers the entry and uses a Vite-importable /...-prefixed key, so validation + import resolve.
• Build: virtual:vite-rsc/server-references is generated from serverReferenceMetaMap during the real RSC build, step [3/5] (plugin-BK29Va7z.js:1648-1665), and the PR writes into that same shared manager instance (correctly captured via the rsc:minimal .api, matching getPluginApi at plugin-BK29Va7z.js:568). transformHoistInlineDirective emits real export function $$hoist_N_… declarations (transforms/index.js:27), so exportNames: result.names are genuine exports the manifest can re-import.

The direction is right. However there are still correctness and process gaps that should be resolved before merge.

Blocking-ish: build-mode key formula diverges from the plugin's toRelativeId

The plugin computes the build key as hashString(manager.toRelativeId(id)), where (plugin-BK29Va7z.js:553-555):

toRelativeId(id) { return normalizePath(path.relative(this.config.root, id)); }

The PR re-implements this with a raw string slice (index.ts:4290-4296):

createHash("sha256")
  .update(id.replace(/\\/g, "/").slice(projectRoot.replace(/\\/g, "/").length + 1))
  .digest().toString("hex").slice(0, 12)

Two divergences:

1. path.relative vs slice. path.relative(root, id) resolves/normalizes both operands (collapses ./.., handles trailing slashes, drive-letter casing on Windows). A literal slice(root.length + 1) only matches when id is exactly root + "/" + rel with no normalization differences. For the happy path they agree; for any non-canonical id they don't, and a mismatch produces server reference not found '<key>' on submit.
2. Wrong root source. toRelativeId uses manager.config.root (the shared/root config — set at plugin-BK29Va7z.js:582). The PR uses this.environment?.config?.root ?? root (index.ts:4287-4288). These usually coincide, but the plugin is authoritative about which root it hashes against, so re-deriving from the environment root is a latent parity risk.

Since the PR already captures rscPluginApi.manager, the robust fix is to reuse the plugin's own helper rather than re-deriving:

const normalizedRefKey = isBuild
  ? hashString(rscPluginApi.manager.toRelativeId(id))   // identical to the plugin
  : /* dev key */;

That guarantees byte-for-byte parity with whatever the plugin would have produced, and removes the fragile slice.

Blocking (process): the added test doesn't test the implementation

tests/use-cache-server-ref-key.test.ts re-implements the key formula locally (buildNormalizedRefKey uses path.relative — tests/use-cache-server-ref-key.test.ts:29-32) and asserts that re-implementation against hashString. It never imports or exercises vinext's actual transform in index.ts. So:

• It can't catch a regression in the real code (and notably, it validates path.relative, while the implementation uses a slice — the test would stay green even if the two diverge, which is exactly the bug surface above).
• It does not cover the behavioral requirement at all. The linked Next.js test (use-cache-with-server-function-props → "should be able to use nested cache functions as props") is a client→server round-trip (button/form action). Serialization succeeding is not the same as the action resolving on submit.

Per AGENTS.md ("Add tests first", and the dev/prod-parity guidance), please add a fixture under tests/fixtures/app-basic that passes an inline "use cache" function as a prop to a client component which invokes it (e.g. via formAction/useActionState), and an E2E/integration test asserting the result actually renders after the interaction — not just that the page serializes. A green preview deploy can mask a prod-only 404 here, which is precisely the failure mode the previous review flagged.

Note: the resolved server reference is the uncached function

The hoisted export is the raw $$hoist_N function; the registerCachedFunction(...) wrapper is applied at the call site and then wrapped by registerServerReference. When the client submits the action, loadServerAction imports the module export — i.e. the uncached hoisted function (cache-runtime.ts:440 returns a separate cachedFn closure that is not what's exported). If the intent is that direct server-action invocation of a "use cache" function should still be cached, this won't do it. If Next.js treats the cached-fn-as-action as just "callable" on submit (cache applies during render, not on direct action invoke), this is fine — but please confirm against Next.js behavior and document the decision, since it's a real semantic difference.

Minor

• Heavy duplication. The transformHoistInlineDirective call + the entire runtime callback (directive-match/variant parsing) is copy-pasted across the RSC and non-RSC branches (index.ts:4318-4329 vs index.ts:4360-4372). Only the registerServerReference wrapping differs. Extract a shared runtime/options builder so the variant-parsing logic can't drift between the two paths.
• rscReactRscUrl resolved per matched module (index.ts:4303-4306) — hoist this resolution (and the IIFE) out of the per-transform path; it's invariant for the build. Also confirm the resolved file URL matches the one cache-runtime.ts already imports for @vitejs/plugin-rsc/react/rsc so the RSC env doesn't load two copies in separate module instances.
• serverReferenceMetaMap collision handling (index.ts:4335-4346): the merge path assumes vinext's entry and any plugin "use server" entry share the same referenceKey. If a file had both, the last writer wins on referenceKey; the merge only unions exportNames. Edge case, but worth a comment that this can't currently happen (or a guard).
• Branch is ~3 commits behind main (missing #1881 etc.). Rebase so CI exercises against current main.

Summary

The approach is now sound in principle — correct normalised key + manifest registration — and clearly better than the prior revision. Remaining must-fix items before merge: (1) derive the build key via manager.toRelativeId instead of the fragile slice (and from the manager's root), and (2) add a real round-trip test/fixture for "nested cache function as prop" rather than a self-referential formula test. The duplication and per-module resolution cleanups are nice-to-have. Please also confirm the uncached-export semantics match Next.js.

[ask-bonk]

Review posted to PR #1871.

Summary

This revision fixes both blockers from the prior review (normalised key + manifest registration), and I confirmed the core mechanics now line up against the @vitejs/plugin-rsc runtime. But I found remaining issues:

Blocking-ish (correctness): The build-mode key uses a fragile string slice (index.ts:4290-4296) instead of the plugin's authoritative manager.toRelativeId(id) = normalizePath(path.relative(manager.config.root, id)) (plugin-BK29Va7z.js:553-555). Two divergences:

• path.relative normalizes paths (./../trailing slashes/Windows casing); a raw slice doesn't.
• It reads this.environment.config.root rather than the manager's shared root.

A mismatch yields server reference not found on submit. Since the PR already holds rscPluginApi.manager, it should just call hashString(manager.toRelativeId(id)).

Blocking (process): The added test (tests/use-cache-server-ref-key.test.ts) re-implements the formula and tests the re-implementation — it never imports vinext's actual transform, and notably uses path.relative while the implementation uses slice, so it can't catch the divergence above. There's still no round-trip/fixture test for the actual "nested cache function as prop" behavior the PR targets (AGENTS.md requires it; the Next.js reference test is a client→server form action).

Notes: the resolved server reference is the uncached hoisted export (caching applies at render, not on direct action invoke) — needs confirmation against Next.js. Plus minor cleanups: heavy duplication of the runtime callback across RSC/non-RSC branches, per-module rscReactRscUrl resolution that should be hoisted, and the branch being ~3 commits behind main.

Verified locally: lint and format pass; the PR unit test passes (but is self-referential).

github run

[james-elicx]

Review addressed (commit 1d3c833):

Build-mode key formula (blocking-ish): The build key is now derived via plugin-rsc's own manager.toRelativeId(id) (against manager.config.root), so the hash input is byte-for-byte identical to the plugin's hashString(toRelativeId(id)). The string-slice is gone; the path.relative fallback only applies when the manager is unavailable (in which case the manifest can't be populated anyway).

Real round-trip test (blocking, process): Replaced tests/use-cache-server-ref-key.test.ts (self-referential formula test) with:

• tests/fixtures/app-basic/app/use-cache-nested-fn-props/ — ported from the Next.js use-cache-with-server-function-props/nested-cache fixture (cached component passing inline "use cache" fns as props to a client useActionState form).
• A dev-mode Playwright round-trip test in tests/e2e/app-router/use-cache.spec.ts (click → assert ISO date / random render).
• A production integration test in tests/app-router-production-server.test.ts that builds the fixture, extracts the serialized reference ids from the flight payload, invokes them via action POSTs against the prod server, and asserts the values resolve — i.e. exactly the prod-manifest path that can't be covered in dev.

Uncached-export semantics (note): Confirmed against Next.js — its use-cache transform exports the cache wrapper itself ($$RSC_SERVER_CACHE_n) as the server reference, so direct action invocation goes through the cache. Matched that: the hoisted export is now reassigned at module level to registerServerReference(registerCachedFunction(fn)) (prepended, so hoisted function declarations are wrapped before any call site or top-level initializer reads the binding). The new prod test asserts cached-invoke semantics (two identical POSTs return the same cached date).

Found while adding the prod test — a real bug in the previous revision: plugin-rsc's rsc:use-server transform runs after vinext:use-cache and deletes manager.serverReferenceMetaMap[id] for every module whose code lacks "use server". The entry written during the use-cache transform was wiped immediately, so prod action POSTs failed with server reference not found (verified: the new prod test fails on the previous revision). Registration now happens in a small vinext:use-cache-server-references plugin placed after the plugin-rsc plugins, consuming entries queued by the transform; it also unions export names if a module legitimately has both directives.

Minor items:

• Deduplicated the RSC/non-RSC branches into a single transformHoistInlineDirective call with a shared variant parser; only the runtime callback differs.
• @vitejs/plugin-rsc/react/rsc resolution hoisted out of the per-module transform (memoised once; resolves to the same file as the cache-runtime shim's bare import, so no duplicate module instance).
• metaMap collision handling now documented + unions export names in the both-directives case.
• Rebased onto current main.

Known limitation (documented in code): closure-captured bind args of inline cached fns passed as props are serialized unencrypted (plugin-rsc encrypts "use server" bound args by default). Encrypting them interacts with cache-key determinism (AES-GCM ciphertext differs per call, which would break cache keying) and needs its own design pass; the ported Next.js fixture has no closures.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(use-cache): support nested cache functions passed as props

I re-traced the @vitejs/plugin-rsc@0.5.27 runtime against this revision and verified the previously-blocking items are now genuinely fixed and exercised end-to-end. Both the build manifest path and the dev import(id) path are now covered by passing round-trip tests (I ran them locally).

Verified working

• Build round-trip (prod manifest): tests/app-router-production-server.test.ts → "resolves nested 'use cache' functions passed as props when invoked as actions" passes locally. It extracts the serialized reference ids from the flight payload and POSTs them as actions against the built server, asserting they resolve (not a 404). This is exactly the prod-only path the first review flagged as broken, and it now passes. It also asserts cached-invoke semantics (two identical POSTs return the same date), confirming the exported binding is the cached wrapper.
• Dev round-trip: the new Playwright test in tests/e2e/app-router/use-cache.spec.ts runs against vp dev (project app-router) and clicks both useActionState form buttons. Passed locally (8 passed, including the 7 pre-existing use-cache specs — no regression).
• Key formula parity: the build key now uses hashString(manager.toRelativeId(id)) via the captured rscPluginApi.manager, byte-for-byte matching plugin-rsc's getNormalizedId() (plugin-BK29Va7z.js:1587). The fragile string-slice is gone. The path.relative fallback only applies when the manager is unavailable (in which case the manifest can't be populated anyway).
• metaMap deletion ordering: confirmed plugin-rsc's rsc:use-server transform deletes serverReferenceMetaMap[id] for any module lacking "use server" (plugin-BK29Va7z.js:1603/1623). The PR correctly defers registration to a separate vinext:use-cache-server-references plugin pushed after the rsc plugins, so the entry survives. The consume-and-delete (useCacheServerRefMeta) prevents stale re-registration on HMR.
• Reassignment validity: transformHoistInlineDirective emits export async function $$hoist_N(...) declarations (transforms/index.js:27), which are reassignable function bindings, so the prepended $$hoist_N = registerServerReference(registerCachedFunction(...)) is valid. Hoisted names are always $$hoist_N, never default, so there's no default = ... hazard.
• Branch is now rebased onto current main (merge-base = latest main). Process gaps from prior reviews (no test, behind main, self-referential test) are resolved — the self-referential use-cache-server-ref-key.test.ts was replaced with real round-trip tests.

vp check packages/vinext/src/index.ts passes (format + lint + types). tests/features.test.ts (311) passes.

Non-blocking

1. Dev key omits cleanUrl//@fs/ handling for node_modules. plugin-rsc's dev getNormalizedId() does id = cleanUrl(id) when id.includes("/node_modules/") (plugin-BK29Va7z.js:1583-1585) and falls back to a /@fs/-prefixed URL for ids outside the project root (normalizeResolvedIdToUrl, plugin-BK29Va7z.js:82-85). The PR's dev branch (index.ts:4360-4365) does a plain id.slice(projectRoot.length) and returns the raw id otherwise — no query stripping, no /@fs/. For under-root app/page modules (the real use case, and what the fixture exercises) the two agree, which is why the dev test passes. But an inline "use cache" fn defined inside a node_modules package, or a module resolved with a query, would produce a divergent key and fail to resolve on submit. This is an edge case worth a short comment (or reusing cleanUrl) rather than a blocker.

2. Encrypted bound args limitation is real and documented. Closure-captured bind args of inline cached fns passed as props are serialized unencrypted, unlike plugin-rsc's "use server" transform. The code comment (index.ts:4325-4331) acknowledges this and ties it to cache-key determinism; the ported fixture has no closures so it's untested. Fine as a documented follow-up, but the gap means a cached fn prop that captures a secret would leak it to the client — worth tracking in an issue so it isn't forgotten.

Summary

The approach is sound and, crucially, now proven by both a dev and a prod round-trip test rather than just serialization. The two original blockers (wrong reference id; module not in manifest) are resolved, the key derivation reuses the plugin's authoritative toRelativeId, and the metaMap-deletion ordering bug discovered while adding the prod test is handled. Remaining items are non-blocking edge cases (node_modules/query dev keys, unencrypted bound args). I'd suggest filing an issue for the bound-args limitation before/after merge.

[ask-bonk]

Review posted to PR #1871.

Summary

I re-traced the @vitejs/plugin-rsc@0.5.27 runtime against the current revision (commit 1d3c833) and ran the new tests locally. Unlike the prior two reviews, the previously-blocking issues are now genuinely fixed and verified end-to-end:

Verified working:

• Prod build round-trip (tests/app-router-production-server.test.ts) passes — extracts serialized reference ids from the flight payload, POSTs them as actions against the built server, and confirms they resolve (not 404). This is the exact prod-only path the first review flagged as broken. Also confirms cached-invoke semantics.
• Dev round-trip (new Playwright test) passes against vp dev — clicks both useActionState form buttons; all 8 use-cache e2e specs pass (no regression).
• Key formula now uses hashString(manager.toRelativeId(id)), byte-for-byte matching the plugin (the fragile slice is gone).
• metaMap deletion ordering correctly handled via a separate plugin pushed after the rsc plugins (the bug discovered while adding the prod test).
• Reassignment validity confirmed: hoisted exports are export async function $$hoist_N declarations (reassignable), never default.
• Branch is rebased on current main; the self-referential test was replaced with real round-trip tests.
• vp check passes (format/lint/types); features.test.ts (311) passes.

Non-blocking notes:

1. Dev key omits the cleanUrl//@fs/ handling plugin-rsc does for node_modules/out-of-root ids — fine for the real app/page use case (which the test covers) but a latent edge case.
2. The unencrypted bound-args limitation is real and documented in code; worth an issue so a cached fn prop capturing a secret doesn't silently leak it to the client.

Bottom line: both original blockers are resolved and now proven by dev + prod tests. Remaining items are non-blocking edge cases.

github run

[james-elicx]

Review addressed (commit e8a434b — comment-only):

Dev key normalisation (non-blocking #1): Documented the scope directly at the dev-key derivation. Two of the divergent shapes are structurally unreachable rather than latent bugs: the transform's filter excludes node_modules entirely (exclude: /node_modules/ plus the handler guard), and the extension-anchored id regex (/\.(tsx?|jsx?|mjs)$/) rejects ids carrying a ?query, so the cleanUrl() and query-stripping branches of the plugin's getNormalizedId() can't apply here. Ids outside the project root (e.g. linked packages) keep the raw absolute path instead of the plugin's /@fs/ URL — intentionally, and self-consistently: the same key is registered in serverReferenceMetaMap (which dev validation checks) and handed to the dev loader's import(id), which accepts absolute paths.

Unencrypted bound args (non-blocking #2): Tracked as a follow-up work item with full context (design constraint: cache keys are computed from the wrapper's arguments, so AES-GCM ciphertext per-call nondeterminism must not leak into the key — likely needs decrypt-inside-the-hoisted-fn before keying, mirroring Next.js' $$cache__/boundArgsLength handling). The in-code "Known limitation" comment remains as the pointer.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(use-cache): support nested cache functions passed as props

I re-traced @vitejs/plugin-rsc@0.5.27 against the current revision (e8a434b) and re-ran both round-trip tests locally. The two original blockers (wrong reference id; module not in manifest) remain resolved, and I confirmed the core mechanics against the plugin source rather than trusting the prior reviews.

Verified working

• Prod build round-trip — tests/app-router-production-server.test.ts → "resolves nested 'use cache' functions passed as props when invoked as actions" passes locally (extracts the <12-hex>#$$hoist_N_name reference ids from the flight payload, POSTs them as actions against the built server, asserts they resolve and that two identical POSTs return the same cached date). This is the prod-only manifest path the first review flagged as broken.
• Dev round-trip — the new Playwright test (app-router project) passes; clicking both useActionState buttons renders the ISO date / random.
• Build key parity — hashString(manager.toRelativeId(id)) is byte-for-byte identical to the plugin's getNormalizedId() (plugin-BK29Va7z.js:1587; hashString = sha256→hex→slice(0,12), plugin-BK29Va7z.js:244).
• metaMap deletion ordering — confirmed rsc:use-server deletes serverReferenceMetaMap[id] for any module lacking "use server" (plugin-BK29Va7z.js:1564). rsc:use-server is a normal (un-enforced) plugin; vinext:use-cache-server-references is pushed after the rsc plugin promise, so it transforms after and re-registers. The consume-and-delete on useCacheServerRefMeta prevents stale re-registration on HMR.
• Reassignment validity — transformHoistInlineDirective emits export async function $$hoist_N_<name>(...) declarations (transforms/index.js:27) and moves the declaration to end-of-file (:29). The prepended $$hoist_N = registerServerReference(registerCachedFunction(...)) still works because function declarations hoist regardless of source position (verified with a standalone repro). registerCachedFunction returns a new closure (it doesn't mutate fn), so the reassignment is necessary for call sites to observe the wrapped fn. Hoisted names are never default, so no default = … hazard.
• Cache-id consistency — both the RSC and non-RSC branches key the cache as id + ":" + name (preserved from pre-PR), so SSR/RSC cache keys still agree.

vp check packages/vinext/src/index.ts passes (format + lint + types).

Non-blocking

1. Closure-captured bound args are unencrypted (real divergence, untested). The "Known limitation" comment (index.ts:4325-4331) is accurate: bound args hoisted into .bind(null, …) are serialized to the client unencrypted, unlike plugin-rsc's "use server" transform which encrypts by default (plugin-BK29Va7z.js:1598). This means a cached fn prop that closes over a secret leaks it to the client — a genuine data-exposure divergence from Next.js. The ported fixture has no closures, so this path has zero test coverage. Please file a tracking issue (and ideally add a fixture exercising a closure so the behavior is at least pinned, even if just asserting the documented limitation) before this expands in scope.

2. Manager-less build fallback produces a latent broken state, not a loud failure. When rscPluginApi is null in a build, the key falls back to path.relative(...) and the code still prepends the registerServerReference call with that fallback key — but vinext:use-cache-server-references bails (!rscPluginApi?.manager), so no manifest entry is written. The result is a serializable-but-unresolvable reference → prod 404 on submit. This shouldn't occur when RSC is active (rsc:minimal is always present), but if it ever did, the failure mode is a runtime 404 rather than a build error. Consider skipping the wrapping entirely (or throwing) when the manager is unavailable, so the broken state can't ship silently.

3. Dev key omits cleanUrl//@fs/ normalization — documented at index.ts:4360-4374 and the two divergent shapes are argued unreachable (the transform filter excludes node_modules; the extension-anchored id regex rejects ?query ids). I agree those are structurally unreachable for the app/page case the fixture covers. Fine as documented; the out-of-root absolute-path branch is self-consistent (same key registered + handed to import(id)).

Summary

Both original blockers are resolved and now proven by dev + prod round-trip tests, and the mechanics check out against the plugin source. No blocking issues remain. The unencrypted-bound-args divergence (#1) is the most important follow-up — it's a real client-exposure gap with no test coverage and should get a tracking issue; the manager-less fallback (#2) would be safer failing loudly than shipping a silent 404.

[ask-bonk]

Review posted to PR #1871.

Summary

I re-traced @vitejs/plugin-rsc@0.5.27 against the current revision (e8a434b) and re-ran both round-trip tests locally. The two original blockers are genuinely resolved and I verified the mechanics against the plugin source.

Verified working (no blocking issues):

• Prod manifest round-trip and dev round-trip tests both pass locally.
• Build key uses hashString(manager.toRelativeId(id)) — byte-for-byte identical to the plugin's getNormalizedId().
• metaMap-deletion ordering is correctly handled by placing vinext:use-cache-server-references after the rsc plugins.
• The prepend-reassignment works because hoisted function declarations are hoisted regardless of source position (the plugin actually moves them to end-of-file) — verified with a standalone repro.
• registerCachedFunction returns a new closure (doesn't mutate), so the reassignment is required and correct.

Non-blocking follow-ups I flagged:

1. Unencrypted closure-captured bound args — a real divergence from Next.js (which encrypts by default) and a genuine client-side secret-leak vector. Documented in code but has zero test coverage (the fixture has no closures). Recommended a tracking issue + a closure fixture.
2. Manager-less build fallback produces a serializable-but-unresolvable reference (silent prod 404) rather than failing loudly — recommended skipping the wrap or throwing when the manager is unavailable.
3. Dev key omitting cleanUrl//@fs/ normalization — documented and argued structurally unreachable; I agree.

Bottom line: implementation is sound and proven end-to-end; remaining items are non-blocking.

github run

[james-elicx]

Addressed both non-blocking follow-ups from the latest review (commits f600230, eeda6cf):

1. Unencrypted closure-captured bound args — coverage + documentation (eeda6cf)

Full encryption parity is out of scope here (as noted in the code, encrypting bound args interacts with cache-key determinism — AES-GCM ciphertext differs per render — and needs its own design pass), so the divergence is now pinned and prominently documented instead of silently untested:

• The use-cache-nested-fn-props fixture gained a third prop, getMessage, which closes over a value from the cached component's scope — exercising the .bind(null, ...) bound-arg path that previously had zero coverage.
• The production round-trip test now asserts the captured value appears verbatim (plaintext) in the flight payload — pinning the divergence from Next.js explicitly (with a note to invert the assertion if encryption lands) — and that invoking the bound reference with the bound arg resolves and observes the captured value.
• The Playwright test clicks the new button, covering the real flight-client round-trip ($$bound serialization → encodeReply → server decode) in dev.
• A new transform-level test (tests/use-cache-transform.test.ts) pins that captures are emitted as plain bind args with no encrypt wrapper.
• User-facing documentation added to the README's Known limitations section ("don't close over secrets in cached functions you pass to the client"), alongside the existing code comment.

2. Manager-less build fallback → fail loud (f600230)

The transform now throws when the rsc environment is active but the plugin-rsc manager is unavailable, instead of computing a fallback key and emitting a serializable-but-unresolvable reference. The throw is placed before the hoist try block (so the swallow-on-hoist-failure catch can't eat it) and covers dev as well as build — dev-mode reference validation reads the same serverReferenceMetaMap the registration plugin can't populate without the manager. Unit tests cover the build and dev throw paths, a non-rsc-environment control (no manager needed), and build reference-key parity (sha256(toRelativeId(id))[:12]) with a fake manager.

3. Dev key cleanUrl//@fs/ normalization — no change, per the review's agreement that the divergent shapes are structurally unreachable (documented at the dev-key computation site).

Verified: vp check clean; new unit tests, the prod-server nested-fn-props test, and the Playwright nested-cache spec all pass locally.

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(use-cache): support nested cache functions passed as props

I re-traced @vitejs/plugin-rsc@0.5.27 against the current revision (eeda6cf) and re-ran the unit, prod-server, and transform tests locally. Both original blockers (wrong reference id; module not in manifest) remain resolved, and the two non-blocking follow-ups from the prior review are addressed. I have no blocking issues.

Verified against the plugin source

• Build key parity. hashString(manager.toRelativeId(id)) is byte-for-byte identical to plugin-rsc's getNormalizedId() build branch (plugin-BK29Va7z.js:1587; hashString = sha256→hex→slice(0,12), :244). The fragile string-slice from earlier revisions is gone.
• Dev key parity (under-root case). plugin-rsc's dev key for an under-root id is resolved.id.slice(root.length) (normalizeResolvedIdToUrl, :82), which keeps the leading / (e.g. /app/.../page.tsx). The PR's id.slice(projectRoot.length) (index.ts:4392-4395) matches exactly. The node_modules/?query//@fs/ shapes the plugin also handles are correctly argued unreachable (the transform filter excludes node_modules; the extension-anchored id regex rejects ?query ids) and documented at the derivation site.
• metaMap deletion ordering. rsc:use-server deletes serverReferenceMetaMap[id] for any module lacking "use server" (:1564, :1603, :1623). The PR correctly defers registration to vinext:use-cache-server-references, pushed after the rsc plugin promise, so its transform runs after rsc:use-server and re-populates the entry. The consume-and-delete on useCacheServerRefMeta prevents stale re-registration on HMR.
• Dev validation path. rsc:reference-validation (:616) checks serverReferenceMetaMap by referenceKey, and the dev loader does import(id) directly (rsc.js:19-21). The registered key is both a valid metaMap entry and a Vite-importable id, so both validation and import resolve.
• Reassignment validity. transformHoistInlineDirective emits export async function $$hoist_N(...) declarations (transforms/index.js:27) — reassignable, fully-hoisted live bindings, never default. The prepended $$hoist_N = registerServerReference(registerCachedFunction($$hoist_N, …)) runs at module-body start when the declaration already exists, and registerCachedFunction returns a new closure (cache-runtime.ts:457), so the reassignment is both necessary and correct.
• Fail-loud placement. The missing-manager throw (index.ts:4355) sits outside the hoist try/catch (:4404, swallow at :4462), so it can't be silently eaten — addressing follow-up #2 from the last review. Unit tests cover the build + dev throw paths and a non-RSC control (use-cache-transform.test.ts).

Tests run locally (all pass)

• tests/use-cache-transform.test.ts — 5 passed. These now exercise vinext's actual transform hook (manager-less throw, build-key parity, plaintext bound-arg pinning) rather than a self-referential formula.
• tests/app-router-production-server.test.ts -t nested — passes; extracts the <12-hex>#$$hoist_N_name reference ids from the flight payload, POSTs them as actions against the built server, asserts they resolve (not 404) and that two identical getDate POSTs return the same cached value.

Non-blocking follow-ups (both adequately addressed)

1. Unencrypted closure-captured bound args — the real divergence from Next.js (which encrypts bound args by default) is now (a) pinned at the transform level (.bind(null, capturedSecret), no encrypt wrapper), (b) pinned at runtime in the prod test (captured value appears verbatim/plaintext in the flight payload), (c) exercised end-to-end in the Playwright test via the new getMessage prop, and (d) documented in the README "Known limitations" with a concrete mitigation ("pass an identifier and re-read the secret on the server"). This is the right outcome given encryption interacts with cache-key determinism and needs its own design pass.
2. Manager-less build fallback — now throws loudly instead of emitting a serializable-but-unresolvable reference.

Minor (optional, non-blocking)

• getRscReactRscUrl() vs the cache-runtime bare import. The prepend imports registerServerReference from a file:// URL while cache-runtime.ts:215 imports @vitejs/plugin-rsc/react/rsc as a bare specifier. The comment asserts Vite normalises these to one module id in the RSC env; the passing round-trip tests confirm it holds today, but it's a latent coupling worth a brief note if plugin-rsc ever changes its module-id normalisation.
• getMessage cross-path cache coverage. The prod test asserts cached-invoke semantics for getDate (two identical POSTs return the same value) but not that the closure-bound getMessage produces the same key on render vs direct action POST. Not a correctness concern given how the bound arg flows, but an extra assertion would fully pin the bound-arg cache key.

Summary

The approach is sound and proven end-to-end by transform-level, prod-build, and dev round-trip tests. Both original blockers are resolved with byte-for-byte key parity against the plugin, the metaMap-deletion ordering is correctly handled, the manager-less path fails loudly, and the unencrypted-bound-args divergence is pinned + documented rather than silently untested. vp check passes. No blocking issues; the remaining items are optional polish.

[ask-bonk]

Review posted to PR #1871.

Summary

I re-traced @vitejs/plugin-rsc@0.5.27 against the current revision (eeda6cf) and re-ran the unit, transform, and prod-server tests locally. The two original blockers remain resolved, both prior non-blocking follow-ups are addressed, and I found no new blocking issues.

Verified against the plugin source:

• Build key hashString(manager.toRelativeId(id)) is byte-for-byte identical to plugin-rsc's getNormalizedId() (hashString = sha256→hex→slice(0,12)).
• Dev key id.slice(projectRoot.length) matches the plugin's normalizeResolvedIdToUrl for under-root ids (keeps the leading /); the unreplicated node_modules/?query//@fs/ shapes are correctly argued structurally unreachable.
• metaMap-deletion ordering is handled by running vinext:use-cache-server-references after the rsc plugins, with consume-and-delete to avoid stale HMR re-registration.
• Reassignment of export async function $$hoist_N declarations is valid (hoisted, reassignable, never default); registerCachedFunction returns a new closure so the reassignment is required.
• The missing-manager throw is outside the hoist try/catch, so it fails loudly rather than producing a silent prod 404.

Tests run locally (all pass): transform unit tests (now exercising the real transform), prod-server nested-fn-props round-trip (extracts reference ids → action POST → resolves + cached semantics), and vp check.

Non-blocking items, both adequately addressed: the unencrypted closure-captured bound-args divergence is now pinned at transform + runtime levels and documented in the README; the manager-less fallback now throws.

I flagged two optional polish items (the file:// vs bare-specifier coupling for @vitejs/plugin-rsc/react/rsc, and an extra cross-path cache assertion for the closure-bound getMessage), but neither blocks merge.

github run

[james-elicx]

Both optional polish items from the latest review are now addressed in-PR (a9cd049f + 67d7fb27):

1. file:// vs bare-specifier coupling for @vitejs/plugin-rsc/react/rsc — decoupled (a9cd049). Rather than documenting the latent coupling, I removed it. The transform's prepend no longer imports registerServerReference from a file:// URL of the plugin-rsc package entry; it imports it from a new vinext-owned shim, packages/vinext/src/shims/cache-server-reference.ts, which re-exports it via the same bare @vitejs/plugin-rsc/react/rsc specifier the cache runtime uses, resolved from the same importer location (vinext's shims directory). That guarantees a single react/rsc module instance by construction — no reliance on Vite normalising a file:// URL and a bare import to one module id, so a future change to plugin-rsc's module-id normalisation can't split the instance. The shim itself is only ever imported via the transform's file:// URL (one specifier shape → one id), the file:// indirection is still needed because transformed modules can live outside the project root where the bare specifier may not resolve (rationale in the shim header), and getRscReactRscUrl() is deleted. The transform unit test now pins that the emitted import targets the shim and never a plugin-rsc file URL.

2. Cross-path cache assertion for the closure-bound getMessage (67d7fb2). getMessage was deterministic, so a repeat-invocation equality check would have been vacuous. The fixture now appends a Math.random() suffix (making hits observable, same trick as getDate's timestamp), and the prod-server round-trip test mirrors the getDate assertion on the bound path: two POSTs with the same bound arg must return the identical cached value (a recompute would change the suffix), and a POST with a different bound arg must miss with a fresh value observing the new arg — pinning both that the bound arg produces a stable cache key and that it participates in the key. The Playwright assertion now matches the suffixed message via an anchored regex.

Verification: vp test run tests/use-cache-transform.test.ts (5 passed), vp test run tests/app-router-production-server.test.ts -t nested (3 passed), playwright test tests/e2e/app-router/use-cache.spec.ts (8 passed), vp check clean.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(use-cache): support nested cache functions passed as props

I traced @vitejs/plugin-rsc@0.5.27 against the current revision (67d7fb2) and re-ran the unit, transform, prod-server, and dev (Playwright) round-trip tests locally. The two original blockers remain resolved, all prior follow-ups are addressed, and I found no blocking issues. I verified the mechanics against the plugin source rather than relying on the prior reviews.

Verified against the plugin source

• Build key parity. The PR's build key is createHash("sha256").update(manager.toRelativeId(id)).digest().toString("hex").slice(0,12) (index.ts:4357-4363). plugin-rsc's hashString is createHash("sha256").update(v).digest().toString("hex").slice(0,12) (plugin-BK29Va7z.js:244-246) applied to manager.toRelativeId(id) (:1587). Byte-for-byte identical, and it reuses the plugin's own toRelativeId so the input can't drift.
• Dev key parity (under-root case). plugin-rsc's dev key for an under-root id is resolved.id.slice(root.length) (normalizeResolvedIdToUrl, :82), keeping the leading /. The PR's id.slice(projectRoot.length) for under-root ids matches (:4379-4382). The node_modules/?query//@fs/ shapes plugin-rsc additionally handles are correctly argued structurally unreachable (the transform filter excludes node_modules; the extension-anchored id regex rejects ?query ids) and documented at the derivation site.
• metaMap-deletion ordering. Confirmed rsc:use-server deletes serverReferenceMetaMap[id] for any module lacking "use server" (:1563-1565, :1603, :1623). The PR correctly defers registration to a separate vinext:use-cache-server-references plugin pushed after the rsc plugin promise, with consume-and-delete on useCacheServerRefMeta to avoid stale HMR re-registration.
• Manifest shape. virtual:vite-rsc/server-references is keyed by meta.referenceKey, imports meta.importId, and destructures meta.exportNames (:1654-1665). The PR writes { importId: id, referenceKey: normalizedRefKey, exportNames: result.names }, and the $$hoist_N_name exports are valid identifiers the manifest can re-import.
• rsc:minimal api lookup. The configResolved lookup (config.plugins.find(p => p.name === "rsc:minimal")?.api) mirrors getPluginApi exactly (:568-569); the manager object reference exists at plugin construction (:577), and manager.config.root (read only at transform time) is populated by rsc:minimal's own configResolved (:581-582) — no hook-ordering hazard.
• Reassignment validity. transformHoistInlineDirective emits export async function $$hoist_N(...) declarations and moves them to end-of-file (transforms/index.js:27-29); inline arrows passed as props also become declarations (declName falsy → still function $$hoist_N_anonymous_server_function). Hoisted declarations exist before the prepended reassignment runs, so every later reader observes the wrapped fn. registerCachedFunction returns a new closure, so the reassignment is necessary. Names are never default.
• Cache-key consistency. Both branches key the cache as id + ":" + name (preserved from main), so SSR/RSC cache keys still agree.
• Fail-loud placement. The missing-manager throw (:4342-4350) is outside the hoist try/catch, so it can't be silently swallowed.

Tests run locally (all pass)

• tests/use-cache-transform.test.ts — 5 passed (manager-less throw in build+dev, non-RSC control, build-key parity with a fake manager, plaintext bound-arg pinning). Crucially these now exercise vinext's actual transform hook, not a self-referential formula.
• tests/app-router-production-server.test.ts -t nested — 3 passed: extracts the <12-hex>#$$hoist_N_name reference ids from the flight payload, POSTs them as actions against the built server, asserts they resolve (not 404), cached-invoke semantics on both the unbound (getDate) and closure-bound (getMessage) paths, and that the bound arg participates in the cache key.
• tests/e2e/app-router/use-cache.spec.ts "nested cache functions as props" — passed against vp dev (after installing the chromium binary). Covers the full dev client→server useActionState round-trip including the bound-arg flight path.
• vp check on the changed source files passes (format + lint + types).

Non-blocking notes (no change required)

1. Unencrypted closure-captured bound args remains a real, documented divergence from Next.js (which encrypts bound args by default — :1594-1599). It is now pinned at the transform level, in the prod test (plaintext-in-payload assertion), end-to-end in Playwright, and surfaced in the README "Known limitations". This is the right call given encryption interacts with cache-key determinism. The README mitigation ("pass an identifier and re-read the secret on the server") is sound. Worth a tracking issue so it isn't lost, but it does not block this PR.
2. Object.defineProperty($$hoist_N, "name", ...) appended at end-of-file by the hoist transform runs after the prepended reassignment, so it sets name on the wrapped closure rather than the original. Benign (cosmetic only), just noting it for completeness.

Summary

The approach is sound and proven end-to-end by transform-level, prod-build, and dev round-trip tests, with byte-for-byte key parity against the plugin and the metaMap-deletion ordering handled correctly. Both original blockers are resolved, the manager-less path fails loudly, and the unencrypted-bound-args divergence is pinned and documented rather than silently untested. No blocking issues.

[ask-bonk]

Review posted to PR #1871.

Summary

I traced @vitejs/plugin-rsc@0.5.27 against the current revision (67d7fb2) and re-ran the unit, transform, prod-server, and dev (Playwright) round-trip tests locally. Both original blockers remain resolved, all prior follow-ups are addressed, and I found no blocking issues.

Verified against the plugin source rather than trusting prior reviews:

• Build key is byte-for-byte identical to hashString(manager.toRelativeId(id)) (plugin-BK29Va7z.js:244,1587).
• Dev key matches normalizeResolvedIdToUrl's under-root slice (:82); the unreplicated shapes are structurally unreachable and documented.
• metaMap-deletion ordering (:1564) is correctly handled by running vinext:use-cache-server-references after the rsc plugins, with consume-and-delete for HMR safety.
• Manifest shape (:1654-1665), rsc:minimal api lookup (:568), reassignment validity (hoisted declarations), and cache-key consistency all check out.
• The missing-manager throw is outside the hoist try/catch, so it fails loudly.

Tests run locally (all pass): transform unit tests (now exercising the real transform), prod-build round-trip (extracts reference ids → action POST → resolves + cached semantics on both unbound and closure-bound paths), dev Playwright round-trip, and vp check.

Non-blocking notes: the unencrypted closure-captured bound-args divergence is real but now pinned + documented (worth a tracking issue); and a benign cosmetic note about Object.defineProperty(name) running after the reassignment. The two Playwright failures in the first run were missing browser binaries, not test logic — they passed after playwright install.

github run

[james-elicx]

WTF?! fix this

[james-elicx]

would it help at all if we exported some extra stuff from that library? there's a potential to make more of their functionality available for us to use if needed. let me know.