Skip to content

Nothing fails the build when a field component ignores the configuration-refresh contract #349

Description

@phmatray

Problem / motivation

The configuration-refresh hook (OnFieldConfigurationChanged, #298 for MudBlazor and #335 for Fluent
UI) has one rule, stated in its own XML docs:

Assign every cached property on every call, including back to its default. […] State derived from
the configuration counts too.

Getting it wrong is silent — the field renders the previous field's settings and nothing throws. And
it has been got wrong repeatedly, by the same person, in the same pass:

Component What was missed Caught by
MudBlazor text _passwordVisible — a revealed password stayed type="text" review (#308)
MudBlazor LOV _selectedItems, _displayText, _isLoading review (#308)
MudBlazor base _shrinkLabelDiagnosticEmitted review (#308)
MudBlazor lookup _displayText — skipped by the sweep entirely review (#308)
Fluent lookup cleared _displayText with nothing to repopulate it — blank for ever review (#336)
Fluent LOV _selectedItems leaked the previous field's values into the new field's model review (#336)
Fluent lookup _isOpen/_rows/_searchText — a stale row click throws review (#336)
Fluent autocomplete _options, _selectedOption review (#336)

Every one of those was caught by a human-style review pass rather than by a test, because the
refresh suites cover only the components someone happened to write a test for — text and numeric on
both sides, plus lookup on the Fluent side after #336. #336's review said so directly: the findings
were all in components the suite does not exercise.

That is the wrong shape for a rule that applies to every field type in both adapters. It should be
a build failure, not a review catch.

Proposed solution

A parity suite that drives a Configuration swap across every field type in both adapters and
asserts the component reflects the new field, so a component added or migrated without honouring the
hook fails the build.

The repo already has the pattern: RenderPipelineParityTests compares the two placements (ordinary
vs collection item) of every field type, and CollectionItemFixture supplies one model per type
precisely so a suite covers them all "by default rather than by remembering to". This is the same idea
on a different axis — placements → adapters, and rendering → configuration refresh.

Alternatives considered

Add a test per component, by hand. What has happened so far, and the reason this issue exists: the
next component added is the next one nobody remembers to cover.

A reflection-driven test that asserts every FieldComponentBase subclass overrides the hook.
Cheap, and it checks the wrong thing — a component can override the hook and still leave half its
state behind, which is exactly what happened seven times above.

A Roslyn analyzer flagging GetAttribute calls outside the hook. Would catch the configuration
half at compile time and nothing of the derived-state half. Worth considering later; not a substitute.

Do nothing. The rule is documented and the reviews did catch it. But the same reviewer caught the
same class of thing twice in two consecutive PRs, which is a coverage gap wearing a diligence costume.

Area

FormCraft.ForMudBlazor.UnitTests, FormCraft.ForFluentUI.UnitTests — a shared parity suite,
alongside the existing Fields/FieldConfigurationRefreshTests.cs in each.

Related: #298, #308, #335, #336, #343

🧠 Brainstorm

Problem / context

The hook is now in core (FieldComponentBase), so both adapters and every future one inherit the
mechanism. What they do not inherit is the discipline the override requires, and that discipline is
the part that keeps being missed.

The failure is silent by construction: a stale property renders plausible output. There is no
exception, no log line, and nothing about the component looks wrong in isolation. Only a swap makes it
visible — which is exactly what no test does unless someone writes one per component.

Approaches

A. A table-driven parity suite, one row per field type per adapter. Each row: build two
configurations differing in one observable attribute, render, swap, assert the observable changed. New
field type → add a row, and the row is a few lines. Cost: a per-type "observable attribute" table,
which is real work to write once and trivial to extend.

B. Extend each adapter's existing refresh suite component-by-component. Same coverage, no shared
table, and no forcing function — the next component still needs someone to remember.

C. Reflection over every component type, asserting OnFieldConfigurationChanged is overridden.
Automatic and shallow; passes for a component that overrides the hook and resets nothing.

Recommendation

A, seeded from the shapes CollectionItemFixture already provides. The table is the forcing
function: it is the thing a reviewer can look at and say "the date components have no row", which is
not a question anyone can answer today.

Include the derived-state cases explicitly, not just bound attributes — the lookup display text,
the LOV selection, the password toggle. Those are where every real miss has been, and a suite that
only checks Min/InputType would have caught none of the eight findings above.

Where a component genuinely has nothing to refresh — the three Fluent date components cache no
configuration — the table should say so explicitly rather than omitting the row, so "not covered" and
"nothing to cover" stop looking identical.

📋 Spec

Goal

A component that does not honour the configuration-refresh contract fails the build, in either adapter.

Scope

  • One parity case per field type per adapter, driving a real Configuration swap.
  • Coverage of derived state, not only of directly-bound attributes.
  • An explicit "nothing to refresh" entry for components that cache no configuration.

Non-goals

  • Changing the hook or any component's behaviour — this is a coverage change. If it finds a real bug,
    that bug gets its own commit within the PR, clearly separated.
  • Collection-item placement, which RenderPipelineParityTests already covers on its own axis.

Design

flowchart TD
    A[per field type: two configurations<br/>differing in one observable] --> B[render with config A]
    B --> C[assert the observable reads A]
    C --> D[swap to config B on the same instance]
    D --> E[assert the observable reads B]
    E --> F{derived state?}
    F -- yes --> G[assert it reflects B, not A]
Loading

Key files

  • create FormCraft.ForMudBlazor.UnitTests/Fields/FieldConfigurationParityTests.cs.
  • create FormCraft.ForFluentUI.UnitTests/Fields/FieldConfigurationParityTests.cs.
  • reuse FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixture.cs models where they fit; see
    The Fluent UI test suite re-declares the collection-item fixture's models #343 for the Fluent suite's missing equivalent, which this will want.

Validation rules

  • Every field type in each adapter has a row, or an explicit "nothing to refresh" entry.
  • Each row swaps Configuration on one component instance rather than rendering twice from scratch —
    rendering fresh cannot fail, which is what makes the bug invisible.
  • Derived-state rows assert the user-visible outcome (rendered DOM or bound parameter), never a private
    field.

Edge cases

  • Components whose observable is which sub-component renders (Fluent text: FluentTextArea vs
    FluentTextInput; MudBlazor text: MudMask presence). Assert the render tree, not a property — the
    repo has been bitten twice by a property that stayed stale while rendering moved on, and once by the
    reverse.
  • The LOV components throw when no LovConfiguration is supplied, so both configurations in that
    row must supply one.
  • A swap that changes TValue is out of scope — Blazor would not reuse the instance anyway.

Assumptions

  • Context.Field is reference-stable in both adapters, which both refresh suites already pin. The
    parity suite depends on it and should reference those tests rather than re-pinning it a third time.

🛠️ Implementation plan

For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (- [ ]) syntax for tracking.

Goal: the configuration-refresh contract is enforced by tests, per field type, in both adapters.
Architecture: a table-driven parity suite per adapter, swapping Configuration on one instance.
Tech stack: .NET 8/10, Blazor, MudBlazor 9.8.0, Fluent UI Blazor v5, xUnit + Shouldly + bUnit.

Global Constraints

  • This is a coverage change. Any production fix it forces is a separate, clearly-labelled commit.
  • Assert what renders, not private state — a property can stay stale while rendering moves on, and vice versa; both have happened here.
  • Swap on one instance. Rendering each configuration from scratch passes trivially and proves nothing.
  • dotnet test --filter is inert here (MTP0001) — run dotnet test -c Release.
  • dotnet format is a CI gate since The .editorconfig's style rules are enforced nowhere, so 574 violations have accumulated #301 — must exit clean before ready.
  • Build in Release: TreatWarningsAsErrors=true; do not relax it.
  • Commit identity: Philippe Matray <phmatray@gmail.com>. PRs target dev.

Task 1: The MudBlazor parity suite

Files: create FormCraft.ForMudBlazor.UnitTests/Fields/FieldConfigurationParityTests.cs.

Interfaces: a table of (field type, two configurations, observable assertion), one entry per component.

  • Step 1: Enumerate every MudBlazor field component and write the table, marking any with nothing to refresh explicitly.
  • Step 2: Implement the rows for the bound-attribute cases and run dotnet test -c Release → record which fail.
  • Step 3: Fix any production miss the failures expose, in its own commit, before continuing.
  • Step 4: Run dotnet test -c Release → PASS.
  • Step 5: Commit: test(mudblazor): assert every field type refreshes its configuration

Task 2: The derived-state rows

Files: modify FormCraft.ForMudBlazor.UnitTests/Fields/FieldConfigurationParityTests.cs.

Interfaces: rows asserting display text, selection lists and toggle state rather than bound attributes.

  • Step 1: Add rows for the state every past miss lived in — lookup display text, LOV selection, the password toggle.
  • Step 2: Run dotnet test -c Release → record failures; each is a real bug.
  • Step 3: Fix any that fail, in a separate commit from the tests.
  • Step 4: Run dotnet test -c Release → PASS.
  • Step 5: Commit: test(mudblazor): cover state derived from the field configuration

Task 3: The Fluent parity suite

Files: create FormCraft.ForFluentUI.UnitTests/Fields/FieldConfigurationParityTests.cs.

Interfaces: the same table shape, over the Fluent components.

  • Step 1: Mirror the MudBlazor table for the Fluent components, including the three date components as explicit "nothing to refresh" entries.
  • Step 2: Run dotnet test -c Release → record failures.
  • Step 3: Fix any production miss in its own commit.
  • Step 4: Run dotnet test -c Release → PASS.
  • Step 5: Commit: test(fluentui): assert every field type refreshes its configuration

Task 4: Make the gap visible for the next component

Files: modify both parity suites; modify README.md.

  • Step 1: Add a test that fails when a field component type exists with neither a parity row nor an explicit exemption, so a new component cannot be added silently uncovered.
  • Step 2: Run dotnet test -c Release → PASS, and confirm it fails when a row is removed.
  • Step 3: Note in the README's custom-component guidance that the parity suite enforces the hook's contract.
  • Step 4: Run dotnet build -c Release, dotnet test -c Release and dotnet format FormCraft.sln --verify-no-changes → all clean.
  • Step 5: Commit: test: fail the build when a field component has no configuration-refresh coverage

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority:mediumFix when possiblestatus:triagedClassified and ready for analysis/worktype:testTest improvements or additions

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions