feat(component): instance-dynamic wire names — keyword escape hatches on the field-compiling helpers - #225
Conversation
… on the field-compiling helpers A form builder's wire name is computed per instance (user[tags]), which the class-level reactive_scope compile can't express — the gap that forced phlex-forms' tag_field draft (zoolutions/phlex-forms#6) onto raw data attrs, losing render-time validation. Every field-compiling helper now takes a verbatim keyword escape hatch (never re-scoped, validated at render, mutually exclusive with the blessed field form): - reactive_tags(name: "user[tags]") - reactive_filter(input: "#tags_query") — a raw CSS selector for the deliberately NAME-LESS query input (re-blesses the kwarg removed in #186, narrowed to this escape-hatch purpose) - nested_field_name(:items, :qty, scope: "order") — per-call prefix, wins over reactive_scope - reactive_nested_list(:items, as: :json, name: "order[items]") Server-side sugar only: the client already resolves these attributes as arbitrary root-scoped selectors; existing calls emit a byte-identical wire. Hardened beyond the plan after adversarial review: verbatim names reject backslashes/control chars (a raw newline makes Chromium's querySelectorAll throw during connect, breaking the whole root), and the kwargs dispatch on nil-presence so `input: cond && "#sel"` with a false condition fails loudly instead of emitting a silent dead binding. ## Test Coverage - unit: verbatim compile, never-re-scoped under reactive_scope, blank/ quote/backslash/control/boolean rejection, mutual exclusion, name: without as: :json, bracketed scope: - system (Puma + Falcon): the form-builder-shaped demo — id-targeted name-less query input filters/adds/removes client-side; a real submit carries user[tags] comma-joined with ZERO stray params ## Verification - [x] bundle exec rubocop passes (gem + docs app) - [x] bundle exec rspec spec/phlex spec/requests — 1381 passed - [x] bundle exec rspec spec/system — full suite green (Puma); new spec also green under CAPYBARA_SERVER=falcon - [x] rake build:js_check — no client drift (no JS changes by design) Closes #224
📝 WalkthroughWalkthroughAdds validated escape hatches for dynamic reactive field names and selectors, documents their form-builder usage, and introduces a dummy tags form with unit and system coverage. ChangesDynamic wire names
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant FormTagsFieldComponent
participant DemosController
Browser->>FormTagsFieldComponent: filter, add, or remove tags
FormTagsFieldComponent-->>Browser: update hidden user[tags] field
Browser->>DemosController: submit GET form
DemosController->>FormTagsFieldComponent: pass submitted tags and stray parameters
FormTagsFieldComponent-->>Browser: render submitted chips and echo
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.44.1)spec/phlex/reactive/component_spec.rbast-grep timed out on this file spec/system/tags_field_form_spec.rbast-grep retry budget exhausted before isolating this batch Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
spec/system/tags_field_form_spec.rb (1)
18-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the duplicated system-spec helpers
install_fetch_spyandhidden_valueare duplicated inspec/system/tags_field_spec.rb; move them to a shared system-spec helper to avoid drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/system/tags_field_form_spec.rb` around lines 18 - 32, Extract the duplicated install_fetch_spy and hidden_value helpers from the tags field system specs into the shared system-spec helper, then remove their local definitions from both spec files and ensure both specs use the shared implementations unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@spec/system/tags_field_form_spec.rb`:
- Around line 18-32: Extract the duplicated install_fetch_spy and hidden_value
helpers from the tags field system specs into the shared system-spec helper,
then remove their local definitions from both spec files and ensure both specs
use the shared implementations unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a5ea5c0-038d-4b3a-a816-d0a8574053b8
📒 Files selected for processing (9)
CHANGELOG.mdREADME.mddocs/app/views/docs/pages/draft_rows_new_parent.rblib/phlex/reactive/component/helpers.rbspec/dummy/app/components/form_tags_field_component.rbspec/dummy/app/controllers/demos_controller.rbspec/dummy/config/routes.rbspec/phlex/reactive/component_spec.rbspec/system/tags_field_form_spec.rb
|
Re CodeRabbit's nitpick (extract the duplicated |
Summary
Implements #224: every helper that compiles a field name through the class-level
reactive_scopenow takes a verbatim keyword escape hatch for instance-dynamic wire names — the gap that forced phlex-forms'tag_fielddraft (zoolutions/phlex-forms#6, caveats 1 & 2) onto raw data attrs, losing render-time validation.reactive_tags(name: "user[tags]")data-reactive-tags-field='[name="user[tags]"]'reactive_filter(input: "#tags_query")data-reactive-filter-input='#tags_query'nested_field_name(:items, :qty, scope: "order")reactive_scopeorder[items_attributes][NEW_ROW][qty]reactive_nested_list(:items, as: :json, name: "order[items]")data-reactive-nested-json-field='[name="order[items]"]'All four are never re-scoped, validated at render (blank /
"/\/ control chars / booleans raise), and mutually exclusive with the blessed positional-field form, which stays the default.reactive_fieldalready had this escape hatch (explicitname:wins) — it is the precedent and is unchanged.Server-side sugar only. The client already resolves
data-reactive-tags-field/data-reactive-filter-inputas arbitrary root-scoped CSS selectors (#tagsField,#syncFilter); no JS changed (rake build:js_checkgreen). Existing calls emit a byte-identical wire.reactive_filter(input:)deliberately re-blesses the kwarg removed in #186 — narrowed to the one case the field form can't express: a deliberately name-less query input inside a real form (a named input would submit a stray param), targeted by id. The pre-0.10input:/option:call shape is valid again with identical semantics instead of raising the removal error (CHANGELOG notes it).Test coverage
spec/phlex/reactive/component_spec.rb): verbatim compile; stays verbatim under a declaredreactive_scope; blank/quote/backslash/control-char/boolean rejection; mutual exclusion;name:withoutas: :jsonraises; bracketedscope:("user[profile]") accepted;option:/group:/empty:compose withinput:.spec/system/tags_field_form_spec.rb, green under Puma AND Falcon): a new dummy demo (FormTagsFieldComponent,/form_tags_field) in the exact phlex-forms shape — verbatimuser[tags]hidden field, id-targeted query input with noname. Proves: filter narrows by haystack, option-click + Enter add chips, remove works, zero action POSTs, no reload — then a real submit carriesuser[tags]comma-joined and the page's stray-param echo (request.query_parameters.keys - ["user"]) is empty.Verification
bundle exec rspec spec/phlex spec/requests— 1381 passedbundle exec rspec spec/system— full suite green (Puma); new spec also green underCAPYBARA_SERVER=falconbundle exec rubocop— clean (gem, 290 files; docs app file linted with its own config)rake build:js_check— no client drift (no JS changes by design)Closes #224
Refs zoolutions/phlex-forms#6
Deviations & judgment calls
Deviations
verbatim_name_selector!also rejects backslashes and control characters (not just"): a raw newline inname:makes real Chromium'squerySelectorAllTHROW during the controller'sconnect()— breaking the whole reactive root, not just the binding (happy-dom is lenient, so the bun suite can't catch it) — and a trailing backslash CSS-escapes the closing quote so the selector silently matches the wrong name.input: cond && "#sel"/name: cond && "…"/scope: cond && "order"idiom with a false condition previously slipped past the guards and emitted a silent dead binding ([name=""]) or a corrupting wire name (false[items_attributes][…]). Booleans now fail loudly (filter_selector!+verbatim_name_selector!reject them;nested_scope!requires String/Symbol).Discoveries
reactive_tags(field = nil)signature,reactive_tags(name: " ")did not raise: Ruby folds unknown keywords into a positional Hash for a method that declares none, so the Hash becamefieldand compiled a garbage-but-non-blank selector. The new signature makesname:a real kwarg, so that call shape now validates properly.TagsFieldComponent) gives its query input aname="tag_query", so its GET submit carries a straytag_query=param — harmless there, but exactly the behaviorinput:exists to avoid. Left unchanged: it covers the blessed field form;FormTagsFieldComponentcovers the escape hatch.Judgment calls
nested_field_name(scope:)rejects non-String/Symbol and blank, but NOT"— the result is anameattribute value (Phlex-escaped), never interpolated into a[name="…"]selector, and a bracketed scope ("user[profile]", a nested fieldset's object name) must stay legal. Thename:hatches DO reject"/\/control chars because they compile into a double-quoted selector the client queries with.reactive_nested_list(name:)withoutas: :jsonraises (loud, guided) rather than silently ignoring the kwarg — the:attributesmode has no field to name, and a silent no-op would hide a wiring mistake.request.query_parameters.keys - ["user"]into the component, which renders them in a testid'd div — the machine-checkable "no stray param" proof.Summary by CodeRabbit
New Features
Documentation
Tests