fix(react): a config-bag data object carrying a source field reaches the renderer whole

.changeset/10288-config-bag-data-object-source.md

@@ -0,0 +1,20 @@
+---
+'@object-ui/react': minor
+---
+
+fix(react): a data object in a node's `properties` / `props` bag reaches the renderer whole, even when it carries a `source` field
+
+`SchemaRenderer` evaluates each value of a node's config bag (`properties`, and its legacy alias `props`) one by one, and handed every value to `ExpressionEvaluator.evaluate`. That call unwraps ANY object with a string `source` to that bare string before it does anything else. So an authored data object that merely had a field called `source` was silently replaced by that one string: an `action:button` with `properties.bodyExtra: { source: 'web', campaign: 'spring' }` sent `"web"` to the action runner instead of the object, with no error and no warning. The same object written at node level, which no loop visits, always arrived intact.
+
+This was never one key's problem. Every object-valued input on those two bags took the same path, including values keyed by field name: form `defaultValues` / `initialValues`, a declarative `patch`, filter `values` and a detail view's `data`, where a field named `source` is ordinary.
+
+On a key outside the predicate chain, an object now reaches `evaluate` only when it is an Expression envelope in `@objectstack/spec`'s sense, meaning it carries a string `dialect` (the spec's `ExpressionSchema` requires one). Every other object is handed to the renderer exactly as authored.
+
+**Behaviour change, stated plainly.** The only value whose meaning changes is an object on a non-predicate config-bag key that has a string `source` and no string `dialect`. It used to collapse to its evaluated `source`; it now arrives as the object. Everything else evaluates as before:
+
+- strings are still interpolated;
+- a `{ dialect: 'template', source: '${…}' }` envelope still resolves to its value;
+- the predicate keys (`visible`, `visibleWhen`, `visibleOn`, `visibility`, `hidden`, `hiddenOn`, `disabled`, `disabledOn`, `enabled`) are unchanged: a CEL envelope is still preserved for the canonical engine (objectui#9100 / #9107), and a dialect-less `{ source }` on them is still evaluated, the form `ExpressionWire` declares for `visible` / `hidden` / `disabled`;
+- `params` is still walked leaf by leaf (objectui#7867).
+
+Declared as a minor (Clause-②): nothing in the published API is added or removed, but what an authored value evaluates to changes for one shape. `ExpressionEvaluator.evaluate`'s signature admits a dialect-less `{ source }`, and these loops handed it every value except a `params` bag and a CEL envelope on a predicate key. So `{ source: '${…}' }` on a config-bag key outside the predicate chain and `params` now arrives as the object, uninterpolated, and the unevaluated-expression diagnostic does not report the `${…}` inside it. To keep such a value evaluated, write the bare string `'${…}'` or `{ dialect: 'template', source: '${…}' }`; both still interpolate on that path. The spec's envelope always carries `dialect`, and `@object-ui/core`'s `isRuntimeDefault` already reads a dialect-less `{ source }` as a literal value.

packages/components/src/renderers/action/__tests__/action-config-bag-source-key-10288.test.tsx

@@ -0,0 +1,127 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * objectui#10288: an authored data object that carries a `source` field must
+ * reach the runner WHOLE, whichever channel it is written on.
+ *
+ * `SchemaRenderer`'s per-value `properties` loop handed every value to
+ * `ExpressionEvaluator.evaluate`, whose first step unwraps ANY object with a
+ * string `source` to that bare string. So `properties.bodyExtra: { source:
+ * 'web', campaign: 'spring' }` reached the handler as `bodyExtra: "web"`. The
+ * same object written at node level is visited by no loop and always arrived
+ * intact: that is the CONTROL below, and it is what makes a red CASE mean the
+ * `properties` leg rather than an unforwarded key.
+ *
+ * Driven end to end through the REAL pieces, for the reason
+ * `action-params-templates-7867.test.tsx` gives: the real `SchemaRenderer`
+ * renders the real `action:button`, the click goes through the real
+ * `ActionRunner`, and the value asserted is read off the `ActionDef` the
+ * registered handler was handed.
+ *
+ * The predicate-key rows are the objectui#9100 / #9107 envelope protection on
+ * the SAME mount path. The fix narrows what the loop hands to `evaluate` on
+ * NON-predicate keys only, so these rows must stay green with the fix and with
+ * it ablated. Their predicate carries a CEL stdlib call (`has`), which the
+ * legacy engine lacks, so a flattened envelope cannot pass them by accident.
+ */
+
+import { describe, it, expect, vi, beforeEach, afterEach, type Mock } from 'vitest';
+import { render, screen, fireEvent, waitFor, cleanup } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import React from 'react';
+import type { ActionContext, ActionDef, ActionResult } from '@object-ui/core';
+import type { DataSource } from '@object-ui/types';
+import {
+  ActionProvider,
+  PredicateScopeProvider,
+  SchemaRenderer,
+  SchemaRendererProvider,
+} from '@object-ui/react';
+// Module-scope side-effect import so `action:button` is registered before the
+// first render (the light `dom` project does not load the components graph).
+// Module scope, not a `beforeAll`, per AGENTS.md 测试纪律.
+import '../action-button';
+
+/** An authored data object whose `source` is a FIELD, not an expression. */
+const BODY = { source: 'web', campaign: 'spring' };
+
+const DATA = { status: 'draft' };
+
+/** CEL stdlib call + a comparison. `has` does not exist on the legacy engine. */
+const HOLDS = { dialect: 'cel', source: 'has(data.status) && data.status == "draft"' };
+const FAILS = { dialect: 'cel', source: 'has(data.status) && data.status == "published"' };
+
+let probe: Mock<(action: ActionDef, ctx: ActionContext) => Promise<ActionResult>>;
+
+beforeEach(() => {
+  probe = vi.fn(async () => ({ success: true }));
+});
+
+afterEach(cleanup);
+
+/**
+ * Render one `action:button`. `DATA` is bound as the `data` root the way the
+ * sibling envelope pins bind it (see `action-enablement-cel-envelope.test.tsx`
+ * for why a plain object crosses the `DataSource` type here).
+ */
+function mount(node: Record<string, unknown>) {
+  return render(
+    <ActionProvider handlers={{ probe_10288: probe }}>
+      <SchemaRendererProvider dataSource={DATA as unknown as DataSource}>
+        <PredicateScopeProvider scope={{ data: DATA }}>
+          <SchemaRenderer
+            schema={{ type: 'action:button', label: 'Send', actionType: 'probe_10288', ...node } as never}
+          />
+        </PredicateScopeProvider>
+      </SchemaRendererProvider>
+    </ActionProvider>,
+  );
+}
+
+/** Click the one button and return the `ActionDef` the handler received. */
+async function defReceived(): Promise<ActionDef> {
+  fireEvent.click(screen.getByRole('button', { name: 'Send' }));
+  await waitFor(() => expect(probe).toHaveBeenCalledTimes(1));
+  return probe.mock.calls[0][0] as ActionDef;
+}
+
+describe('objectui#10288: a data object carrying `source` reaches the runner whole', () => {
+  it('CONTROL: node-level `bodyExtra` arrives intact (no loop visits it)', async () => {
+    mount({ bodyExtra: BODY });
+    expect((await defReceived()).bodyExtra).toEqual(BODY);
+  });
+
+  it('CASE: `properties.bodyExtra` arrives intact, not collapsed to its `source` string', async () => {
+    mount({ properties: { bodyExtra: BODY } });
+    const def = await defReceived();
+    expect(def.bodyExtra).toEqual(BODY);
+    // The collapsed spelling, named so a red run reads as the defect itself.
+    expect(def.bodyExtra).not.toBe('web');
+  });
+});
+
+describe('objectui#9100 / #9107 CONTROL: a CEL envelope on a predicate key still gates this button', () => {
+  it('properties.visible: the holding row renders, the failing row does not', () => {
+    mount({ properties: { visible: HOLDS } });
+    const holds = screen.queryByRole('button', { name: 'Send' }) !== null;
+    cleanup();
+    mount({ properties: { visible: FAILS } });
+    const fails = screen.queryByRole('button', { name: 'Send' }) !== null;
+    expect({ holds, fails }).toEqual({ holds: true, fails: false });
+  });
+
+  it('properties.disabled: the holding row is disabled, the failing row is usable', () => {
+    mount({ properties: { disabled: HOLDS } });
+    const holds = (screen.getByRole('button', { name: 'Send' }) as HTMLButtonElement).disabled;
+    cleanup();
+    mount({ properties: { disabled: FAILS } });
+    const fails = (screen.getByRole('button', { name: 'Send' }) as HTMLButtonElement).disabled;
+    expect({ holds, fails }).toEqual({ holds: true, fails: false });
+  });
+});

packages/react/src/SchemaRenderer.tsx

@@ -472,6 +472,70 @@ const preservePredicateEnvelope = (
   evaluate: (v: unknown) => unknown,
 ): unknown => (PREDICATE_CHAIN_KEYS.has(key) && isCelEnvelope(value) ? value : evaluate(value));
 
+/**
+ * Is this value an Expression envelope in the spec's sense: an object carrying
+ * a string `dialect` AND a string `source`? (objectui#10288)
+ *
+ * `@objectstack/spec`'s `ExpressionSchema` declares `dialect` REQUIRED, so every
+ * envelope a spec-parsed artifact carries has one. The shape test is the one
+ * `@object-ui/core` already applies to a declared `defaultValue`
+ * (`isRuntimeDefault`, "Same shape test the engine applies before handing a
+ * default to `ExpressionEngine`"), where an object with a `source` and no
+ * `dialect` is a literal value, not an instruction. {@link isCelEnvelope} is
+ * this question narrowed to one dialect.
+ */
+const isExpressionEnvelope = (value: unknown): boolean =>
+  isConfigBag(value)
+  && typeof (value as { dialect?: unknown }).dialect === 'string'
+  && typeof (value as { source?: unknown }).source === 'string';
+
+/**
+ * Is this config-bag value a DATA OBJECT, which the per-value loops hand to the
+ * renderer exactly as authored instead of passing it to
+ * `ExpressionEvaluator.evaluate`? (objectui#10288)
+ *
+ * ## The defect this closes
+ *
+ * `evaluate` unwraps ANY object with a string `source` to that bare string
+ * before it does anything else (see {@link preservePredicateEnvelope}). The
+ * loops handed it every value, so an authored data object that happened to
+ * carry a `source` field was silently replaced by that one string. Measured
+ * through the real `SchemaRenderer` -> `action:button` -> runner:
+ * `properties.bodyExtra: { source: 'web', campaign: 'spring' }` reached the
+ * handler as `bodyExtra: "web"`, while the same object written at node level
+ * (which no loop visits) arrived intact. So the canonical channel was the
+ * broken one. That pair is pinned end to end in `@object-ui/components`
+ * (`action-config-bag-source-key-10288.test.tsx`), and this guard by
+ * `__tests__/SchemaRenderer.configBagDataObject-10288.test.tsx`.
+ * Every object-valued key a node carries in `properties` / `props`
+ * goes through the same loop, so the defect was never one key's: form
+ * `defaultValues`, a declarative `patch`, filter `values` and a detail view's
+ * `data` are all keyed by FIELD NAME, and a field named `source` is ordinary.
+ *
+ * ## What still reaches `evaluate`
+ *
+ *   - every value that is not an object in {@link isConfigBag}'s sense (a
+ *     string, any other primitive, an array), exactly as before: `evaluate`
+ *     interpolates a string and returns the rest untouched;
+ *   - a spec Expression envelope ({@link isExpressionEnvelope}), so a
+ *     `{ dialect: 'template', source: '${…}' }` value interpolates as it always
+ *     did;
+ *   - every value of a predicate-chain key ({@link PREDICATE_CHAIN_KEYS}),
+ *     unchanged. `BaseSchema.visible` / `.hidden` / `.disabled` declare the
+ *     dialect-less `{ source }` envelope as an authorable wire form
+ *     (`ExpressionWire`, objectui#7530), so on those keys it stays an
+ *     expression, and {@link preservePredicateEnvelope} still holds back a CEL
+ *     envelope (objectui#9100 / #9107).
+ *
+ * So the only value whose meaning changes is an object on a non-predicate key
+ * with a string `source` and no string `dialect`, and that object now arrives
+ * whole. Nothing is walked: a template nested inside a data object stays raw,
+ * as it always did for an object without a `source`. The loops stay per-value
+ * and shallow, which is the radius the unevaluated-expression diagnostic reads.
+ */
+const isDataObjectValue = (key: string, value: unknown): boolean =>
+  !PREDICATE_CHAIN_KEYS.has(key) && isConfigBag(value) && !isExpressionEnvelope(value);
+
 /**
  * Which CONSEQUENCE the diagnostic should print for a faulting predicate on
  * this leg (objectui#6503).
@@ -1218,10 +1282,16 @@ export const SchemaRenderer: ForwardRefExoticComponent<
      * text (that is the evaluator's own contract) and is reported by the
      * unevaluated-expression diagnostic, so a wrong template stays loud.
      */
+    //
+    // A DATA OBJECT on any other key ({@link isDataObjectValue},
+    // objectui#10288) is handed over as authored: `evaluate` would collapse
+    // one that carries a string `source` to that string.
     const evaluateConfigValue = (key: string, value: unknown): unknown =>
       key === PARAMS_KEY && isParamsBag(value)
         ? mapParamsLeaves(value, (leaf) => evaluator.evaluate(leaf))
-        : preservePredicateEnvelope(key, value, (v) => evaluator.evaluate(v as any));
+        : isDataObjectValue(key, value)
+          ? value
+          : preservePredicateEnvelope(key, value, (v) => evaluator.evaluate(v as any));
 
     // Carrier 1 of the `params` rule above: the node-level bag. A non-bag
     // node-level `params` (the `ActionParam[]` definition list, or anything
@@ -1264,6 +1334,12 @@ export const SchemaRenderer: ForwardRefExoticComponent<
     // is evaluated, through {@link evaluateConfigValue} above. Every other key
     // keeps the shallow reading this paragraph describes.
     //
+    // "Passed through" used to have one exception: `evaluate` unwraps an object
+    // with a string `source` to that string, so a data object carrying a
+    // `source` field was collapsed rather than passed. Since objectui#10288 an
+    // object on a non-predicate key reaches `evaluate` only when it is a spec
+    // Expression envelope (it carries a `dialect`); see {@link isDataObjectValue}.
+    //
     // Guarded by {@link isConfigBag}: a degenerate value must not have its shape
     // reinterpreted by an object spread. Non-objects skip evaluation and reach
     // the hoist — which, since objectui#6760, refuses them on its own.