Xamarin.forms: [XamlC] DataTemplate clears the namescope of the child

[akihikodaki]

Description

This is probably a regression of #2556. DataTemplate clears namescope of the child.

This is caused by void Xamarin.Forms.Build.Tasks.SetNamescopesAndRegisterNamesVisitor.Visit(ElementNode node, INode parentNode). Consider the following steps which a compiled code may perform, for example.

• A is a class with compiled XAML.

1 (CreateObjectVisitor)
1a. LoadDataTemplate begins instatiating A.
1b. A.ctor calls A.InitializeComponent.
1c. A.InitializeComponent sets a new namescope to A.
1d. A.InitializeComponent registers some names.
1e. A.InitializeComponent ends.
1f. A.ctor ends.
2 (SetNamescopesAndRegisterNamesVisitor)
2a. DataTemplate sets a new namescope to A, removing the existent namescope and registered names!

1. Children of A and NameScopeExtensions.FindByName(Element, String) try to refer to names of A, and find nothing. The world ends.

Steps to Reproduce

Run a test included in the reproduction link.

Expected Behavior

Preserve the namescope of the child.

Actual Behavior

The namescope is overwritten with another, effectively clears it.

Basic Information

• Version with issue: 3.2.0
• Last known good version: Probably any older versions are fine, but I have not tested.
• IDE: Not used.
• Platform Target Frameworks:
  
  
  
  • iOS: Not used.
  
  
  • Android: Not used.
  
  
  • UWP: Not used.
  
  
• Android Support Library Version: Not used.
• Nuget Packages: Only Xamarin.Forms.
• Affected Devices: Probably everything.

Screenshots

N/A

Reproduction Link

https://github.com/akihikodaki/Xamarin.Forms/tree/gh4010

[kingces95]

Well, the unit test fails as described..

[akihikodaki]

By the way, the simplest solution for this issue is just to keep the pre-existent namescope as is if it is not null, but I didn't submit such a fix because it may make #2090 impossible. Such a fix makes two compilation unit (the compilation unit which include DataTemplate and the compilation unit of the child) share the same namescope. That allows to refer to name registered in another compilation unit. That is not good.

Another option is to make a "hidden" namescope for DataTemplate but it gives a question to the semantics of FindByName, which made public in #2556.

Maybe I'm thinking too much. The problem I described earlier was also present before #2556 as far as I can tell. But it is insightful that FindByName and compilation of x:Reference is inherently incompatible.

[StephaneDelcroix]

might be related to #3821

[akihikodaki]

I have not tested, but I think its fix, #4089, partially solves this issue. But the fix is not complete; FindByName should still be broken. (the "hidden namespace" case described in https://github.com/xamarin/Xamarin.Forms/issues/4010#issuecomment-427542190)

[BradChase2011]

@akihikodaki Correct, this is still a problem, especially when late loading templates.

[Vencode]

Any update on this issue? This is the only issue that prevent us from updating to the latest version of the nuget

[akihikodaki]

@Vencode I'm only aware of the case that breaks FindByName method, and which was not available for the older versions. What is regressing for your case?

[akihikodaki]

I was wondering about a fix for broken FindByName. WPF has System.Windows.FrameworkTemplate.FindName(String, FrameworkElement) to separate template namescope. However I am not sure if the API is the best for Xamarin.Forms.

The WPF method is an instance method and a template namescope is bound to a particular template instance. It is difficult to implement on Xamarin.Forms because the content generator does not have an argument to pass the instance of template (System.Func<System.Object>.) I think there are two options to implement template namescope separation.

1. Accept a new content generator type. (e.g. System.Func<Xamarin.Forms.ElementTemplate, System.Object>)
2. Don't associate template namescope to template instance.

1 requires changes of Xamarin.Forms.ElementTemplate API. 2 is an alternative possible as long as I can tell since there is no reason to associate template namescope to template instance. However, this questions whether a Xamarin.Forms counterpart of System.Windows.FrameworkTemplate.FindName(String, FrameworkElement) should be an instance method or not, or even whether it should be a member of Xamarin.Forms.ElementTemplate or Xamarin.Forms.Internals.NameScope.

