From 655ad04f4967946c21f7820187f47cb4ac37263f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timoth=C3=A9=20Larivi=C3=A8re?=
 <timothe.lariviere@outlook.com>
Date: Fri, 10 May 2019 20:23:30 +0200
Subject: [PATCH 1/2] Added check to prevent control reuse with different
 AutomationId

---
 .paket/Paket.Restore.targets        | 58 ++---------------------------
 src/Fabulous.Core/ViewConverters.fs | 19 ++++++++--
 2 files changed, 20 insertions(+), 57 deletions(-)

diff --git a/.paket/Paket.Restore.targets b/.paket/Paket.Restore.targets
index 017f39497..c9506cc71 100644
--- a/.paket/Paket.Restore.targets
+++ b/.paket/Paket.Restore.targets
@@ -105,8 +105,8 @@
       <PaketRestoreRequired Condition=" '$(PaketRestoreLockFileHash)' == '' ">true</PaketRestoreRequired>
     </PropertyGroup>
 
-	<!--
-		This value should match the version in the props generated by paket
+	<!-- 
+		This value should match the version in the props generated by paket 
 		If they differ, this means we need to do a restore in order to ensure correct dependencies
 	-->
     <PropertyGroup Condition="'$(PaketPropsVersion)' != '5.185.3' ">
@@ -228,7 +228,6 @@
   </Target>
 
   <Target Name="PaketOverrideNuspec" AfterTargets="GenerateNuspec" Condition="('$(IsPackable)' == '' Or '$(IsPackable)' == 'true') And Exists('$(MSBuildProjectDirectory)/obj/$(MSBuildProjectFile).references')" >
-
     <ItemGroup>
       <_NuspecFilesNewLocation Include="$(BaseIntermediateOutputPath)$(Configuration)\*.nuspec"/>
       <MSBuildMajorVersion Include="$(DetectedMSBuildVersion.Replace(`-`, `.`).Split(`.`)[0])" />
@@ -238,12 +237,10 @@
     <PropertyGroup>
       <PaketProjectFile>$(MSBuildProjectDirectory)/$(MSBuildProjectFile)</PaketProjectFile>
       <ContinuePackingAfterGeneratingNuspec>true</ContinuePackingAfterGeneratingNuspec>
-      <UseMSBuild16_0_Pack>false</UseMSBuild16_0_Pack>
-      <UseMSBuild16_0_Pack Condition=" '@(MSBuildMajorVersion)' >= '16' ">true</UseMSBuild16_0_Pack>
       <UseMSBuild15_9_Pack>false</UseMSBuild15_9_Pack>
-      <UseMSBuild15_9_Pack Condition=" '@(MSBuildMajorVersion)' == '15' AND '@(MSBuildMinorVersion)' > '8' ">true</UseMSBuild15_9_Pack>
+      <UseMSBuild15_9_Pack Condition=" '@(MSBuildMajorVersion)' > '15' OR ('@(MSBuildMajorVersion)' == '15' AND '@(MSBuildMinorVersion)' > '8') ">true</UseMSBuild15_9_Pack>
       <UseMSBuild15_8_Pack>false</UseMSBuild15_8_Pack>
-      <UseMSBuild15_8_Pack Condition=" '$(NuGetToolVersion)' != '4.0.0' AND (! $(UseMSBuild15_9_Pack)) AND (! $(UseMSBuild16_0_Pack)) ">true</UseMSBuild15_8_Pack>
+      <UseMSBuild15_8_Pack Condition=" '$(NuGetToolVersion)' != '4.0.0' AND (! $(UseMSBuild15_9_Pack)) ">true</UseMSBuild15_8_Pack>
       <UseNuGet4_Pack>false</UseNuGet4_Pack>
       <UseNuGet4_Pack Condition=" (! $(UseMSBuild15_8_Pack)) AND (! $(UseMSBuild15_9_Pack)) ">true</UseNuGet4_Pack>
       <AdjustedNuspecOutputPath>$(BaseIntermediateOutputPath)$(Configuration)</AdjustedNuspecOutputPath>
@@ -261,53 +258,6 @@
     </ConvertToAbsolutePath>
 
     <!-- Call Pack -->
