refactor(v4): converge the two expression evaluators

packages/v4/migration/Action/expression.ts

@@ -1,46 +1,21 @@
-/**
- * Expression evaluator for action bindings. It uses `new Function`, so it
- * requires a Content Security Policy that permits `unsafe-eval`.
- */
+import { compileExpression, type CompiledExpression } from '../expression.js';
 
 /**
  * A compiled effect. Called with `this` bound to the action's element and the
  * argument list `executeEffect()` assembles; may return a function, which is
  * then called with the same `this` and the same arguments.
  */
-export type EffectFunction = (...args: unknown[]) => unknown;
-
-const cache = new Map<string, EffectFunction>();
-
-/**
- * Compile `body` into a function taking `argNames`, memoised under `cacheKey`.
- */
-function compile(argNames: readonly string[], body: string, cacheKey: string): EffectFunction {
-  let callback = cache.get(cacheKey);
-  if (!callback) {
-    // Compiling the author's expression is the whole feature.
-    // oxlint-disable-next-line no-new-func, typescript/no-implied-eval
-    callback = new Function(...argNames, body) as EffectFunction;
-    cache.set(cacheKey, callback);
-  }
-  return callback;
-}
+export type EffectFunction = CompiledExpression;
 
 /** Effect argument names in `executeEffect()` order. */
 export const EFFECT_ARGUMENTS = ['ctx', 'event', 'target', 'action', 'self', '$el'] as const;
 
 /**
  * Compile one `data-on:*` or `data-option-effect` expression.
  *
- * The cache key includes the source and complete parameter list.
- *
  * @param code The expression source, for example `target.open()`.
  * @param instanceNames Co-located component names added as parameters.
  */
 export function getEffect(code: string, instanceNames: readonly string[]): EffectFunction {
-  return compile(
-    [...EFFECT_ARGUMENTS, ...instanceNames],
-    `return ${code}`,
-    `${instanceNames.length}:${instanceNames.join(',')}:${code}`,
-  );
+  return compileExpression([...EFFECT_ARGUMENTS, ...instanceNames], `return ${code}`);
 }

packages/v4/migration/Data/DataBind.ts

