feature: Remove Component from ValidationState, add IValidationState about reactiveui.validation HOT 6 CLOSED

Thanks for describing your vision of the modified API in detail! Sounds really reasonable. Worth noting btw that we should define the new interface as somewhat like the following to avoid introducing a massive breaking change:

public interface IValidationState
{
    bool IsValid { get; }
    ValidationText Text { get; }
    [Obsolete("This property will be removed soon.")]
    IValidationComponent Component { get; }
}

Currently, we are planning to give people a couple of months to adapt before removing the [Obsolete] APIs completely. Who knows, probably there are projects that are relying on the ValidationState.Component property in some observable chains currently exposed from the library etc.

from reactiveui.validation.

Hi @worldbeater,

As IValidationState would be a new interface anyway it does not need (and shouldn't have) the Component property. It won't break any existing code for it to be absent as no existing code would know about the interface.

Changing any API method that acccepts ValidationState to now accept IValidationState should likewise be low impact as they would accept the existing ValidationState object (so long as it is modified to implement IValidationState. Of course, such code would not be able to use the Component property, as it doesn't exist on the new interface (it's only used once in the library anyway as mentioned).

To make changes to ValidationState backwards compatible you can make the Component nullable, and obsolete it. You can also change the constructor parameter for the component to be optional and obsolete and remove the null check. If you want to belt and brace it, you could make the whole object obsolete, and instead provide a new ValidationState struct in a different namespace location, that doesn't include Component.

from reactiveui.validation.

Assuming we have IValidationComponent.ValidationStatusChange exposed to the outer world, and also assuming the fact, that the ValidationContext class implements IValidationComponent, changing ValidationStatusChange from IObservable<ValidationState> to IObservable<IValidationState> would break existing code like shown in this snippet:

IValidationComponent context = new ValidationContext();
context.ValidationStatusChange
    .Select(state => state.Component) // Who knows, there may be folks who rely on this Component property.

Adding an [Obsolete] Component property to the new interface could make the transition less breaking and less painful. I mean, if we are introducing a new IValidationState, we are planning to change the return type of ValidationStatusChange observable anyway. Or do you mean it is worth continuing returning IObservable<ValidationState>, and change the return type to IObservable<IValidationState> later, when we will be removing stuff marked as [Obsolete], or that we just shouldn't change that return type?

from reactiveui.validation.

That's a good point, though I wonder how many people actually do use the Component. My suspicion is it would be very rare at best as the existing recommended way to use the library is through the ValidationRule extension overloads and the underlying components and states are not particularly surfaced. The problem of adding it to the new interface is that people coming to use it will have to implement it (though you could provide a default implementation in the interface that returns null to prevent the need for its explicit implementation), whereas, the goal would be to discourage its use entirely. It seems a shame to add an Obsolete default implementation property to a brand new interface, but it is a safe option.

from reactiveui.validation.

This has been fully addressed in #130.

from reactiveui.validation.

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

from reactiveui.validation.