refactor(core)!: share diagnostics warning log and per-form latch across adapters (#398) - #407
Merged
Merged
Conversation
… latch across adapters
…dBlazor into core
…nstead of a local copy
…tics-log-and-latch
phmatray
marked this pull request as ready for review
September 23, 2026 23:30
phmatray
added a commit
that referenced
this pull request
Sep 23, 2026
… diagnostics move #407 (merged into dev while this branch was in flight) renamed DiagnosticLog to FormDiagnosticLog and moved it into FormCraft.Diagnostics. The merge picked up the rename on the read side (GetCustomTemplateValue) automatically but left the write side (UpdateFieldValue), added by this branch after #407 branched, on the old name.
7 tasks done
phmatray
added a commit
that referenced
this pull request
Sep 23, 2026
… custom templates (#413) * feat(core): add FieldValueSetterCache mirroring the value-getter cache Compiles a write-back Action<TModel, object?> from a field's ValueExpression, unwrapping the Convert node FieldConfigurationWrapper adds and assigning through the resulting member chain at any depth. Coerces the incoming boxed value to the target member's type first, the same Convert.ChangeType step both adapters' UpdateFieldValue used to perform against a GetProperty(fieldName) lookup that only ever resolved a top-level property. Part of #396. * fix(adapters): write back through the field's compiled expression for custom templates UpdateFieldValue now takes the IFieldConfiguration<TModel, object> instead of a bare field name and writes through FieldValueSetterCache<TModel>, replacing the typeof(TModel).GetProperty(fieldName)/SetValue reflection that only ever resolved a direct top-level property. A custom-template field bound to a nested path (x => x.Nested.Value) now writes back exactly as reliably as it already reads (#330). A write that fails (most commonly a null intermediate) is caught and reported once per field through each adapter's existing #330 diagnostic latch instead of throwing out of the event callback. Closes #396 * fix(adapters): use the renamed FormDiagnosticLog after syncing #407's diagnostics move #407 (merged into dev while this branch was in flight) renamed DiagnosticLog to FormDiagnosticLog and moved it into FormCraft.Diagnostics. The merge picked up the rename on the read side (GetCustomTemplateValue) automatically but left the write side (UpdateFieldValue), added by this branch after #407 branched, on the old name. * fix(adapters): report the coerced post-write value on OnFieldChanged UpdateFieldValue was reporting the caller's raw pre-coercion value through OnFieldChanged, whereas the reflection-based code it replaced reported its own Convert.ChangeType result. Read the value back through the same compiled FieldValueGetterCache the render path already uses, which reflects the true post-write state rather than re-deriving the conversion. Also strengthens both adapters' "warn once" regression test: the custom template body now reads context.Value, so the read path genuinely fires (and warns) during the initial render before ValueChanged is invoked, so the assertion that the write does not add a second warning actually exercises the shared-latch dedup it claims to. Found in code review of #396.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements #398.
Closes #398.
Executing the implementation plan task-by-task; the checklist below — and the plan on the issue — are
ticked as each task lands.
Plan
DiagnosticLogand the per-form latch into coreReview (Standards / Spec / Verification, run per implement-issue Step 7)
Standards — one real finding: a stale doc comment in
FormCraft.ForFluentUI.UnitTests/Components/CustomTemplateTests.csclaiming the adapter has "noshared diagnostics infrastructure" (now false, since this PR gives it one) and naming the renamed
DiagnosticLogtype. Fixed.Spec — flagged that
FormCraftComponent.razor.cs(FluentUI) now emits its custom-templatewarning under a new fixed category,
"FormCraft.ForFluentUI.CustomTemplateField", in place of theprior per-
TModelILogger<FormCraftComponent<TModel>>>()category, and read the issue's Spec text("FluentUI... keeps its own... category string for the unresolved-custom-template-field diagnostic")
as forbidding that. Verdict: disagree, kept as implemented. The issue's own Implementation
Plan (Task 2, Step 3) explicitly directs adding this exact constant, "matching MudBlazor's call
shape exactly" — MudBlazor's identical diagnostic already uses a named category
(
FormCraft.ForMudBlazor.CustomTemplateField), so FluentUI's prior per-TModelcategory was itselfthe inconsistency this issue exists to remove, not a fact to preserve. The Spec text's assumption
that FluentUI "already has" a dedicated category string doesn't match the pre-PR code. Also flagged
(and independently found by the Verification pass): the issue's acceptance criterion 4's grep
pattern was already non-clean before this PR touched anything, because of the unrelated, pre-existing
SecurityEnforcer(#321/#402) logger resolution at the same file — the criterion's grep isover-broad, not something this PR's diff could ever satisfy literally.
Verification — one real gap: the new category string had no test asserting its value (only
message text was checked). Fixed —
CustomTemplateTests.cs's localCapturingLoggerProvidernowtracks category like the core one does, and the regression test asserts
entries[0].Category.ShouldBe("FormCraft.ForFluentUI.CustomTemplateField"). Also caught: anEntriesproperty onFormCraft.ForMudBlazor.UnitTests/TestSupport/CapturingLoggerProvider.csleftwith zero callers once
DiagnosticLogTestsmoved to core — removed.Notes for the owner
FormDiagnosticScopewaspublicinFormCraft.ForMudBlazor(cascaded as a public parameter onMudBlazorFieldComponentBase), notinternalas the issue's Spec assumed. Moving it toFormCraft.Diagnosticswithout a forwardingshim (per the issue's own explicit instruction — "not kept as a compatibility forwarder") is a
source break for any third-party code that referenced
FormCraft.ForMudBlazor.FormDiagnosticScopedirectly. This is a preview series (
3.1.1-preview.*) and the demo apps build clean, but the PRtitle (
refactor(core): ...) won't flag this to release-please the way a!would. Flagging herefor a call on whether to add one.
Fixed along the way
Entriesproperty onFormCraft.ForMudBlazor.UnitTests'sCapturingLoggerProvider, orphaned by movingDiagnosticLogTeststo core.Follow-ups
CapturingLoggerProvidertest doubles now exist (MudBlazor's sharedTestSupportone, FluentUI's local one, and core's newDiagnosticTestDoubles.cs), each justifiedas "one file's/project's worth of use." Consolidating them into one shared test-support package is
out of scope for this issue (FluentUI hand-rolls its own diagnostics warning latch instead of sharing MudBlazor's DiagnosticLog #398 only moves production diagnostics code) but is the same
duplication shape Diagnostic emission is copy-pasted four times across the MudBlazor package #284/Diagnostic test-support helpers are still duplicated per suite #305 already named once; worth a dedicated issue if a fourth copy appears.
BREAKING CHANGE:
FormDiagnosticScopeandDiagnosticLogmove fromFormCraft.ForMudBlazortoFormCraft.Diagnostics(core). Code that references them by the old namespace must update itsusingdirective. No forwarding shim is provided.BEGIN_COMMIT_OVERRIDE
refactor(core)!: share diagnostics warning log and per-form latch across adapters (#398)
BREAKING CHANGE:
FormDiagnosticScopeandDiagnosticLogmoved fromFormCraft.ForMudBlazortoFormCraft.Diagnostics(core). Code that references them by the old namespace must update itsusingdirective. No forwarding shim is provided.END_COMMIT_OVERRIDE