Repository navigation
Commit 29b9949
fix(plugin-grid,fields,app-shell): a promoted bulk action's field-backed param resolves through the shared action-param resolver (objectui#12105) (#12119)
Fixes #12105
Clause-②: yes
A promoted bulk action's field-backed param (`{ field, objectOverride
}`) now resolves through the same resolver as the single-record path, so
the selection bar's dialog renders the record picker and the confirm
step shows the record's label. One resolver, shared: it moved out of
`app-shell` into `@object-ui/fields`. There is no second copy.
Implemented by `os-dev` for the `domain:ui` seat, session
`https://claude.ai/code/session_01AswpQDLCKiZos2jCXknwKz`, branch
`claude/issue-12105-bulk-param-field-resolve`.
## The defect, reproduced red on `main` `a21ff9a`
A throwaway probe against `main`'s source, with HotCRM's shape (`params:
[{ field: 'crm_campaign', objectOverride: 'crm_campaign_member' }]`):
- `resolveBulkActions` produced
`{"field":"crm_campaign","objectOverride":"crm_campaign_member","required":true,"name":"crm_campaign","type":"text"}`.
- `bulkParamToField` turned that into a `text` widget.
- `BulkActionDialog` rendered `input#bulk-param-crm_campaign` with
`type="text"`.
The card's premise holds. `toBulkParam` renamed `field` onto `name`,
defaulted `type` to `'text'`, and read `object` only off an inline
`reference`. It never looked at the field definition.
## What changed, and why each package is touched
- **`@object-ui/fields` receives the resolver.**
`resolveActionParams.ts` moved here with `git mv`, unchanged in
behaviour, together with the three-row param-type alias table and
`resolveParamWidgetType`. The index exports them (list under Clause-②
below). The one other edit to the moved file is a module-scoped type for
the bundler-replaced `process.env.NODE_ENV` read. This package has no
node typings. The runtime read is the same bytes it was in app-shell,
and nothing ambient is declared, so it cannot shadow node's global
anywhere.
- **`@object-ui/app-shell` now imports the resolver from
`@object-ui/fields`.** Its two callers changed
(`useConsoleActionRuntime`, `RecordDetailView`), and `paramToField` /
`ActionPreview` now read `resolveParamWidgetType` from fields. The
resolver was never exported from app-shell, so its public face is
unchanged. Most of the files counted under app-shell are tests whose
import line moved. Two tests key on the file's path: the
`INHERITED_TARGET` citation pin and core's column-identity ratchet. Both
were repointed.
- **`@object-ui/plugin-grid` is the fix.**
- `resolveBulkActions.ts`: the fold no longer attaches promoted
`params`. A new `resolvePromotedBulkParams(def, ctx)` runs
`resolveActionParams` and renames the resolved param onto the bulk
vocabulary (`helpText` → `help`, `defaultValue` → `default`,
`referenceTo` → `object`, `displayField` → `labelField`, plus
`dependsOn`, `accept` and `maxSize`). It contains no field lookup of its
own.
- `ObjectGrid.tsx`: `dispatchBulkActionDef` resolves when the dialog
opens. It first awaits `useMetadata().ensureType('object')`, then unions
the grid's own object schema with the session's objects
(`withKnownObjects`, the grid's first). That is the record dialog's
order, so an `objectOverride` target that has not loaded yet is waited
for, not refused.
- `BulkActionDialog.tsx`: a param the resolver could not resolve
(`unresolvedField`) is refused before any widget is built, and Next is
disabled. This matches `ActionParamDialog` and reuses its
`actionDialog.unresolvedParam` copy. Without it, the resolver's own
warning, "The action dialog refuses the param", would be false on the
bulk path, and the missing-field case would still draw a raw text box.
The key travels on a plugin-grid-local reader (`unresolvedFieldOf`), not
as a declared `BulkActionParam` member, because that interface is also
the authoring face (AGENTS.md #0.1).
- The stale "left open there" sentence in the dialog's `dependsOn` note
now says the promoted route exists.
- **`@object-ui/core`: comments only.** Four comments named the
resolver's old home (`expand-fields.ts`, `predicate-fields.ts`,
`reference-keys.ts`, one test). The column-identity ratchet total goes
from 12 to 11, because the hand-mapper's `name || field` read is gone.
- **Docs:** the `fields` README has a new "Resolving declared action
params" section. `plugin-grid.mdx` and `enhanced-actions.mdx` each gain
a short paragraph. Changeset:
`.changeset/12105-bulk-param-field-resolve.md`.
## Why the resolver moved instead of being injected
I measured what the resolver imports before choosing. It reads
`@object-ui/core` (`EXPANDABLE_FIELD_TYPES` and the `ActionParamDef` /
`ActionParamOption` types), the fields widget fold
(`resolveFormWidgetType`, through `resolveParamWidgetType`) and
`@objectstack/spec/ui` (`resolveI18nLabel`). Its only app-shell-local
import was `paramToField.ts`'s `resolveParamWidgetType`, a thin wrapper
over the fields fold plus a three-row alias table. plugin-grid already
depends on all three of those packages. `@object-ui/fields` is the one
package both sides depend on that holds all of them. `@object-ui/core`
cannot host the resolver, because it cannot import the fields widget
map.
- **Move (chosen).** It adds 5 values and 3 types to
`@object-ui/fields`, and nothing to plugin-grid's or app-shell's
published face. Every host gets the fix, including a grid rendered
without the console.
- **Inject (rejected).** It would add a context, a provider and a value
type to plugin-grid, plus a provider mount in app-shell. That is fewer
exports, but the fix would depend on the host. A grid with no provider
would still need some answer for a field-backed param, and the old
`toBulkParam` mapping is that answer. That leaves two resolution
behaviours, and the one a host gets by forgetting the provider is the
defect this card reports.
- **The cost of the move.** `fields/src/resolveActionParams.ts` imports
`resolveFormWidgetType` from the package barrel that re-exports it. That
is a cycle in the source graph only, and only at call time: nothing at
the module's top level reads it, and the published bundle is one file.
The fields build printed no circular-dependency warning, and
`check:node-esm-load` loads the built entry.
## Spec guidance sentence for the objectstack follow-up
Per the card, objectstack is not edited here. Quoted verbatim from
objectstack `origin/main` `7098acaef`,
`packages/spec/src/ui/bulk-action.zod.ts`, `BulkActionParamSchema`'s
`guidance.field`:
> `field` declares a FIELD-BACKED param, and the bulk surface has no
such route: `resolveActionParams` consults the object's field
definitions for the single-record dialog, `resolveBulkActions`'s
`toBulkParam` never does. Declare the param inline instead — `name` +
`type`, plus `object` (and optionally `labelField`) for a picker.
After this PR the clause "`toBulkParam` never does" is false for a
promoted action. An action declared on the object and named in a view's
`bulkActions` now resolves its field-backed params. An authored
`bulkActionDefs[].params` entry still has no field-backed route, so the
refusal stays right and only its reason and remedy change. The remedy
could be: "or declare the action on the object with a field-backed param
and name it in `bulkActions`". Three more sentences in the same file
make the same claim:
- the `BULK_PARAM_WIDGET_CONFIG_KEYS` prescription: "⛔ Declaring the key
on the object's FIELD does not reach this dialog either: the bulk
surface has no field-backed param route."
- the JSDoc above that constant: "There is no field-backed param route
on the bulk surface — `toBulkParam` never consults the object's field
definitions"
- the `dependsOn` JSDoc: "the FIELD-BACKED route (`resolveActionParams`
resolves the object's field definitions), which is the very route the
bulk surface does not have."
For the widget-config one, note that a promoted lookup param receives
only `object`, `labelField` and `dependsOn` from its field so far (see
Acceptance notes).
## Tests
New file:
`packages/plugin-grid/src/__tests__/bulkFieldBackedParam-12105.test.tsx`,
8 cases.
- **Pin:** a field-backed lookup param on the bulk path. Unit level: it
resolves to `type: 'lookup'`, `object: 'crm_campaign'`, `labelField:
'name'`, and `bulkParamToField` builds the picker. A parity case shows
the same answer as `resolveActionParam` for the same declaration.
- **Pin, through the real `ObjectGrid` + `BulkActionDialog`:**
`lookup-trigger-crm_campaign` renders and
`input#bulk-param-crm_campaign` does not. The confirm step shows "Spring
Launch" (via `findOne('crm_campaign', 'c1')`). Run sends `crm_campaign:
'c1'` once per selected record. The metadata store loads late, so the
case also pins the `ensureType('object')` wait.
- **CONTROL:** a literal-typed param is unchanged at unit level (same
widget, label, help, default, placeholder; an inline `reference:
'sys_user'` still promotes to the person picker) and at grid level
(`input#bulk-param-note` with `type="text"`).
- **Refusal:** a field-backed param whose field is missing shows
`bulk-param-unresolved-crm_campaign` with the `OBJECT.FIELD` locator,
and Next is disabled.
- `resolveBulkActions.test.ts`'s key-mapping case now asserts through
`resolvePromotedBulkParams`.
Results:
- **Affected suites at `ef07e4e`:** `pnpm exec vitest run` over 47 files
(plugin-grid bulk suites, app-shell `resolveActionParams*` /
`paramToField*` / `ActionParamDialog*` / `ActionPreview*` / the identity
and twin pins, core's ratchet and `ActionParamDef.options`, and
`one-authority-per-exported-name-6273`): `Test Files 47 passed (47)`,
`Tests 510 passed (510)`. The commits after it change typing, comments,
docs and the changeset. The rerun at the final head is in the
`os-dev-report` on the card.
- **Ablation A2,** resolver starved of metadata
(`resolveActionParams(raw, { ...ctx, objects: [] })`): the three
field-backed pins went red, and both CONTROLs, the refusal case and the
authored-def case stayed green. The mutation was proven on disk (anchor
1 → 0, blob `e2e6b01a` → `e770d6fb`), and so was the restore (blob equal
to HEAD, `git diff HEAD` empty).
- **Ablation A1,** the `ensureType('object')` wait removed from
`ObjectGrid`: only the grid-level picker pin went red (it got the
refusal instead), 7 of 8 green. Restore proven the same way.
- **Reverse type check:** with an unknown key added to the context
`ObjectGrid` hands the resolver, `pnpm --filter @object-ui/plugin-grid
type-check` reported `TS2353 … does not exist in type
'ResolveActionParamsContext'`. That proves plugin-grid reads the rebuilt
fields `.d.ts`. Restored, `git diff HEAD` empty.
- **Type-check** (every output echoes `type-check`):
`@object-ui/fields`, `@object-ui/plugin-grid`, `@object-ui/app-shell`
and `@object-ui/core`, all exit 0, after a turbo build of each closure.
The plugin-grid and fields run was at `d5e2f59`; the app-shell and core
run was on the same code.
## Gates, all exit 0 at `5c915e9`
- **Spec, deps and imports:** `check:spec-symbols`,
`check:handler-key-reads`, `check:action-forward-parity`,
`check:phantom-deps`, `check:unused-deps`, `check:self-import`,
`check:esm-specifiers` (specifiers-only, as the root script runs it),
`check:unreferenced-sources`.
- **Line citations and bytes:** `check:new-line-citations` (`VERDICT
new-cross-file-line-citations: 0 new citation(s)`),
`check-control-bytes`.
- **Changesets:** `check-changeset-presence` ("30 source file(s) of 4
released package(s) changed, and this change declares 1 changeset(s)"),
`check-changeset-no-major`, `check-changeset-fixed`,
`check:changeset-claims`, `check:pending-changeset-literals`.
- **Tests and mocks:** `check:test-path-roots`, the three
`check:vi-mock-*`, `check-type-check-coverage`.
- **i18n:** `check:i18n-keys`. It confirms that the reused
`actionDialog.unresolvedParam` default matches the en pack. No locale
copy changed, so the other i18n checks were not run.
- **Docs:** `check:readme-exports`, `check-doc-snippet-types`,
`check-doc-example-types`, `check-doc-component-types`,
`check:doc-example-readers`, `check:doc-fences`,
`check-doc-example-ids`, `check-doc-links`.
- **Dist and budget:**
- `check:published-dist`.
- `check:node-esm-load --force-build` at `5c915e9`: "37 of 37 gradable
entries were built by this tree". A first run without `--force-build`
was refused on provenance alone, because 6 untouched packages were
replayed from a sibling worktree's shared turbo cache.
- `check:eager-closure` at head: "Console eager closure is 3170.6 KB
gzipped across 290 of 2474 chunks (budget: 3204.6 KB, headroom: 34.0
KB)". The base-to-head delta is in the `os-dev-report`.
- **Lint, narrowed:**
- Population: `eslint.config.js` applies its rules to `**/*.{ts,tsx}`.
All 30 changed code files are in that population, and none came back as
ignored.
- Count: `eslint --format json` over those 30 files reported 30 results,
0 errors.
- Invariance: the config enables no type-aware linting (no
`parserOptions.project` or `projectService`), and no rule under
`eslint-rules/` reads the disk. So this diff cannot change a verdict on
a file it did not touch.
- With `--no-inline-config` the same run shows 4 errors. They sit
exactly on the 4 pre-existing `eslint-disable-next-line
react-hooks/static-components` comments in these files. Repo-wide `pnpm
lint` is CI's.
## Clause-② surface added
New exports from `@object-ui/fields`:
- values: `resolveActionParam`, `resolveActionParams`,
`resolveParamWidgetType`, `withKnownObjects`, `RESOLVED_ONLY_PARAM_KEYS`
- types: `RawActionParam`, `RawActionParamOption`,
`ResolveActionParamsContext`
Before this PR they were internal to app-shell (not exported from it).
They are now exported from fields' package entry.
Nothing is added to plugin-grid's package entry.
`resolvePromotedBulkParams` and `unresolvedFieldOf` are module exports
of `resolveBulkActions.ts`, which the index does not re-export. No prop,
context member or locale key is added. The bulk dialog reuses the
existing `actionDialog.unresolvedParam` key.
## Acceptance notes (noted, not filed)
- **The bulk adapter forwards only part of a lookup's picker config.** A
resolved lookup param reaches the bulk picker with only `object`,
`labelField` and `dependsOn`. `lookupFilters`, `lookupColumns`,
`lookupPageSize`, `descriptionField`, `idField` and `titleFormat` are
resolved but not forwarded. `bulkParamToField` has no rename for them,
while app-shell's `paramToField` does. Converging the two param adapters
is its own change. The changeset and the docs say so. Carrier: whoever
next converges `bulkParamToField` with `paramToField`.
- **Bulk param labels skip `_actions.ACTION.params.PARAM`
localization.** The record dialog runs that pass after resolution
(`actionParamText`), and the bulk path never had it. This is a gap from
before this PR, not something it introduces. Carrier: none.
- **The bulk dialog's `dependsOn` note is still stale elsewhere.** It
says `BulkActionParamSchema` "is not strict", but the spec has since
closed that shape and declares `dependsOn`. Only the sentence this PR
made false was edited. Carrier: none.
- **`BULK_PARAM_TYPE_ALIASES` in `bulkParamToField.ts` duplicates the
param alias table.** The table now lives in fields beside
`resolveParamWidgetType`. Not touched, because it is a different defect
class. Carrier: the same as the first note.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01AswpQDLCKiZos2jCXknwKz)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent b7cd5c8 commit 29b9949
34 files changed
Lines changed: 805 additions & 109 deletions
File tree
- .changeset
- content/docs
- core
- plugins
- packages
- app-shell/src
- hooks
- utils
- __tests__
- views
- metadata-admin
- previews
- __tests__
- core/src/utils
- __tests__
- fields
- src
- plugin-grid/src
- __tests__
- components
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
153 | 153 | | |
154 | 154 | | |
155 | 155 | | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
156 | 161 | | |
157 | 162 | | |
158 | 163 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
579 | 579 | | |
580 | 580 | | |
581 | 581 | | |
| 582 | + | |
| 583 | + | |
| 584 | + | |
| 585 | + | |
| 586 | + | |
| 587 | + | |
| 588 | + | |
| 589 | + | |
| 590 | + | |
582 | 591 | | |
583 | 592 | | |
584 | 593 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
49 | | - | |
| 49 | + | |
50 | 50 | | |
51 | 51 | | |
52 | 52 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
51 | 51 | | |
52 | 52 | | |
53 | 53 | | |
54 | | - | |
| 54 | + | |
55 | 55 | | |
56 | 56 | | |
57 | 57 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
30 | | - | |
| 30 | + | |
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
60 | 60 | | |
61 | 61 | | |
62 | 62 | | |
63 | | - | |
| 63 | + | |
64 | 64 | | |
65 | 65 | | |
66 | 66 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
21 | | - | |
| 21 | + | |
| 22 | + | |
22 | 23 | | |
23 | 24 | | |
24 | 25 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
16 | | - | |
17 | | - | |
18 | | - | |
19 | | - | |
20 | | - | |
21 | | - | |
22 | | - | |
23 | | - | |
24 | | - | |
25 | | - | |
26 | | - | |
27 | | - | |
28 | | - | |
29 | | - | |
30 | | - | |
31 | | - | |
32 | | - | |
33 | | - | |
34 | | - | |
35 | | - | |
36 | | - | |
37 | | - | |
38 | | - | |
39 | | - | |
40 | | - | |
41 | | - | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
42 | 21 | | |
43 | 22 | | |
44 | 23 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
56 | 56 | | |
57 | 57 | | |
58 | 58 | | |
59 | | - | |
| 59 | + | |
60 | 60 | | |
61 | 61 | | |
62 | 62 | | |
| |||
0 commit comments