diff --git a/src/Components/Blazor/testassets/StandaloneApp/Pages/FetchData.cshtml b/src/Components/Blazor/testassets/StandaloneApp/Pages/FetchData.cshtml index 2113b17af92e..0ecea85876a3 100644 --- a/src/Components/Blazor/testassets/StandaloneApp/Pages/FetchData.cshtml +++ b/src/Components/Blazor/testassets/StandaloneApp/Pages/FetchData.cshtml @@ -48,14 +48,14 @@ else WeatherForecast[] forecasts; - protected override async Task OnParametersSetAsync() + public override Task SetParametersAsync(ParameterCollection parameters) { - // If no value was given in the URL for StartDate, apply a default - if (StartDate == default) - { - StartDate = DateTime.Now; - } + StartDate = DateTime.Now; + return base.SetParametersAsync(parameters); + } + protected override async Task OnParametersSetAsync() + { forecasts = await Http.GetJsonAsync( $"sample-data/weather.json?date={StartDate.ToString("yyyy-MM-dd")}"); diff --git a/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj b/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj index b0608b621d64..41799dd9fb4a 100644 --- a/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj +++ b/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj @@ -3,7 +3,6 @@ netstandard2.0 Components feature for ASP.NET Core. - true true diff --git a/src/Components/Components/src/ParameterCollectionExtensions.cs b/src/Components/Components/src/ParameterCollectionExtensions.cs index 436f8f6d29dd..323662f403f2 100644 --- a/src/Components/Components/src/ParameterCollectionExtensions.cs +++ b/src/Components/Components/src/ParameterCollectionExtensions.cs @@ -25,7 +25,7 @@ private readonly static ConcurrentDictionary _cachedWriter /// /// The . /// An object that has a public writable property matching each parameter's name and type. - public unsafe static void SetParameterProperties( + public static void SetParameterProperties( in this ParameterCollection parameterCollection, object target) { @@ -41,17 +41,6 @@ public unsafe static void SetParameterProperties( _cachedWritersByType[targetType] = writers; } - // We only want to iterate through the parameterCollection once, and by the end of it, - // need to have tracked which of the parameter properties haven't yet been written. - // To avoid allocating any list/dictionary to track that, here we stackalloc an array - // of flags and set them based on the indices of the writers we use. - var numWriters = writers.WritersByIndex.Count; - var numUsedWriters = 0; - - // TODO: Once we're able to move to netstandard2.1, this can be changed to be - // a Span and then the enclosing method no longer needs to be 'unsafe' - bool* usageFlags = stackalloc bool[numWriters]; - foreach (var parameter in parameterCollection) { var parameterName = parameter.Name; @@ -62,9 +51,7 @@ public unsafe static void SetParameterProperties( try { - writerWithIndex.Writer.SetValue(target, parameter.Value); - usageFlags[writerWithIndex.Index] = true; - numUsedWriters++; + writerWithIndex.SetValue(target, parameter.Value); } catch (Exception ex) { @@ -73,23 +60,6 @@ public unsafe static void SetParameterProperties( $"type '{target.GetType().FullName}'. The error was: {ex.Message}", ex); } } - - // Now we can determine whether any writers have not been used, and if there are - // some unused ones, find them. - for (var index = 0; numUsedWriters < numWriters; index++) - { - if (index >= numWriters) - { - // This should not be possible - throw new InvalidOperationException("Ran out of writers before marking them all as used."); - } - - if (!usageFlags[index]) - { - writers.WritersByIndex[index].SetDefaultValue(target); - numUsedWriters++; - } - } } internal static IEnumerable GetCandidateBindableProperties(Type targetType) @@ -125,12 +95,11 @@ private static void ThrowForUnknownIncomingParameterName(Type targetType, string class WritersForType { - public Dictionary WritersByName { get; } - public List WritersByIndex { get; } + public Dictionary WritersByName { get; } public WritersForType(Type targetType) { - var propertySettersByName = new Dictionary(StringComparer.OrdinalIgnoreCase); + WritersByName = new Dictionary(StringComparer.OrdinalIgnoreCase); foreach (var propertyInfo in GetCandidateBindableProperties(targetType)) { var shouldCreateWriter = propertyInfo.IsDefined(typeof(ParameterAttribute)) @@ -143,24 +112,14 @@ public WritersForType(Type targetType) var propertySetter = MemberAssignment.CreatePropertySetter(targetType, propertyInfo); var propertyName = propertyInfo.Name; - if (propertySettersByName.ContainsKey(propertyName)) + if (WritersByName.ContainsKey(propertyName)) { throw new InvalidOperationException( $"The type '{targetType.FullName}' declares more than one parameter matching the " + $"name '{propertyName.ToLowerInvariant()}'. Parameter names are case-insensitive and must be unique."); } - propertySettersByName.Add(propertyName, propertySetter); - } - - // Now we know all the entries, construct the resulting list/dictionary - // with well-defined indices - WritersByIndex = new List(); - WritersByName = new Dictionary(StringComparer.OrdinalIgnoreCase); - foreach (var pair in propertySettersByName) - { - WritersByName.Add(pair.Key, (WritersByIndex.Count, pair.Value)); - WritersByIndex.Add(pair.Value); + WritersByName.Add(propertyName, propertySetter); } } } diff --git a/src/Components/Components/src/Reflection/IPropertySetter.cs b/src/Components/Components/src/Reflection/IPropertySetter.cs index 8cb862433f59..575b2e669b4d 100644 --- a/src/Components/Components/src/Reflection/IPropertySetter.cs +++ b/src/Components/Components/src/Reflection/IPropertySetter.cs @@ -6,7 +6,5 @@ namespace Microsoft.AspNetCore.Components.Reflection internal interface IPropertySetter { void SetValue(object target, object value); - - void SetDefaultValue(object target); } } diff --git a/src/Components/Components/src/Reflection/MemberAssignment.cs b/src/Components/Components/src/Reflection/MemberAssignment.cs index ede570059599..46d2b8056944 100644 --- a/src/Components/Components/src/Reflection/MemberAssignment.cs +++ b/src/Components/Components/src/Reflection/MemberAssignment.cs @@ -48,14 +48,10 @@ public PropertySetter(MethodInfo setMethod) { _setterDelegate = (Action)Delegate.CreateDelegate( typeof(Action), setMethod); - var propertyType = typeof(TValue); } public void SetValue(object target, object value) => _setterDelegate((TTarget)target, (TValue)value); - - public void SetDefaultValue(object target) - => _setterDelegate((TTarget)target, default); } } } diff --git a/src/Components/Components/test/ParameterCollectionAssignmentExtensionsTest.cs b/src/Components/Components/test/ParameterCollectionAssignmentExtensionsTest.cs index 24c2d832c906..44c58dd4d6a1 100644 --- a/src/Components/Components/test/ParameterCollectionAssignmentExtensionsTest.cs +++ b/src/Components/Components/test/ParameterCollectionAssignmentExtensionsTest.cs @@ -73,7 +73,7 @@ public void IncomingParameterMatchesInheritedDeclaredParameter_SetsValue() } [Fact] - public void NoIncomingParameterMatchesDeclaredParameter_SetValuesDefault() + public void NoIncomingParameterMatchesDeclaredParameter_LeavesValueUnchanged() { // Arrange var existingObjectValue = new object(); @@ -90,9 +90,9 @@ public void NoIncomingParameterMatchesDeclaredParameter_SetValuesDefault() parameterCollection.SetParameterProperties(target); // Assert - Assert.Equal(0, target.IntProp); - Assert.Null(target.StringProp); - Assert.Null(target.ObjectPropCurrentValue); + Assert.Equal(456, target.IntProp); + Assert.Equal("Existing value", target.StringProp); + Assert.Same(existingObjectValue, target.ObjectPropCurrentValue); } [Fact] diff --git a/src/Components/test/E2ETest/Tests/RoutingTest.cs b/src/Components/test/E2ETest/Tests/RoutingTest.cs index a060b1734f69..6605c729e311 100644 --- a/src/Components/test/E2ETest/Tests/RoutingTest.cs +++ b/src/Components/test/E2ETest/Tests/RoutingTest.cs @@ -221,6 +221,10 @@ public void CanFollowLinkToPageWithParameters() AssertHighlightedLinks("With parameters", "With more parameters"); // Can remove parameters while remaining on same page + // WARNING: This only works because the WithParameters component overrides SetParametersAsync + // and explicitly resets its parameters to default when each new set of parameters arrives. + // Without that, the page would retain the old value. + // See https://github.com/aspnet/AspNetCore/issues/6864 where we reverted the logic to auto-reset. app.FindElement(By.LinkText("With parameters")).Click(); WaitAssert.Equal("Your full name is Abc .", () => app.FindElement(By.Id("test-info")).Text); AssertHighlightedLinks("With parameters"); diff --git a/src/Components/test/testassets/BasicTestApp/RouterTest/WithParameters.cshtml b/src/Components/test/testassets/BasicTestApp/RouterTest/WithParameters.cshtml index 68f540fcaa2d..2caf8c67bb53 100644 --- a/src/Components/test/testassets/BasicTestApp/RouterTest/WithParameters.cshtml +++ b/src/Components/test/testassets/BasicTestApp/RouterTest/WithParameters.cshtml @@ -5,9 +5,15 @@ @functions { - [Parameter] - string FirstName { get; set; } + [Parameter] string FirstName { get; set; } - [Parameter] - string LastName { get ; set; } + [Parameter] string LastName { get ; set; } + + public override Task SetParametersAsync(ParameterCollection parameters) + { + // Manually reset parameters to defaults so we don't retain any from an earlier URL + FirstName = default; + LastName = default; + return base.SetParametersAsync(parameters); + } } diff --git a/src/Components/test/testassets/ComponentsApp.App/Pages/FetchData.cshtml b/src/Components/test/testassets/ComponentsApp.App/Pages/FetchData.cshtml index 30677449d683..425ac534dfb5 100644 --- a/src/Components/test/testassets/ComponentsApp.App/Pages/FetchData.cshtml +++ b/src/Components/test/testassets/ComponentsApp.App/Pages/FetchData.cshtml @@ -48,14 +48,14 @@ else WeatherForecast[] forecasts; - protected override async Task OnParametersSetAsync() + public override Task SetParametersAsync(ParameterCollection parameters) { - // If no value was given in the URL for StartDate, apply a default - if (StartDate == default) - { - StartDate = DateTime.Now; - } + StartDate = DateTime.Now; + return base.SetParametersAsync(parameters); + } + protected override async Task OnParametersSetAsync() + { forecasts = await ForecastService.GetForecastAsync(StartDate); } }