fix: use cache plugin (#387)

The `use-cache-inline` plugin had several bugs in how it tracks closure
variables and handles exported cached functions. The root cause was a
flat `locals` array that accumulated variable names from all function
scopes without any scoping — variables declared inside one function
would leak into the closure capture list of completely unrelated sibling
functions. This caused the plugin to emit invalid `.bind()` calls on
module-level exported cached functions, producing parse errors like
`export __cache_name__.bind(null, src, selector, isolate)` during
production builds.
The fix replaces the flat `locals` array with a proper scope stack. Each
non-cached function pushes a scope entry on enter and pops it on leave,
so only variables from actual enclosing lexical scopes are considered
for closure capture. Module-level cached functions now correctly see an
empty scope stack and skip `.bind()` entirely.

Additional fixes included in this change: destructured binding patterns
(`ObjectPattern`, `ArrayPattern`, `AssignmentPattern`, `RestElement`)
are now properly walked when collecting local variable names. Function
parameters in enclosing scopes are tracked for nested cached function
closure capture. Identifiers in non-reference positions (object property
keys, member expression properties, declaration ids, function names) are
excluded from closure matching — this was necessary because the JSX
transform runs before the cache plugin, turning JSX attribute names into
regular Property key Identifiers that would otherwise be falsely
captured. Exported cached functions are now wrapped in
`__react_cache__()` at module level and get correct
`CACHE_KEY`/`CACHE_PROVIDER` assignments using scoping-aware target
resolution.

Author: lazarv

packages/react-server/lib/plugins/use-cache-inline.mjs

@@ -129,8 +129,46 @@ export default function useCacheInline(profiles, providers = {}, type) {
           ...resolvedProviders,
         };
 
+        // Extract all binding names from a pattern node (Identifier, ObjectPattern,
+        // ArrayPattern, AssignmentPattern, RestElement).
+        function collectBindingNames(pattern, out = []) {
+          if (!pattern) return out;
+          switch (pattern.type) {
+            case "Identifier":
+              out.push(pattern.name);
+              break;
+            case "ObjectPattern":
+              for (const prop of pattern.properties) {
+                if (prop.type === "RestElement") {
+                  collectBindingNames(prop.argument, out);
+                } else {
+                  collectBindingNames(prop.value, out);
+                }
+              }
+              break;
+            case "ArrayPattern":
+              for (const el of pattern.elements) {
+                if (el) collectBindingNames(el, out);
+              }
+              break;
+            case "AssignmentPattern":
+              collectBindingNames(pattern.left, out);
+              break;
+            case "RestElement":
+              collectBindingNames(pattern.argument, out);
+              break;
+          }
+          return out;
+        }
+
         const caches = [];
-        const locals = [];
+        // Scope stack: each entry is { node, names: Set<string> } representing
+        // a function scope. Only non-cached function scopes are pushed.
+        // When checking closure captures for a cached function, we look at all
+        // entries on the stack — they represent the enclosing lexical scopes.
+        // This prevents variables from unrelated sibling functions from leaking
+        // into the closure capture list.
+        const scopeStack = [];
         let parent = null;
         let useCacheNode = null;
         let useCache = null;
@@ -208,14 +246,53 @@ export default function useCacheInline(profiles, providers = {}, type) {
             }
 
             if (useCacheNode && node.type === "Identifier") {
+              // Skip identifiers in non-reference positions (object property
+              // keys, non-computed member expression properties, declaration
+              // ids). After JSX transformation, JSX attribute names become
+              // Property keys (e.g., { name: "World" }) and must not be
+              // treated as closure variable references.
+              const isNonRef =
+                (node.parent?.type === "Property" &&
+                  node.parent.key === node &&
+                  !node.parent.computed) ||
+                (node.parent?.type === "MemberExpression" &&
+                  node.parent.property === node &&
+                  !node.parent.computed) ||
+                (node.parent?.type === "VariableDeclarator" &&
+                  node.parent.id === node) ||
+                ((node.parent?.type === "FunctionDeclaration" ||
+                  node.parent?.type === "FunctionExpression") &&
+                  node.parent.id === node);
               if (
-                locals.includes(node.name) &&
-                !useCache.params.includes(node.name)
+                !isNonRef &&
+                scopeStack.some((s) => s.names.has(node.name)) &&
+                !useCache.params.includes(node.name) &&
+                !useCache.locals.includes(node.name)
               ) {
                 useCache.params.push(node.name);
               }
             }
 
+            // Track function/arrow parameters as locals (these are bindings in
+            // the enclosing scope that a nested "use cache" function may close
+            // over). Push a new scope entry onto the stack so that variables
+            // from sibling/unrelated functions don't leak into the closure
+            // capture list.
+            if (
+              !useCacheNode &&
+              (node.type === "FunctionDeclaration" ||
+                node.type === "FunctionExpression" ||
+                node.type === "ArrowFunctionExpression")
+            ) {
+              const names = new Set();
+              for (const param of node.params) {
+                for (const n of collectBindingNames(param)) {
+                  names.add(n);
+                }
+              }
+              scopeStack.push({ node, names });
+            }
+
             if (node.type === "VariableDeclarator") {
               let parent = node.parent;
               while (parent) {
@@ -228,17 +305,28 @@ export default function useCacheInline(profiles, providers = {}, type) {
                 parent = parent.parent;
               }
               if (parent) {
+                const names = collectBindingNames(node.id);
                 if (useCacheNode) {
-                  useCache.locals.push(node.id.name);
-                } else {
-                  locals.push(node.id.name);
+                  useCache.locals.push(...names);
+                } else if (scopeStack.length > 0) {
+                  const topScope = scopeStack[scopeStack.length - 1].names;
+                  for (const name of names) topScope.add(name);
                 }
               }
             }
 
             parent = node;
           },
           leave(node) {
+            // Pop scope when leaving a non-cached function whose scope was
+            // pushed in `enter`.
+            if (
+              scopeStack.length > 0 &&
+              scopeStack[scopeStack.length - 1].node === node
+            ) {
+              scopeStack.pop();
+            }
+
             if (node === useCacheNode) {
               if (useCache.params.length > 0) {
                 useCacheNode.type = "CallExpression";
@@ -264,14 +352,14 @@ export default function useCacheInline(profiles, providers = {}, type) {
                   })),
                 ];
               } else if (useCache.parent?.type === "ExportNamedDeclaration") {
-                useCache.name = useCache.parent.declaration.id.name;
+                useCache.exported = true;
                 useCache.parent.parent.body =
                   useCache.parent.parent.body.filter(
                     (n) => n !== useCache.parent
                   );
               } else if (useCache.parent?.type === "ExportDefaultDeclaration") {
+                useCache.exported = true;
                 useCache.identifier = "_default";
-                useCache.name = "_default";
                 useCache.parent.parent.body =
                   useCache.parent.parent.body.filter(
                     (n) => n !== useCache.parent
@@ -548,11 +636,8 @@ export default function useCacheInline(profiles, providers = {}, type) {
                           type: "ArrayPattern",
                           elements: [
                             ...cache.params.map((param) => ({
-                              type: "VariableDeclarator",
-                              id: {
-                                type: "Identifier",
-                                name: param,
-                              },
+                              type: "Identifier",
+                              name: param,
                             })),
                             ...cache.node.params,
                           ],
@@ -790,29 +875,72 @@ export default function useCacheInline(profiles, providers = {}, type) {
               },
             },
           ];
-          ast.body.push(
-            {
-              type: "FunctionDeclaration",
-              async: !(cache.provider === "request" && isClient),
-              id: {
-                type: "Identifier",
-                name: cache.name,
-              },
-              params: [
-                ...(argsName
-                  ? [
+          ast.body.push({
+            type: "FunctionDeclaration",
+            async: !(cache.provider === "request" && isClient),
+            id: {
+              type: "Identifier",
+              name: cache.name,
+            },
+            params: [
+              ...(argsName
+                ? [
+                    {
+                      type: "RestElement",
+                      argument: {
+                        type: "Identifier",
+                        name: argsName,
+                      },
+                    },
+                  ]
+                : cache.node.params),
+            ],
+            body: cache.node.body,
+          });
+
+          // For exported cached functions, wrap in __react_cache__() so they
+          // get the same per-request memoization as non-exported cached
+          // functions.
+          if (cache.exported && cache.identifier) {
+            ast.body.push({
+              type: "VariableDeclaration",
+              kind: "const",
+              declarations: [
+                {
+                  type: "VariableDeclarator",
+                  id: {
+                    type: "Identifier",
+                    name: cache.identifier,
+                  },
+                  init: {
+                    type: "CallExpression",
+                    callee: {
+                      type: "Identifier",
+                      name: "__react_cache__",
+                    },
+                    arguments: [
                       {
-                        type: "RestElement",
-                        argument: {
-                          type: "Identifier",
-                          name: argsName,
-                        },
+                        type: "Identifier",
+                        name: cache.name,
                       },
-                    ]
-                  : cache.node.params),
+                    ],
+                  },
+                },
               ],
-              body: cache.node.body,
-            },
+            });
+          }
+
+          // For nested cached functions (parent is a BlockStatement inside
+          // another function), the identifier is scoped to the enclosing
+          // function and not accessible at module level. Use the mangled impl
+          // name (which is always module-level) for the CACHE_KEY/PROVIDER
+          // assignments.
+          const isModuleScoped =
+            cache.exported || cache.parent?.type === "Program";
+          const cacheTarget = isModuleScoped
+            ? (cache.identifier ?? cache.name)
+            : cache.name;
+          ast.body.push(
             {
               type: "ExpressionStatement",
               expression: {
@@ -823,7 +951,7 @@ export default function useCacheInline(profiles, providers = {}, type) {
                   computed: true,
                   object: {
                     type: "Identifier",
-                    name: cache.identifier ?? cache.name,
+                    name: cacheTarget,
                   },
                   property: {
                     type: "Identifier",
@@ -843,7 +971,7 @@ export default function useCacheInline(profiles, providers = {}, type) {
                   computed: true,
                   object: {
                     type: "Identifier",
-                    name: cache.identifier ?? cache.name,
+                    name: cacheTarget,
                   },
                   property: {
                     type: "Identifier",
@@ -863,14 +991,38 @@ export default function useCacheInline(profiles, providers = {}, type) {
         ast.body.push({
           type: "ExportNamedDeclaration",
           declaration: null,
-          specifiers: caches.map((cache) => ({
-            type: "ExportSpecifier",
-            exported: {
-              type: "Identifier",
-              name: cache.name === "_default" ? "default" : cache.name,
-            },
-            local: { type: "Identifier", name: cache.name },
-          })),
+          specifiers: caches.flatMap((cache) => {
+            const specs = [
+              {
+                type: "ExportSpecifier",
+                exported: {
+                  type: "Identifier",
+                  name: cache.name === "_default" ? "default" : cache.name,
+                },
+                local: { type: "Identifier", name: cache.name },
+              },
+            ];
+            // For exported cached functions, also export the user-facing
+            // identifier (which is the __react_cache__-wrapped version).
+            if (
+              cache.exported &&
+              cache.identifier &&
+              cache.identifier !== cache.name
+            ) {
+              specs.push({
+                type: "ExportSpecifier",
+                exported: {
+                  type: "Identifier",
+                  name:
+                    cache.identifier === "_default"
+                      ? "default"
+                      : cache.identifier,
+                },
+                local: { type: "Identifier", name: cache.identifier },
+              });
+            }
+            return specs;
+          }),
         });
 
         return codegen(ast, id);

test/__test__/use-cache-locals.spec.mjs

@@ -0,0 +1,34 @@
+import { hostname, page, server } from "playground/utils";
+import { expect, test } from "vitest";
+
+test("use cache locals", async () => {
+  await server("fixtures/use-cache-locals.jsx");
+  await page.goto(hostname);
+
+  // Test 1: Function parameter closure — `prefix` captured as a local
+  expect(await page.textContent("#greeting")).toBe("Hello: World");
+
+  // Test 2: Destructured variable closure — `locale` and `currency` captured
+  expect(await page.textContent("#formatted")).toBe("$42.00");
+
+  // Test 3: Array destructured variable closure — `left` and `right` captured
+  expect(await page.textContent("#labeled")).toBe("[test]");
+
+  // Test 4: Exported cached function
+  const cachedId = await page.textContent("#cached-id");
+  expect(cachedId).toBe("default");
+
+  const cachedTime = await page.textContent("#cached-time");
+  expect(cachedTime).toBeTruthy();
+
+  // Verify caching works — reload should return the same cached time
+  await page.reload();
+  expect(await page.textContent("#cached-time")).toBe(cachedTime);
+  expect(await page.textContent("#greeting")).toBe("Hello: World");
+  expect(await page.textContent("#formatted")).toBe("$42.00");
+  expect(await page.textContent("#labeled")).toBe("[test]");
+
+  // Verify dynamic args work — different id should produce different cached-id
+  await page.goto(hostname + "?id=test123");
+  expect(await page.textContent("#cached-id")).toBe("test123");
+});

test/__test__/use-cache-sibling-scope.spec.mjs

@@ -0,0 +1,34 @@
+import { hostname, page, server } from "playground/utils";
+import { expect, test } from "vitest";
+
+test("use cache sibling scope - no false closure capture across sibling functions", async () => {
+  await server("fixtures/use-cache-sibling-scope.jsx");
+  await page.goto(hostname);
+
+  // Verify the prepared (non-cached helper → cached function) path works
+  expect(await page.textContent("#prepared-label")).toBe("[foo]");
+  expect(await page.textContent("#prepared-value")).toBe("42");
+  expect(await page.textContent("#prepared-extra")).toBe("bonus");
+
+  const preparedTimestamp = await page.textContent("#prepared-timestamp");
+  expect(preparedTimestamp).toBeTruthy();
+
+  // Verify the direct cached function call works
+  expect(await page.textContent("#direct-label")).toBe("direct");
+  expect(await page.textContent("#direct-value")).toBe("99");
+  expect(await page.textContent("#direct-extra")).toBe("none");
+
+  const directTimestamp = await page.textContent("#direct-timestamp");
+  expect(directTimestamp).toBeTruthy();
+
+  // Verify caching: reload should return the same cached timestamps
+  await page.reload();
+  expect(await page.textContent("#prepared-timestamp")).toBe(preparedTimestamp);
+  expect(await page.textContent("#direct-timestamp")).toBe(directTimestamp);
+
+  // Verify values still correct after reload
+  expect(await page.textContent("#prepared-label")).toBe("[foo]");
+  expect(await page.textContent("#prepared-value")).toBe("42");
+  expect(await page.textContent("#direct-label")).toBe("direct");
+  expect(await page.textContent("#direct-value")).toBe("99");
+});