-    <PackTask Condition="$(UseMSBuild16_0_Pack)"
-              PackItem="$(PackProjectInputFile)"
-              PackageFiles="@(_PackageFiles)"
-              PackageFilesToExclude="@(_PackageFilesToExclude)"
-              PackageVersion="$(PackageVersion)"
-              PackageId="$(PackageId)"
-              Title="$(Title)"
-              Authors="$(Authors)"
-              Description="$(Description)"
-              Copyright="$(Copyright)"
-              RequireLicenseAcceptance="$(PackageRequireLicenseAcceptance)"
-              LicenseUrl="$(PackageLicenseUrl)"
-              ProjectUrl="$(PackageProjectUrl)"
-              IconUrl="$(PackageIconUrl)"
-              ReleaseNotes="$(PackageReleaseNotes)"
-              Tags="$(PackageTags)"
-              DevelopmentDependency="$(DevelopmentDependency)"
-              BuildOutputInPackage="@(_BuildOutputInPackage)"
-              TargetPathsToSymbols="@(_TargetPathsToSymbols)"
-              SymbolPackageFormat="symbols.nupkg"
-              TargetFrameworks="@(_TargetFrameworks)"
-              AssemblyName="$(AssemblyName)"
-              PackageOutputPath="$(PackageOutputAbsolutePath)"
-              IncludeSymbols="$(IncludeSymbols)"
-              IncludeSource="$(IncludeSource)"
-              PackageTypes="$(PackageType)"
-              IsTool="$(IsTool)"
-              RepositoryUrl="$(RepositoryUrl)"
-              RepositoryType="$(RepositoryType)"
-              SourceFiles="@(_SourceFiles->Distinct())"
-              NoPackageAnalysis="$(NoPackageAnalysis)"
-              MinClientVersion="$(MinClientVersion)"
-              Serviceable="$(Serviceable)"
-              FrameworkAssemblyReferences="@(_FrameworkAssemblyReferences)"
-              ContinuePackingAfterGeneratingNuspec="$(ContinuePackingAfterGeneratingNuspec)"
-              NuspecOutputPath="$(AdjustedNuspecOutputPath)"
-              IncludeBuildOutput="$(IncludeBuildOutput)"
-              BuildOutputFolders="$(BuildOutputTargetFolder)"
-              ContentTargetFolders="$(ContentTargetFolders)"
-              RestoreOutputPath="$(RestoreOutputAbsolutePath)"
-              NuspecFile="$(NuspecFileAbsolutePath)"
-              NuspecBasePath="$(NuspecBasePath)"
-              NuspecProperties="$(NuspecProperties)"
-              PackageLicenseFile="$(PackageLicenseFile)"
-              PackageLicenseExpression="$(PackageLicenseExpression)"
-              PackageLicenseExpressionVersion="$(PackageLicenseExpressionVersion)" />
-
     <PackTask Condition="$(UseMSBuild15_9_Pack)"
               PackItem="$(PackProjectInputFile)"
               PackageFiles="@(_PackageFiles)"
diff --git a/src/Fabulous.Core/ViewConverters.fs b/src/Fabulous.Core/ViewConverters.fs
index fc62c9c14..c1d1041a7 100644
--- a/src/Fabulous.Core/ViewConverters.fs
+++ b/src/Fabulous.Core/ViewConverters.fs
@@ -220,10 +220,13 @@ module Converters =
     /// Checks whether an underlying control can be reused given the previous and new view elements
     let rec canReuseChild (prevChild:ViewElement) (newChild:ViewElement) =
         if prevChild.TargetType = newChild.TargetType then
-            if newChild.TargetType.IsAssignableFrom(typeof<NavigationPage>) then
-                canReuseNavigationPage prevChild newChild
+            if canReuseAutomationId prevChild newChild then
+                if newChild.TargetType.IsAssignableFrom(typeof<NavigationPage>) then
+                    canReuseNavigationPage prevChild newChild
+                else
+                    true
             else
-                true
+                false
         else
             false
 
@@ -239,6 +242,16 @@ module Converters =
         | ValueSome prevPages, ValueSome newPages -> (prevPages, newPages) ||> Seq.forall2 canReuseChild
         | _, _ -> true
 
+    /// Checks whether the control can be reused given the previous and the new AutomationId.
+    /// Xamarin.Forms can't change an already setted AutomationId
+    and internal canReuseAutomationId (prevChild: ViewElement) (newChild: ViewElement) =
+        let prevAutomationId = prevChild.TryGetAttribute<string>("AutomationId")
+        let newAutomationId = newChild.TryGetAttribute<string>("AutomationId")
+
+        match prevAutomationId, newAutomationId with
+        | ValueSome _, _ when prevAutomationId <> newAutomationId -> false
+        | _, _ -> true
+
     /// Update a control given the previous and new view elements
     let inline updateChild (prevChild:ViewElement) (newChild:ViewElement) targetChild = 
         newChild.UpdateIncremental(prevChild, targetChild)

From 36d677e8b5130c5263381a9c87e109fe316a6a72 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timoth=C3=A9=20Larivi=C3=A8re?=
 <timothe.lariviere@outlook.com>
Date: Fri, 10 May 2019 21:03:39 +0200
Subject: [PATCH 2/2] More concise check

---
 src/Fabulous.Core/ViewConverters.fs | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/src/Fabulous.Core/ViewConverters.fs b/src/Fabulous.Core/ViewConverters.fs
index c1d1041a7..d8c7d14e7 100644
--- a/src/Fabulous.Core/ViewConverters.fs
+++ b/src/Fabulous.Core/ViewConverters.fs
@@ -219,14 +219,11 @@ module Converters =
 
     /// Checks whether an underlying control can be reused given the previous and new view elements
     let rec canReuseChild (prevChild:ViewElement) (newChild:ViewElement) =
-        if prevChild.TargetType = newChild.TargetType then
-            if canReuseAutomationId prevChild newChild then
-                if newChild.TargetType.IsAssignableFrom(typeof<NavigationPage>) then
-                    canReuseNavigationPage prevChild newChild
-                else
-                    true
+        if prevChild.TargetType = newChild.TargetType && canReuseAutomationId prevChild newChild then
+            if newChild.TargetType.IsAssignableFrom(typeof<NavigationPage>) then
+                canReuseNavigationPage prevChild newChild
             else
-                false
+                true
         else
             false
 
@@ -248,9 +245,9 @@ module Converters =
         let prevAutomationId = prevChild.TryGetAttribute<string>("AutomationId")
         let newAutomationId = newChild.TryGetAttribute<string>("AutomationId")
 
-        match prevAutomationId, newAutomationId with
-        | ValueSome _, _ when prevAutomationId <> newAutomationId -> false
-        | _, _ -> true
+        match prevAutomationId with
+        | ValueSome _ when prevAutomationId <> newAutomationId -> false
+        | _ -> true
 
     /// Update a control given the previous and new view elements
     let inline updateChild (prevChild:ViewElement) (newChild:ViewElement) targetChild =