@@ -325,11 +325,7 @@ export class DataBind<T extends BaseProps = DataBindProps>
 
       if (binding.expression) {
         try {
-          result = getCallback(this.group, `return ${binding.expression};`)(
-            value,
-            this.target,
-            this.$data,
-          );
+          result = getCallback(`return ${binding.expression};`)(value, this.target, this.$data);
         } catch (error) {
           console.error('[data] Binding expression failed:', error);
           continue;

packages/v4/migration/Data/DataComputed.ts

@@ -22,8 +22,7 @@ export class DataComputed extends DataBind<DataComputedProps> {
   }
 
   get compute(): DataExpression {
-    const { group, compute } = this.$options;
-    return getCallback(group, `return ${compute};`);
+    return getCallback(`return ${this.$options.compute};`);
   }
 
   override set(value: DataValue): void {

packages/v4/migration/Data/DataEffect.ts

@@ -22,8 +22,7 @@ export class DataEffect extends DataBind<DataEffectProps> {
   }
 
   get effect(): DataExpression {
-    const { group, effect } = this.$options;
-    return getCallback(group, effect);
+    return getCallback(this.$options.effect);
   }
 
   override set(value: DataValue): void {

packages/v4/migration/Data/expression.ts

@@ -1,23 +1,14 @@
-/**
- * Expression evaluator for Data components. It uses `new Function`, so it
- * requires a Content Security Policy that permits `unsafe-eval`. The cache
- * keeps identical expressions in different groups separate.
- */
+import { compileExpression } from '../expression.js';
 
 export type DataExpression = (value: unknown, target: HTMLElement, $data: unknown) => unknown;
 
-const callbacks = new Map<string, DataExpression>();
-
-export function getCallback(name: string, code: string): DataExpression {
-  const key = code + name;
+const ARG_NAMES = ['value', 'target', '$data'] as const;
 
-  let callback = callbacks.get(key);
-  if (!callback) {
-    // Compiling the author's expression is the whole feature.
-    // oxlint-disable-next-line no-new-func, typescript/no-implied-eval
-    callback = new Function('value', 'target', '$data', code) as DataExpression;
-    callbacks.set(key, callback);
-  }
-
-  return callback;
+/**
+ * Compile a `data-bind:*`, `data-compute` or `data-effect` expression.
+ *
+ * @param code The expression source, for example `return value * 2;`.
+ */
+export function getCallback(code: string): DataExpression {
+  return compileExpression(ARG_NAMES, code);
 }

packages/v4/migration/Fetch/Fetch.ts

@@ -12,6 +12,7 @@ import {
   type SwapMode,
 } from '../../src/index.js';
 import { historyPush } from '../../src/utils/history.js';
+import { compileExpression } from '../expression.js';
 
 /**
  * The lifecycle events a `Fetch` announces.
@@ -46,6 +47,9 @@ export const HEADER_NAMES = Object.freeze({
 /** One parser for every instance: it holds no state between calls. */
 const domParser = new DOMParser();
 
+/** `response` expression argument names, in `parseResponse()`'s call order. */
+const RESPONSE_ARGUMENTS = ['response', 'url', 'requestInit', 'self'] as const;
+
 /** The context every lifecycle event carries. */
 export interface FetchEventBase {
   instance: Fetch;
@@ -351,15 +355,12 @@ export class Fetch<T extends BaseProps = BaseProps> extends Base<FetchProps & T>
    * @protected
    */
   parseResponse(response: Response, url: URL, requestInit: RequestInit): Promise<string> | string {
-    // Evaluating the author's `response` expression is the documented feature.
-    // oxlint-disable-next-line no-new-func, typescript/no-implied-eval
-    const fn = new Function(
-      'response',
-      'url',
-      'requestInit',
-      'self',
-      `return ${this.$options.response}`,
-    ) as (response: Response, url: URL, requestInit: RequestInit, self: unknown) => string;
+    const fn = compileExpression(RESPONSE_ARGUMENTS, `return ${this.$options.response}`) as (
+      response: Response,
+      url: URL,
+      requestInit: RequestInit,
+      self: unknown,
+    ) => string;
     return fn.call(this, response, url, requestInit, self);
   }
 

packages/v4/migration/expression.spec.ts

@@ -0,0 +1,24 @@
+import { describe, expect, it } from 'vitest';
+import { compileExpression } from './expression.js';
+
+describe('compileExpression', () => {
+  it('does not collide two argument lists split at different boundaries', () => {
+    // `Action`'s v3 predecessor keyed its cache by `effectDefinition +
+    // keys.join('')`, under which `['Ab', 'C']` and `['A', 'bC']` land in the
+    // same entry: joined without a separator, both produce `AbC`. A body that
+    // returns its first argument must resolve `Ab` and `A` to their own value,
+    // not to whichever list compiled first.
+    const first = compileExpression(['Ab', 'C'], 'return Ab;');
+    const second = compileExpression(['A', 'bC'], 'return A;');
+
+    expect(first('one', 'two')).toBe('one');
+    expect(second('three', 'four')).toBe('three');
+  });
+
+  it('reuses the compiled function for the same argument names and body', () => {
+    const first = compileExpression(['x'], 'return x;');
+    const second = compileExpression(['x'], 'return x;');
+
+    expect(first).toBe(second);
+  });
+});

packages/v4/migration/expression.ts

@@ -0,0 +1,42 @@
+/**
+ * The one `new Function` compiler, shared by every family that evaluates an
+ * author's expression — `Action`, `Data` and `Fetch`. It uses `new Function`,
+ * so it requires a Content Security Policy that permits `unsafe-eval`; that
+ * is why the boundary is one file, not a helper each family reinvents.
+ *
+ * `Action` and `Data` used to cache under a `cacheKey` the caller built by
+ * hand — `Data`'s was `code + name`, under which `('bc', 'return "A"')` and
+ * `('c', 'return "A"b')` land in the same entry. There is no `cacheKey`
+ * parameter here: the key is the argument list and the body, kept apart in
+ * two map levels rather than joined into one string, so nothing a caller
+ * writes can collide with it.
+ */
+
+export type CompiledExpression = (...args: unknown[]) => unknown;
+
+const cache = new Map<string, Map<string, CompiledExpression>>();
+
+/**
+ * Compile `body` into a function taking `argNames`, memoised by both.
+ *
+ * @param argNames The parameter names `body` may reference, in order.
+ * @param body The function body, for example `return target.open();`.
+ */
+export function compileExpression(argNames: readonly string[], body: string): CompiledExpression {
+  const argsKey = argNames.join(',');
+  let byBody = cache.get(argsKey);
+  if (!byBody) {
+    byBody = new Map();
+    cache.set(argsKey, byBody);
+  }
+
+  let compiled = byBody.get(body);
+  if (!compiled) {
+    // Compiling the author's expression is the whole feature.
+    // oxlint-disable-next-line no-new-func, typescript/no-implied-eval
+    compiled = new Function(...argNames, body) as CompiledExpression;
+    byBody.set(body, compiled);
+  }
+
+  return compiled;
+}

packages/v4/migration/index.ts

@@ -19,3 +19,6 @@ export * from './Transition/index.js';
 
 /** Shared vocabulary, not a family: what the parts of a declaration mean. */
 export * from './event-modifiers.js';
+
+/** Shared primitive, not a family: the one `new Function` compiler. */
+export * from './expression.js';