Skip to content

fix(render): honor every ActionBinding field, per-item visible, real element lifecycle and state paths - #1056

Merged
blove merged 2 commits into
mainfrom
blove/render-followups
Sep 8, 2026
Merged

fix(render): honor every ActionBinding field, per-item visible, real element lifecycle and state paths#1056
blove merged 2 commits into
mainfrom
blove/render-followups

Conversation

@blove

@blove blove commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Six defects in @threadplane/render that yesterday's audit had to write into the docs as caveats. Each one has a vitest spec that fails before the change and passes after.

What changed

(a) signalStateStore silently dropped the first path segment. parsePointer filtered out segment 0 unconditionally, so set('count', 1) parsed to the root pointer and replaced the entire state model with 1. A path is now ''/'/' (root) or must start with /; anything else throws an Error naming the path and the required leading slash, in production as well as development — silent data loss is worse than a throw. A rejected update() applies none of its entries.

(b) Element-scope lifecycle events fired for almost nothing. <render-element> gated its mounted/destroyed emits on a truthy lifecycle field that UIElement does not declare, so in practice only the two spec-scope events ever fired and mountCount/lastMountAt on RenderLifecycleService never moved past the spec mount. The gate is gone: every element the renderer mounts emits both events with its elementKey and elementType.

(c) stateChange could not name the path that changed. StateStore.subscribe carries no path, so the event hardcoded path: '/' with the full snapshot as value. signalStateStore() now records its last mutation, exposed as lastChange() on a new exported SignalStateStore extends StateStore interface, and RenderSpecComponent reads it when present. A store from another implementation has no lastChange and keeps the '/' plus snapshot fallback.

(d) Four ActionBinding fields were accepted and never read. confirm, onSuccess, onError and preventDefault are all implemented, mirroring executeAction in @json-render/core:

  • preventDefault calls preventDefault() on the emitted payload when it is a DOM Event (or carries one under event).
  • confirm asks window.confirm(message) through the injected DOCUMENT's default view; a falsy answer skips the handler and both follow-ups. Without a default view there is nobody to ask and the handler proceeds.
  • onSuccess (set / action / navigate) and onError (set / action) dispatch after the handler's returned promise settles, or synchronously for a non-promise result or throw. '$error.message' in an onError set map is replaced by the thrown error's message. With no onError, an error propagates as before.

(e) Action params were spread unresolved. A $state expression inside params reached the handler as a literal { $state: '/selected' }. They now go through resolveElementProps with the element's repeat scope, exactly as element props do; the emit payload still wins on key collisions.

(f) visible was not evaluated on the repeat branch. A repeating element mounted every item regardless of its condition. It is now evaluated once per item in that item's scope, so { $item: 'done', eq: false } and $index conditions hide individual rows.

Specs added

  • libs/render/src/lib/signal-state-store.spec.ts — path validation on get/set/update, the root forms, and lastChange() (set, update, read from a subscriber, no-op writes).
  • libs/render/src/lib/render-events.spec.ts — three-element mount emits one spec-scope plus three element-scope mounted events, an element leaving the tree emits destroyed, mountCount reaches 4, and stateChange reports the real path for a signalStateStore while falling back to '/' for a foreign store.
  • libs/render/src/lib/action-bindings.spec.ts — resolved params, confirm accepted/declined, preventDefault on and off, onSuccess set/action/navigate (sync and async), onError set/action (sync throw and async rejection), and four repeat-visibility cases.

24 of these failed before the implementation and all pass after.

Docs

The pages the audit made describe the defects now describe the shipped behavior, with the warning callouts removed: render/api/signal-state-store.mdx (throw instead of silent drop, lastChange(), SignalStateStore return type), render/guides/events.mdx (full action-binding table plus sections on resolved params, confirmation, follow-ups and preventDefault; stateChange path; element-scope lifecycle), render/guides/lifecycle.mdx (mountCount counts element mounts), render/guides/repeat-loops.mdx and render/getting-started/introduction.mdx (per-item visible). api-docs.json regenerated for render and chat — chat's liveStore type moved to SignalStateStore.

