diff --git a/.changeset/11638-member-properties-params-retired.md b/.changeset/11638-member-properties-params-retired.md new file mode 100644 index 0000000000..464e29cbc5 --- /dev/null +++ b/.changeset/11638-member-properties-params-retired.md @@ -0,0 +1,32 @@ +--- +'@object-ui/components': minor +--- + +`action:group` and `action:menu` no longer read a member's `properties.params` (objectui#11638). A container member's `properties.params` no longer reaches the action runner. + +**Why.** `@objectstack/spec` refuses a `properties` key on an `action:group` / `action:menu` member, with this prescription: "A member carries no `properties` bag: its static parameter values (`properties.params`) are not part of the inline action vocabulary. For a `type: 'api'` member's request body write `bodyExtra`; to run an action with static parameter values, author it as its own `action:button` node, whose `params` object carries them." The census behind that ruling found no writer of the key. The renderers kept a read the spec refuses, so the read is retired. + +**Behaviour change**, shipped as `minor` per this repository's version policy: + +- A member's `properties.params` is not forwarded as the runner's `params`, and it is no longer template-evaluated. This holds for an `action:group` member (inline and dropdown), an `action:menu` item, and an `action:bar` member that lands in the overflow menu. +- A member whose `params` is an array forwards it as `actionParams` alone. The runner's `params` then carries only what the user answers in the parameter dialog. +- A `type: 'api'` member's object `params` is its request payload (the objectstack#5777 window), and a `properties.params` beside it no longer replaces that payload. +- Unchanged: an `action:button` / `action:icon` node reads its static values from `properties.params` as before, and so does an `action:bar` member drawn inline, which the bar mounts on one of those two renderers. + +**FROM** a container member carrying static values: + +```json +{ "type": "action:group", "actions": [ + { "name": "edit", "label": "Edit", "type": "navigate_edit", "properties": { "params": { "recordId": "${record.id}" } } } +] } +``` + +**TO** its own `action:button` node: + +```json +{ "type": "action:button", "properties": { "label": "Edit", "actionType": "navigate_edit", "params": { "recordId": "${record.id}" } } } +``` + +For a `type: 'api'` member's request body, write `bodyExtra` on the member. + +**Clause-②: no.** No export, prop, type member or i18n key is added or removed. The retired helper lived in a module that `@object-ui/components` does not re-export from its entry. diff --git a/packages/components/src/renderers/action/__tests__/action-container-member-params-10290.test.tsx b/packages/components/src/renderers/action/__tests__/action-container-member-params-10290.test.tsx index 3864f5222a..14538e1f26 100644 --- a/packages/components/src/renderers/action/__tests__/action-container-member-params-10290.test.tsx +++ b/packages/components/src/renderers/action/__tests__/action-container-member-params-10290.test.tsx @@ -7,31 +7,46 @@ */ /** - * objectui#10290: a member of an action CONTAINER (`action:bar`, - * `action:group`, `action:menu`) carries its static execution values in - * `properties.params` (objectui#10289, ruling A), and those values are - * templates evaluated where `properties` are (objectui#7867, ruling A). A - * container member never passes through the `SchemaRenderer` evaluation memo, - * so each container evaluates its members' `properties` through the evaluator - * that memo uses, against the scope that memo builds. + * objectui#10290, as narrowed by objectui#11638. + * + * objectui#10290 made every action CONTAINER (`action:bar`, `action:group`, + * `action:menu`) evaluate a member's `properties.params` where `properties` + * are (objectui#7867, ruling A), because a container member never passes + * through the `SchemaRenderer` evaluation memo. objectui#11638 retired the + * MEMBER half of that: the spec refuses a `properties` bag on an + * `action:group` / `action:menu` member ("A member carries no `properties` + * bag: its static parameter values (`properties.params`) are not part of the + * inline action vocabulary"), and an action that needs static values is its + * own `action:button` node. So the paths split by WHICH reader the member + * reaches: + * + * - NODE path: `action:bar` mounts an inline member on `action:button` / + * `action:icon`, whose own node reader (`readStaticParamValues`) reads + * `properties.params`. The bar still evaluates it first (objectui#10290); + * those pins are unchanged. + * - MEMBER path: `action:group` / `action:menu` run the member themselves, + * and so does an `action:bar` member that lands in the overflow menu. A + * member's `properties.params` reaches the runner on none of them + * (objectui#11638). These are the objectui#10290 pins INVERTED: each used + * to assert the resolved values arrive. * * Driven end to end through the real pieces: the real `SchemaRenderer` renders * the real container, the click or menu selection goes through the real - * `ActionRunner`, and the value asserted is the `params` on the `ActionDef` the - * registered `navigate_edit` handler receives. The row is bound the way a - * record page binds it, through `RecordContextProvider`. + * `ActionRunner`, and the value asserted is the `ActionDef` the registered + * handler receives. The row is bound the way a record page binds it, through + * `RecordContextProvider`. * * The CONTROL is a top-level `action:button` with the same `properties.params`. - * It goes through the `SchemaRenderer` memo, so it resolved before this change - * too: a red container row beside a green control means the container, never - * an unbound `record` root. + * It goes through the `SchemaRenderer` memo, so it resolves on every path: a + * member row that differs from a green control means the container, never an + * unbound `record` root. */ import { describe, it, expect, vi, beforeEach, afterEach, type Mock } from 'vitest'; import { render, screen, fireEvent, waitFor, within } from '@testing-library/react'; import '@testing-library/jest-dom'; import React from 'react'; -import type { ActionContext, ActionDef, ActionResult } from '@object-ui/core'; +import type { ActionContext, ActionDef, ActionResult, ParamCollectionHandler } from '@object-ui/core'; // These nodes are written the way the runtime reads them, flat on the node, // which the closed `action:*` node types refuse: measured on objectui#11466, // typing the fixtures as `DeclaredNode` refuses them line by line. So each @@ -57,15 +72,30 @@ const AUTHORED = { objectName: 'account', recordId: '${record.id}' }; /** What the handler must receive once the template is evaluated. */ const RESOLVED = { objectName: 'account', recordId: 'rec_1' }; +/** An `api` member's object `params`: its request payload (objectstack#5777 window). */ +const PAYLOAD = { objectName: 'account', recordId: 'rec_7' }; + +/** An `ActionParam[]` input list, and what the user answers it with. */ +const INPUTS = [{ name: 'reason', type: 'text', label: 'Reason' }]; +const COLLECTED = { reason: 'because' }; + /** The overflow trigger's accessible name, with no i18n bundle loaded. */ const MORE = 'More actions'; -let navigateEdit: Mock<(action: ActionDef, ctx: ActionContext) => Promise>; +type Handler = Mock<(action: ActionDef, ctx: ActionContext) => Promise>; + +let navigateEdit: Handler; +let api: Handler; +let onParamCollection: Mock; let warn: ReturnType; let consoleError: ReturnType; beforeEach(() => { navigateEdit = vi.fn(async () => ({ success: true })); + api = vi.fn(async () => ({ success: true })); + // The user fills the one input in; the handler then receives what was + // collected, merged over whatever static `params` the container forwarded. + onParamCollection = vi.fn(async () => COLLECTED); warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); resetStaticParamsWarnings(); @@ -84,7 +114,7 @@ function member(extra: Record = {}): Record { /** Render `schema` on a record page bound to {@link ROW}. */ function renderOnRecordPage(schema: Record) { return render( - + @@ -110,12 +140,17 @@ const viaTrigger = (triggerName: string): Reach => async (label) => { await selectFromMenu(screen.getByRole('button', { name: triggerName }), label); }; -/** Every path a container member reaches the runner by. */ -const CONTAINERS: ReadonlyArray<{ +type Container = { path: string; schema: (m: Record) => Record; reach: Reach; -}> = [ +}; + +/** + * NODE path: `action:bar` mounts the member on `action:button` / + * `action:icon`, whose node reader reads `properties.params`. + */ +const NODE_PATH: ReadonlyArray = [ { path: 'action:bar, inline member', schema: (m) => ({ type: 'action:bar', actions: [m] }), @@ -131,6 +166,13 @@ const CONTAINERS: ReadonlyArray<{ schema: (m) => ({ type: 'action:bar', actions: [{ ...m, component: 'action:group' }] }), reach: clickButton, }, +]; + +/** + * MEMBER path: the container runs the member itself (`action:group`, + * `action:menu`, and the `action:menu` an `action:bar` overflows into). + */ +const MEMBER_PATH: ReadonlyArray = [ { path: 'action:bar, member placed in the overflow menu (`component: action:menu`)', schema: (m) => ({ type: 'action:bar', actions: [{ ...m, component: 'action:menu' }] }), @@ -163,15 +205,27 @@ const CONTAINERS: ReadonlyArray<{ }, ]; -/** Mount, reach the member, return the `params` the handler received. */ -async function paramsReceived(schema: Record, reach: Reach): Promise { +/** Every path a container member reaches the runner by. */ +const CONTAINERS: ReadonlyArray = [...NODE_PATH, ...MEMBER_PATH]; + +/** Mount, reach the member, return the `ActionDef` `handler` received. */ +async function defReceived( + schema: Record, + reach: Reach, + handler: Handler = navigateEdit, +): Promise { renderOnRecordPage(schema); await reach('Edit'); - await waitFor(() => expect(navigateEdit).toHaveBeenCalledTimes(1)); - return (navigateEdit.mock.calls[0][0] as ActionDef).params; + await waitFor(() => expect(handler).toHaveBeenCalledTimes(1)); + return handler.mock.calls[0][0] as ActionDef; } -describe('objectui#10290 - a container member\'s `properties.params` is evaluated where `properties` are', () => { +/** Mount, reach the member, return the `params` the handler received. */ +async function paramsReceived(schema: Record, reach: Reach): Promise { + return (await defReceived(schema, reach)).params; +} + +describe('objectui#10290 - an `action:bar` member\'s `properties.params` is evaluated where `properties` are', () => { it('CONTROL: a top-level `action:button` resolves `${record.id}` through the SchemaRenderer memo', async () => { const params = await paramsReceived( { @@ -186,12 +240,15 @@ describe('objectui#10290 - a container member\'s `properties.params` is evaluate expect(params).toEqual(RESOLVED); }); - describe.each(CONTAINERS)('$path', ({ schema, reach }) => { + describe.each(NODE_PATH)('$path', ({ schema, reach }) => { it('the handler receives `properties.params` with `${record.id}` resolved', async () => { const params = await paramsReceived(schema(member({ properties: { params: AUTHORED } })), reach); expect(params).toEqual(RESOLVED); }); + }); + // On every path, node and member alike. + describe.each(CONTAINERS)('$path', ({ schema, reach }) => { it('a node-level `params` OBJECT is still not a values channel, and says so once (objectui#10289)', async () => { const params = await paramsReceived(schema(member({ params: AUTHORED })), reach); expect(params).toBeUndefined(); @@ -202,3 +259,36 @@ describe('objectui#10290 - a container member\'s `properties.params` is evaluate }); }); }); + +describe('objectui#11638 - a container member\'s `properties.params` does not reach the runner', () => { + describe.each(MEMBER_PATH)('$path', ({ schema, reach }) => { + // INVERTED from objectui#10290's "the handler receives `properties.params` + // with `${record.id}` resolved" on this path. + it('a member\'s `properties.params` is not forwarded as static values', async () => { + const params = await paramsReceived(schema(member({ properties: { params: AUTHORED } })), reach); + expect(params).toBeUndefined(); + }); + + it('an array `params` reaches the runner as `actionParams` alone, beside a `properties.params`', async () => { + const def = await defReceived( + schema(member({ params: INPUTS, properties: { params: AUTHORED } })), + reach, + ); + expect(onParamCollection).toHaveBeenCalledTimes(1); + expect(onParamCollection.mock.calls[0][0]).toEqual(INPUTS); + expect(def.actionParams).toEqual(INPUTS); + // Only what the user answered: no static values were forwarded to merge + // the answer over. + expect(def.params).toEqual(COLLECTED); + }); + + it('an `api` member\'s object `params` is its payload, and a `properties.params` beside it no longer replaces it', async () => { + const def = await defReceived( + schema(member({ type: 'api', target: '/api/v1/ping', params: PAYLOAD, properties: { params: AUTHORED } })), + reach, + api, + ); + expect(def.params).toEqual(PAYLOAD); + }); + }); +}); diff --git a/packages/components/src/renderers/action/action-bar.tsx b/packages/components/src/renderers/action/action-bar.tsx index 2bb2e4c750..ad9d089b67 100644 --- a/packages/components/src/renderers/action/action-bar.tsx +++ b/packages/components/src/renderers/action/action-bar.tsx @@ -306,8 +306,8 @@ const ActionBarRenderer = forwardRef { const Renderer = ComponentRegistry.get(componentType); if (!Renderer) return null; diff --git a/packages/components/src/renderers/action/action-group.tsx b/packages/components/src/renderers/action/action-group.tsx index 31235ea711..6aa3c366a5 100644 --- a/packages/components/src/renderers/action/action-group.tsx +++ b/packages/components/src/renderers/action/action-group.tsx @@ -22,7 +22,7 @@ import type { ActionDef } from '@object-ui/core'; import type { UIActionSchema, ActionLocation } from '@object-ui/types'; import { ACTION_LOCATIONS, actionRendersAt } from '@object-ui/types'; import { useAction } from '@object-ui/react'; -import { useCondition, toPredicateInput, usePredicateRecordContext, useConfigBagEvaluator } from '@object-ui/react'; +import { useCondition, toPredicateInput, usePredicateRecordContext } from '@object-ui/react'; import { Button } from '../../ui'; import { DropdownMenu, @@ -35,7 +35,7 @@ import { cn } from '../../lib/utils'; import { Loader2, ChevronDown } from 'lucide-react'; import { resolveIcon } from './resolve-icon'; import { hasDeclaredVisibilityGate } from './visibility-gate'; -import { readActionEntryParamValues, readMemberStaticParamValues } from './static-params'; +import { readActionEntryParamValues } from './static-params'; // No group-level `name` (objectui#11168): nothing renders, forwards or keys on // it, and `@objectstack/spec` refuses it on this block, so the registration @@ -276,9 +276,6 @@ const ActionGroupRenderer = forwardRef unknown; /** - * A container MEMBER with its `properties` evaluated (objectui#10290). + * An `action:bar` member with its `properties` evaluated (objectui#10290). * - * `action:bar`, `action:group` and `action:menu` draw their member actions - * themselves, so a member never passes through the `SchemaRenderer` memo that - * evaluates a node's `properties`. Its static values ride `properties.params` - * (objectui#10289), and they are templates (objectui#7867): on a record page, + * `action:bar` mounts its inline members on `action:button` / `action:icon` + * itself, so a member never passes through the `SchemaRenderer` memo that + * evaluates a node's `properties`. The renderer it lands on reads the static + * values off `properties.params` ({@link readStaticParamValues}, + * objectui#10289), and they are templates (objectui#7867): on a record page, * `"recordId": "${record.id}"` has to reach the handler as the record's id. The - * container evaluates the member's `properties` with the memo's own evaluator - * and scope (`evaluateBag`, from `useConfigBagEvaluator()`), once, where it - * composes or runs the member. + * bar evaluates the member's `properties` with the memo's own evaluator and + * scope (`evaluateBag`, from `useConfigBagEvaluator()`), once, where it + * composes the member. * * Returns the member itself when it carries no `properties` bag. Only * `properties` is evaluated. It is not copied onto the member (the memo's * hoist), and the member's other keys are left as authored. + * + * `action:group` and `action:menu` do not call this: their members carry no + * `properties` bag (objectui#11638). */ export function withEvaluatedProperties( member: T, @@ -130,34 +141,15 @@ export function withEvaluatedProperties( return isConfigBag(properties) ? { ...member, properties: evaluateBag(properties) } : member; } -/** - * A container member's static values (objectui#10290): its `properties.params`, - * evaluated through {@link withEvaluatedProperties}, or `undefined`. - * - * For `action:group` / `action:menu` items, which run the member themselves. - * (`action:bar` hands an inline member to `action:button` / `action:icon` with - * its `properties` already evaluated, and `readStaticParamValues` reads it - * there.) - */ -export function readMemberStaticParamValues( - member: StaticParamsSubject, - evaluateBag: ConfigBagEvaluator, -): Record | undefined { - if (!propertiesParams(member)) return undefined; - return propertiesParams(withEvaluatedProperties(member, evaluateBag)); -} - /** * The same ruling on a spec ACTION ENTRY (objectui#10462): `element:button`'s * inline `action`, `action:group` / `action:menu` items and `page:header`'s * actions. `params` is an entry's `ActionParam[]` input list and nothing else. - * (An `action:group` / `action:menu` item is also a container member, so its - * static values are read from `properties.params` and evaluated by the - * container through {@link readMemberStaticParamValues} (objectui#10290). - * `UIActionSchema` declares no `properties`, so on a container member this bag - * is read off the authored object, not off a declared key. `element:button`'s - * inline `action` and `page:header`'s actions do not read - * `properties.params`.) + * (No entry surface reads a `properties.params` bag. `UIActionSchema` declares + * no `properties`, and the spec refuses one on an `action:group` / + * `action:menu` member: its static parameter values are not part of the inline + * action vocabulary, and an action that needs them is its own `action:button` + * node (objectui#11638).) * * One type is still different: `api`. The objectstack#5777 window keeps the * runner reading an object `params` as the request payload (with its own diff --git a/packages/react/src/hooks/useConfigBagEvaluator.ts b/packages/react/src/hooks/useConfigBagEvaluator.ts index 8b01c52958..f0ab09e4ff 100644 --- a/packages/react/src/hooks/useConfigBagEvaluator.ts +++ b/packages/react/src/hooks/useConfigBagEvaluator.ts @@ -15,10 +15,11 @@ import { usePageVariables } from './usePageVariables.js'; * The `properties` evaluation of the `SchemaRenderer` memo, for a node that is * rendered WITHOUT `SchemaRenderer` (objectui#10290). * - * The action containers (`action:bar`, `action:group`, `action:menu` in - * `@object-ui/components`) draw their member actions themselves, so a member's - * `properties` never passes through the memo. Its static execution values ride - * `properties.params` (objectui#10289), and those values are templates + * `action:bar` (in `@object-ui/components`) mounts its member actions itself, + * so a member's `properties` never passes through the memo. Its static + * execution values ride `properties.params` (objectui#10289; an `action:group` + * / `action:menu` member carries no `properties` bag and its container reads + * none, objectui#11638), and those values are templates * evaluated where `properties` are (objectui#7867). This hook is that * evaluation: the memo's per-key rule, its `ExpressionEvaluator`, and its * scope (the host's ambient roots, `current_user`, the page's bound row as