Xamarin.forms: Open/Closed principle missing

[smstuebe]

Description

I try to answer questions on stackoverflow from time to time. And always think "Ok this is a 5min Task. Just override a Renderer and that's it." But not with Xamarin.Forms. :( internal is used all over the place and this makes it impossible to extend native views without rewriting whole renderers. I encountered the following problem back to when Xamarin.Forms was closed source and I had to decompile it just to find out it is often impossible to do some customization.

Steps to Reproduce (just an example)

1. Imagine you want to override a method in a EditText (Entry) in Android
2. Look up the native Renderer EntryRenderer. Ok this is creating a FormsEditText as native control.
3. Look up the native view FormsEditText
4. Try to inherit it. Doesn't work, because of an internal constructor.
5. Inherit from EntryRenderer and try to override CreateNativeControl. Damn! It has to return a FormsEditText and you can't inherit it (see 4.)
6. Solution: Copy all the code from EntryRenderer and FormsEditText prefix them with My and add the tiny override you intended to do in step 1.

Expected Behavior

1. inherit MyFormsEditText from FormsEditText override your method
2. inherit MyEntryRenderer from EntryRenderer return your new MyFormsEditText
3. done.

Actual Behavior

see steps to reproduce 2. - 6.

Basic Information

• Version with issue: latest
• Last known good version: none
• IDE: all of them
• Platform Target Frameworks:
  
  
  
  • Android: all
  
  

General Questions / Remarks

• This is just an example. Why are internal / non virtual / extension methods used so widely in the code? You can't override them
• Is there a general design decision document where this style was decided for a good reason?
• The problem is: The developer is forced to copy a lot of code from the Xamarin.Forms repo. He will miss bugfixes and gets a lot of unmaintained code :(
• it's inconsistent (see FormsTextView (public/protected ctors) vs FormsEditText (internal ctor))

[jassmith]

I think we can spend some time investigating if we can open this API up. I will try to get it put into the F100 queue if possible.

As for your questions:

This is just an example. Why are internal / non virtual / extension methods used so widely in the code? You can't override them

Yeah unfortunately once something is public API its permanent. This means no matter what happens with it we can't change it. On many MANY occasions this has forced us to keep around buggy code/behaviors to avoid breaking subclasses/derived code.

Its a balancing act for sure, the more we open up the harder it becomes to move the product forward without risking a regression for someone who overrode a (for example) label renderer and added a feature and oops now we are clobbering over the same API they were using.

It's a real problem.

Is there a general design decision document where this style was decided for a good reason?

Pretty much the above mixed with a healthy dose of constantly having to not do things because we can't ensure backwards compat. Its an emergent behavior more than a desire. If there were no drawbacks or expectations that API's will remain untouched forever we would simply open everything.

The problem is: The developer is forced to copy a lot of code from the Xamarin.Forms repo. He will miss bugfixes and gets a lot of unmaintained code :(

That definitely sucks. We're working on some ways to make this a little better with the next round of renderers.

it's inconsistent (see FormsTextView (public/protected ctors) vs FormsEditText (internal ctor))

History is fun. I don't remember the exact history on this but we argue about this quite a bit ourselves internally.

[lothrop]

I've seen things in the .NET Framework change from internal to public, e.g., Task.CompletedTask. In which cases is opening up an API an issue?

[jassmith]

Moving from internal to public isn't a problem in itself. Its the new modes of usage which now have to be supported forever which are a problem sometimes.

[smstuebe]

And whats about semantic versioning? If you break something give it a new major version number. I think MvvmCross does it this way and is doing well.

[jassmith]

Making breaks like that is a decision made above my pay grade. Historically it has never happened. While I am not the person making that decision, I can say that the people who are, are very smart individuals with the best of intentions in mind. I have full confidence in their rationale behind not doing major breaking changes.

It is however the reason we are very very cautious about doing things that will cause us to wish we could.

[programmation]

Some observations:
1) Using semantic versioning enables a strategy where nothing has to be "supported forever".
2) Things break all the time anyway, regardless of how hard we try to prevent them from doing so. I had a breaking change from the latest XF a few days ago that wasn't signalled in the update docs. I filed a bug report, but it was analysed and returned as invalid because it was done on purpose. The net effect was a breaking change for my code. Of course I was a bit upset/perplexed/puzzled for a few minutes - then I thought of a solution, and moved on. If there are good reasons for the breaking change, and it's explained well with potential mitigation strategies, I think many adults will have a similar reaction.
3) For most of us, breaking changes signal the fact that things are improving in some way. I would prefer things to improve, even if it means my code breaks because of the improvement. I can deal with improvements. I can't deal with sealed or internal things that make it impossible to extend things that should be extensible.

[jassmith]

@programmation which issue did you have a break with?

[programmation]

Opacity stopped affecting InputTransparent. I had a bunch of views that switched Opacity from 0.0 to 1.0 and back where fully transparent would enable touches to pass through to views underneath. Now this is controlled by InputTransparent alone. I've adjusted my logic to make it work again.

[programmation]

Here's the thread: https://github.com/xamarin/Xamarin.Forms/issues/1758#event-1455942076

[jassmith]

Found it, looking into a revert as a service release. We'll probably have to add a quirk mode to make the new more consistent behavior opt-in

[programmation]

Well, as I said, I'm not so bothered about breaking changes if they improve things. I can understand the rationale and I've already adjusted my logic. So unless you have legitimate objections from another corner, I'm happy with things as they are. :)

[dhaligas]

I agree with opening things up. For example you cannot extend the NavigationPageRenderer in Android to get access to the Toolbar to add things like TitleView or change the NavigationIcon on the left. Plus when trying to copy the code from source into your own project you have to copy a bunch of internal classes to get it to work which usually doesn't end well.

[andreinitescu]

I think a key principle should be:

1. For any class/property/method must check if making it public/virtual would break anything. Never make it closed unless there's really a good reason.
2. If it breaks something, how complicated would it be to fix it so that it doesn't break it anymore
3. If it doesn't break, obviously make it accessible or overridable

I'm not so bothered about breaking changes if they improve things

From a framework perspective, breaking changes always sound bad, but if the reason is improvement and extensibility, they are well worth it. It's not like Xamarin Forms is perfect and there's no good reason to change things :)

[migueldeicaza]

We strive to never break working code. Every time we do that, a portion of our users are affected and get very upset.

Semantic versioning does not solve the ecosystem problem. Yes, we can roll out new versions and break code, but we also leave an entire ecosystem behind, users behind, code behind.

So we work very hard to avoid this, with the rare exception that happens.

What we will do is study any proposal to open up and decide based on the maturity of a particular code to open it up.

I am leaning at this point in our history to open things up generally unless we have strong reservations about a particular contract.

[samhouts]

Closed by #2103.