Verification

  • npx nx run-many -t lint,test,build --projects=render — green, 136 tests.
  • npx nx run-many -t lint,test,build --projects=chat — green.
  • npx vitest run --root apps/website — 1326 tests, 131 files, all passing.
  • GROWTH_FORM_POLICY=growth_v1 npx nx build website — succeeds.
  • Built all seven render example apps; nx e2e green for cockpit-render-repeat-loops-angular, cockpit-render-state-management-angular, cockpit-render-element-rendering-angular, cockpit-render-registry-angular, cockpit-render-spec-rendering-angular, cockpit-render-computed-functions-angular, cockpit-ag-ui-json-render-angular, plus cockpit-chat-generative-ui-angular and cockpit-langgraph-durable-execution-angular as store consumers.

There is no CHANGELOG.md under libs/render, so no changelog entry was added. No cockpit-render-events-angular project exists.

🤖 Generated with Claude Code

@vercel

vercel Bot commented Sep 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
threadplane Ignored Ignored Preview Sep 8, 2026 3:18am UTC

Request Review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated approval: this PR received an intelligent (AI) code review. See the review comments on this PR.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Claude finished @blove's task in 0s —— View job


I'll analyze this and get back to you.

@blove
blove force-pushed the blove/render-followups branch from 1e92f95 to d652dfc Compare September 8, 2026 01:09
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Claude finished @blove's task in 0s —— View job


I'll analyze this and get back to you.

blove and others added 2 commits September 7, 2026 20:13
…element lifecycle and state paths

Six defects the docs audit had to describe truthfully, each with a spec that
fails before the change:

- `signalStateStore` paths: `parsePointer` dropped segment 0 unconditionally,
  so `set('count', 1)` replaced the whole state model. A path must now be
  `''`/`'/'` (root) or start with `/`; anything else throws an Error naming
  the path. Silent data loss is worse than a throw, so this applies in
  production too.
- Element-scope lifecycle events fired only for elements carrying an
  undeclared `lifecycle` field. `<render-element>` now emits `mounted` and
  `destroyed` for every element it mounts, which is what makes `mountCount`
  and `lastMountAt` on `RenderLifecycleService` mean something.
- `stateChange` hardcoded `path: '/'` and the full snapshot as `value`.
  `signalStateStore()` now records its last mutation on the exported
  `SignalStateStore` interface, and `RenderSpecComponent` reads it to report
  the real path and value. Foreign stores keep `'/'` plus the snapshot.
- `ActionBinding.confirm`, `onSuccess`, `onError` and `preventDefault` were
  accepted by the spec format and never read. All four are implemented,
  mirroring `executeAction` in `@json-render/core` (including the
  `'$error.message'` substitution in an `onError` set map).
- Action `params` were spread unresolved, so a `$state` expression reached
  the handler as a literal object. They now go through `resolveElementProps`
  with the element's repeat scope, as element props do.
- `visible` was ignored on the repeat branch. It is now evaluated per
  repeated item in that item's scope, so `$item` and `$index` conditions can
  hide individual rows.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…lifecycle and state paths

Follows the library change. The pages the audit made describe these defects
now describe the behavior that shipped:

- `render/api/signal-state-store.mdx` — the silent-drop warning is replaced by
  the thrown error, and `lastChange()` plus the `SignalStateStore` return type
  are documented.
- `render/guides/events.mdx` — the action-binding table lists all six fields,
  with sections for resolved params, confirmation, follow-ups and
  `preventDefault`; `stateChange` now reports a real path.
- `render/guides/lifecycle.mdx` — `mountCount` counts element mounts too.
- `render/guides/repeat-loops.mdx` and `render/getting-started/introduction.mdx`
  — `visible` is evaluated per repeated item.

Also regenerates api-docs for the new exports.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@blove
blove force-pushed the blove/render-followups branch from d652dfc to 061c4ca Compare September 8, 2026 03:14
@blove
blove enabled auto-merge (squash) September 8, 2026 03:14
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Claude finished @blove's task in 0s —— View job


I'll analyze this and get back to you.

@blove
blove merged commit 8ae6a86 into main Sep 8, 2026
76 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant