Skip to content

feat(forms): @-mention field tokens + multi-value logic, with review fixes#110

Merged
Vijayabaskar56 merged 2 commits into
devfrom
fix/logic-mention-review-findings
Jun 15, 2026
Merged

feat(forms): @-mention field tokens + multi-value logic, with review fixes#110
Vijayabaskar56 merged 2 commits into
devfrom
fix/logic-mention-review-findings

Conversation

@Vijayabaskar56

@Vijayabaskar56 Vijayabaskar56 commented Jun 15, 2026

Copy link
Copy Markdown
Member

Summary

Field-mention (@) tokens in labels/descriptions that resolve to live answers (built on @platejs/mention), plus logic-engine support for multi-value set-values and an optional action.

This branch also applies the fixes from the /code-review pass.

Review fixes

  • LabelBody conditional hook → crash (correctness). useStore was called after an early return; removing a label's last @-mention flips labelNodes to undefined, changing the hook count and crashing the live preview. Moved the subscription into a LabelBodyWithMentions child that only mounts when labelNodes && form, so hook order is always stable. (src/components/form-components/fields/shared.tsx)
  • Duplicated asString coercion (maintainability). The logic engine carried its own copy of the array→string coercion that mirrors operators.ts. Exported the canonical asString from operators.ts and imported it in engine.ts so the two can't drift. (src/lib/logic/engine.ts, src/lib/logic/operators.ts)
  • O(fields × rules) visibility convergence per keystroke (efficiency). The fixpoint loop re-accumulated every rule up to fields.length times. It only matters when a show/hide-toggled field is itself a condition source; otherwise masking can't change any outcome. Added that guard so unrelated forms run a single pass. (src/lib/logic/engine.ts)

Findings investigated and dismissed

  • A reported set-value key collision was a false positive — the key function already joins arrays on a NUL byte (``), which rendered as a space in the diff viewer.
  • return clear in form-option-item-node.tsx and getClientRects()[0] in block-draggable.tsx are both correctly handled (cleanup return / rect ? guard).

Verification

  • pnpm exec tsc --noEmit — clean
  • vitest run on engine / operators / extract-ruleset / resolve-mentions — 116 passed

CI

Initial CI flagged two issues, both now fixed in a follow-up commit:

  • Formatmulti-select.tsx was left unformatted; ran oxfmt.
  • Lint (knip) — removed the dead useFormIsDark export. The feature dropped getMultiSelectColor's isDark argument and the dark chip palette (form-preview chips render neutral now), so nothing consumed the hook anymore.

…fixes

Field-mention (`@`) tokens in labels/descriptions that resolve to live answers,
built on @platejs/mention, plus logic-engine support for multi-value set-values
and an `optional` action.

Review fixes applied on top:
- LabelBody: move the values useStore into a child component so the hook order
  stays stable when a label's last mention is removed (was a conditional hook
  that could crash the live preview).
- engine: share a single asString coercion with operators.ts instead of a
  duplicated copy that could drift.
- engine: skip the visibility-convergence loop unless a show/hide-toggled field
  is itself a condition source — one pass is exact otherwise, avoiding
  O(fields × rules) re-accumulation per keystroke on large forms.
@vercel

vercel Bot commented Jun 15, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
reform Ready Ready Preview, Comment Jun 15, 2026 1:55pm

Request Review

- oxfmt multi-select.tsx (was left unformatted in the prior commit → Format job).
- Remove the now-unused useFormIsDark hook (and its orphaned useResolvedTheme
  import): this feature dropped getMultiSelectColor's isDark argument and the
  dark chip palette, so form-preview chips render neutral and nothing reads the
  hook anymore → knip --strict failed the Lint job. Update the stale comment.
@Vijayabaskar56 Vijayabaskar56 merged commit c62f4bb into dev Jun 15, 2026
4 of 5 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