You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Converters from AddExtraSettings cannot override built-in converters #1834
AddExtraSettings also applies the callback to the already-built jsonSettings, which likewise appends.
Argon resolves converters first-match-wins: JsonSerializer.GetMatchingConverter walks Converters by index and returns the first whose CanConvert returns true. So a user doing the natural thing:
gets a converter that is never invoked, with no error and no warning. Verify's DateConverter keeps winning. This came up in VerifyTests/Argon#394, where the user spent time debugging a DateOnly converter that could not have fired.
The workaround is settings.Converters.Insert(0, ...), but that requires knowing both that Verify preseeds the collection and that Argon is first-match-wins. Neither is discoverable.
Suggestion: have Verify insert extra-settings converters ahead of its built-ins, so user converters take precedence by default. Options:
Run the extraSettings callbacks against the settings before adding the built-in converters, then append the built-ins. Simple, but changes ordering for any other converter-collection mutation the callback performs.
Snapshot converters.Count before running the callbacks and move newly added converters to the front afterwards. More targeted.
Leave the behaviour as is and document it, plus expose something explicit like VerifierSettings.AddConverter that inserts at index 0.
Option 1 or 2 seem preferable to me: a user-registered converter losing to a framework built-in is surprising in either direction, but silently losing is the worse of the two.
Activity