So, in summary, there are design decisions to be made even if Xamarin.Forms follows WPF.

[Vencode]

@akihikodaki I've attached a file that contains a page that we have issues with.

On this page we use a DataTemplates, when we use x:Reference for the direct parent(ex: line 54) it works as expected and is able to access it, however when we try to get the root of the page/contentview(ex:line 64) it never finding the element.

With the version 3.1.0.697729 this works fine, any version after that the x:Reference stopped worked inside DataTemplates

NewsLargeCardContentView.zip

[StephaneDelcroix]

@Vencode

when we try to get the root of the page/contentview(ex:line 64)

in this case, you're trying to escape the ResourceDictionary, and the Namescope doesn't bubble past that. It's a design choice, and the fact that it worked before looks like a bug to me. You probably should be able to solve this using the (coming) RelativeSource (#4375) binding property...

[akihikodaki]

@StephaneDelcroix I thought the problem was the nested DataTemplate. Actually it didn't work for me, and I confirmed it was fixed with #4609. Now I wonder:

1. if making x:Reference to a value out of DataTemplate is fine
2. if the bug that ResourceDictionary does not function as namescope is really fixed

[StephaneDelcroix]

@akihikodaki

1. we should prefer RelativeSource for that
2. I'm no longer sure of anything. my gut feeling tells me accessing reference out of the resource can introduce subtle bugs, and weird behaviors, when the resources are shared...

[StephaneDelcroix]

also, I'm trying to confirm the original issue reported here, but the test looks wrong

[akihikodaki]

1. we should prefer RelativeSource for that

Is it fine for users of versions which do not support RelativeSource to use x:Reference as an alternative? Should an implementer allow this kind of x:Reference? (I would like to know that since I made #4609.)

also, I'm trying to confirm the original issue reported here, but the test looks wrong

What is wrong? I may fix it.

[StephaneDelcroix]

What is wrong? I may fix it.

there's a special task in the unit test that replaces the content of the constructor, and the test should look like:

    public partial class Gh4010 : ContentPage
    {

        public Xamarin.Forms.DataTemplate Template { get; set; }

        public Gh4010() => InitializeComponent();
        public Gh4010(bool useCompiledXaml)
        {
            //this stub will be replaced at compile time
        }

        [TestFixture]
        class Tests
        {
            [SetUp] public void Setup() => Device.PlatformServices = new MockPlatformServices();
            [TearDown] public void TearDown() => Device.PlatformServices = null;

            [Test]
            public void PreserveName([Values (false, true)]bool useCompiledXaml)
            {
                var i = new Gh4010(useCompiledXaml);
                var templated = (Label)i.Template.CreateContent();
                var dt = templated.FindByName("template");
                Assert.IsInstanceOf<Xamarin.Forms.DataTemplate>(templated.FindByName("template"));
            }
        }
    }

this new version tests the behavior with XamlC on and off. And both exhibits the exact same behavior (template.FindByName is null) ! So this doesn't look like a XamlC only issue. I keep investigating...

[StephaneDelcroix]

ok, it took me some time to re-learn what's going on here...

1. FindByName only lookup in the current Namescope. Namescopes no longer bubble up. this change was introduced in #2556. This is not a regression as FindByName wasn't public before. Parent and child namescopes only make sense in the context of a Xaml file, not in some c# code.
2. {x:Reference} is still able to bubble up, using the IReferenceProvider. that capability is public.

As I'm quite sure there's no bug here, I'm going to close this issue.

@akihikodaki Thanks for filing this, your investigation, and your patience. I'm going to review your compiled x:Reference soon (#4609), please make sure that FindByName doesn't bubble up namescopes, as it would make a difference with the non-compiled Xaml inflater.

@Vencode if you think there's still a bug for x:Reference, please open a new issue with a very small (the smallest possible) projects reproducing the bug attached to the ticket.