Xamarin.forms: Make XmlnsDefinitionAttribute Public

Created on 12 May 2018  Â·  14Comments  Â·  Source: xamarin/Xamarin.Forms

Summary

Make XmlnsDefinitionAttribute Public

API Changes

Change XmlnsDefinitionAttribute accessibility to public.

Intended Use Case

Control library authors would be able to use their own custom namespace schema definitions (e.g. http://mycompany.com/xaml rather than having to specify the exact assembly name and CLR namespace in the xmlns definitions. It would permit using a single namespace prefix for all custom controls in a library package even if the controls are in different assemblies / namespaces.

API-change Xaml </> enhancement âž•

All 14 comments

This would have a lot of great uses including for Frameworks, Toolkits, really any library built around Forms

Interestingly it was originally public, and then changed to internal:
https://github.com/xamarin/Xamarin.Forms/pull/557
I don't see any linked issue or explanation as to why it was changed. Not really sure what the harm would be in having it public.

It is internal because the parser only looks for those attributes in known assemblies. Making it public won't make it work, it requires some changes. Not impossible, not even hard...

Ah, that makes sense and also explains this comment!
https://github.com/xamarin/Xamarin.Forms/blob/bf8d70617c51704279c989726e0ab9ee93f1095f/Xamarin.Forms.Xaml/XamlParser.cs#L314

Would this be something that y'all would entertain a PR on?

PR is welcome

should be fixed in the XamlParser and in Xamarin.Forms.Build.Tasks/XmlTypeExtensions.cs for XamlC support
https://github.com/xamarin/Xamarin.Forms/blob/bf8d70617c51704279c989726e0ab9ee93f1095f/Xamarin.Forms.Build.Tasks/XmlTypeExtensions.cs#L15

Now that I think about it, it was made internal, and only looking into known assemblies, so we're sure it doesn't collide with UWP's (and now WPF's) XmlnsDefinitionAttribute.

I don't think it's a problem, as we define our own XmlnsDefinitionAttribute (not part of netstandard2.0), but it's worth checking

Well I've made good progress but hit a stumbling block in XamlGenerator.cs at GetClrNamespace:
https://github.com/xamarin/Xamarin.Forms/blob/master/Xamarin.Forms.Build.Tasks/XamlGenerator.cs

This gets called during the XAML build process to generate the code-behind .g.cs files.

The problem is that at this point the assembly doesn't exist yet, so I can't use reflection to find the assemblies referenced in order to look for XmlnsDefinitionAttribute and thus find the full qualified Type names. I'm unclear how, or if it's even possible, to do so at this point in the build process. The existing code is hard-wired to only look for http://xamarin.com/schemas/2014/forms and http://schemas.microsoft.com/winfx/2006/xaml.

I'm gonna check out the .NET Reference Source to see how WPF handles this but would love any suggestions. Thanks!

@peter0302 oh, yes, there is that too... lemme think about it

@peter0302 iirc, my conclusion at the time was that XamlG would need to be rebased to work on top of Roslyn to allow that.
option 2 is to pass all the project references, and all the files of the current project to the XamlG task.

I have an experimental branch around, doing Xaml-related work while having all the Roslyn context. I'll see if I can do something with it...

I'm making some progress on the MSBuild Task front. I finally figured out how to pass the @(ReferencePath) variable into the XamlG task in Xamarin.Forms.targets.. For some reason it refused to work without me first assigning @(ReferencePath) to a new variable in a PropertyGroup and then referencing the new variable when initializing XamlGTask. Meanwhile XamlCTask had no problem taking @(ReferencePath) directly... Weird.

Anyway, now that I have those reference paths I can look for the XmlnsDefinitionAttribute on the assemblies and use that in GetClrNamespace in XamlGenerator.

The only thing that bothers me is that there'll now be three different places where it'll be checking for the NS definitions: XamlGenerator, XamlParser, and XmlTypeExtensions. Trying to minimize code changes but probably a little refactoring will be needed to avoid this getting too messy.

Ok it's looking good! I have custom namespaces working with XamlParser, XamlG, and XamlC and all the unit tests pass. I should have a PR ready in a couple more days; I just want to test on VS for Mac first to make sure there are no gotchas with the MSBuild Task.

My work is here if anyone wants to take a look and comment first:

https://github.com/peter0302/Xamarin.Forms/tree/2691-xmlnsdefinitionattribute-public

@StephaneDelcroix did you need to mark this proposal as accepted first?

@StephaneDelcroix did you need to mark this proposal as accepted first?

no, that's fine

@StephaneDelcroix just a quick update. Was finally able to test successfully in VS for Mac yesterday with iOS and MacOS platforms and that's working fine. Just cleaning up the commits in my own fork and should have a PR to you today or tomorrow.

All XAML unit tests are passing (and I added three directed to this issue) but not all UITests are, and I haven't been able to run the Windows UI unit tests at all. This is true in master even without my changes. Is this normal/expected? It looks like some of them are looking for network locations that don't exist so I'm wondering if they're out of date or just need to be run on your internal Network?

Also I don't think I can make a meaningful UI unit test for this because - correct me if I'm wrong - it looks like they all build their UI in C# code and not in XAML so there wouldn't be any point. I did add the issue to the control gallery though so it can be looked up and a demonstration viewed when you run it as an app. I also added XAML unit tests like I mentioned that test XamlG, XamlC, and XamlParser, respectively.

this doesn't require uitests. it can be all unittested

Was this page helpful?
0 / 5 - 0 ratings