From b101545475e6017a348f3d4bb1350ab59b5401f8 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Fri, 26 Mar 2021 07:31:27 -0700 Subject: [PATCH 1/5] Remove IReceiveHotReloadContext and HotReloadContext React to API review feedback. Substitute IReceiveHotReloadContext and HotReloadContext with RenderHandle.IsHotReloading --- .../Components/src/ComponentBase.cs | 11 ++------ .../src/HotReload/HotReloadContext.cs | 16 ----------- .../src/HotReload/IReceiveHotReloadContext.cs | 17 ------------ .../src/Properties/ILLink.Substitutions.xml | 10 ++----- .../Components/src/PublicAPI.Unshipped.txt | 6 +---- src/Components/Components/src/RenderHandle.cs | 5 ++++ .../src/RenderTree/RenderTreeDiffBuilder.cs | 7 +---- .../Components/src/RenderTree/Renderer.cs | 27 +++++-------------- 8 files changed, 18 insertions(+), 81 deletions(-) delete mode 100644 src/Components/Components/src/HotReload/HotReloadContext.cs delete mode 100644 src/Components/Components/src/HotReload/IReceiveHotReloadContext.cs diff --git a/src/Components/Components/src/ComponentBase.cs b/src/Components/Components/src/ComponentBase.cs index 56cf2f3d6138..51483977d68d 100644 --- a/src/Components/Components/src/ComponentBase.cs +++ b/src/Components/Components/src/ComponentBase.cs @@ -3,7 +3,6 @@ using System; using System.Threading.Tasks; -using Microsoft.AspNetCore.Components.HotReload; using Microsoft.AspNetCore.Components.Rendering; namespace Microsoft.AspNetCore.Components @@ -22,7 +21,7 @@ namespace Microsoft.AspNetCore.Components /// Optional base class for components. Alternatively, components may /// implement directly. /// - public abstract class ComponentBase : IComponent, IHandleEvent, IHandleAfterRender, IReceiveHotReloadContext + public abstract class ComponentBase : IComponent, IHandleEvent, IHandleAfterRender { private readonly RenderFragment _renderFragment; private RenderHandle _renderHandle; @@ -30,7 +29,6 @@ public abstract class ComponentBase : IComponent, IHandleEvent, IHandleAfterRend private bool _hasNeverRendered = true; private bool _hasPendingQueuedRender; private bool _hasCalledOnAfterRender; - private HotReloadContext? _hotReloadContext; /// /// Constructs an instance of . @@ -104,7 +102,7 @@ protected void StateHasChanged() return; } - if (_hasNeverRendered || ShouldRender() || (_hotReloadContext?.IsHotReloading ?? false)) + if (_hasNeverRendered || ShouldRender() || _renderHandle.IsHotReloading) { _hasPendingQueuedRender = true; @@ -331,10 +329,5 @@ Task IHandleAfterRender.OnAfterRenderAsync() // have to use "async void" and do their own exception handling in // the case where they want to start an async task. } - - void IReceiveHotReloadContext.Receive(HotReloadContext context) - { - _hotReloadContext = context; - } } } diff --git a/src/Components/Components/src/HotReload/HotReloadContext.cs b/src/Components/Components/src/HotReload/HotReloadContext.cs deleted file mode 100644 index 35948bdc6859..000000000000 --- a/src/Components/Components/src/HotReload/HotReloadContext.cs +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -namespace Microsoft.AspNetCore.Components.HotReload -{ - /// - /// A context that indicates when a component is being rendered because of a hot reload operation. - /// - public sealed class HotReloadContext - { - /// - /// Gets a value that indicates if the application is re-rendering in response to a hot-reload change. - /// - public bool IsHotReloading { get; internal set; } - } -} diff --git a/src/Components/Components/src/HotReload/IReceiveHotReloadContext.cs b/src/Components/Components/src/HotReload/IReceiveHotReloadContext.cs deleted file mode 100644 index 8de60e382310..000000000000 --- a/src/Components/Components/src/HotReload/IReceiveHotReloadContext.cs +++ /dev/null @@ -1,17 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -namespace Microsoft.AspNetCore.Components.HotReload -{ - /// - /// Allows a component to receive a . - /// - public interface IReceiveHotReloadContext : IComponent - { - /// - /// Configures a component to use the hot reload context. - /// - /// The hot reload context. - void Receive(HotReloadContext context); - } -} diff --git a/src/Components/Components/src/Properties/ILLink.Substitutions.xml b/src/Components/Components/src/Properties/ILLink.Substitutions.xml index edd5fb323df5..01ec769c3fb9 100644 --- a/src/Components/Components/src/Properties/ILLink.Substitutions.xml +++ b/src/Components/Components/src/Properties/ILLink.Substitutions.xml @@ -2,18 +2,12 @@ + + - - - - - - - - diff --git a/src/Components/Components/src/PublicAPI.Unshipped.txt b/src/Components/Components/src/PublicAPI.Unshipped.txt index ab4b3f271702..c66ca69e214f 100644 --- a/src/Components/Components/src/PublicAPI.Unshipped.txt +++ b/src/Components/Components/src/PublicAPI.Unshipped.txt @@ -8,11 +8,6 @@ Microsoft.AspNetCore.Components.ComponentApplicationState.PersistAsJson( Microsoft.AspNetCore.Components.ComponentApplicationState.PersistState(string! key, byte[]! value) -> void Microsoft.AspNetCore.Components.ComponentApplicationState.TryTakeAsJson(string! key, out TValue? instance) -> bool Microsoft.AspNetCore.Components.ComponentApplicationState.TryTakePersistedState(string! key, out byte[]? value) -> bool -Microsoft.AspNetCore.Components.HotReload.HotReloadContext -Microsoft.AspNetCore.Components.HotReload.HotReloadContext.HotReloadContext() -> void -Microsoft.AspNetCore.Components.HotReload.HotReloadContext.IsHotReloading.get -> bool -Microsoft.AspNetCore.Components.HotReload.IReceiveHotReloadContext -Microsoft.AspNetCore.Components.HotReload.IReceiveHotReloadContext.Receive(Microsoft.AspNetCore.Components.HotReload.HotReloadContext! context) -> void Microsoft.AspNetCore.Components.Lifetime.ComponentApplicationLifetime Microsoft.AspNetCore.Components.Lifetime.ComponentApplicationLifetime.ComponentApplicationLifetime(Microsoft.Extensions.Logging.ILogger! logger) -> void Microsoft.AspNetCore.Components.Lifetime.ComponentApplicationLifetime.PersistStateAsync(Microsoft.AspNetCore.Components.Lifetime.IComponentApplicationStateStore! store, Microsoft.AspNetCore.Components.RenderTree.Renderer! renderer) -> System.Threading.Tasks.Task! @@ -21,6 +16,7 @@ Microsoft.AspNetCore.Components.Lifetime.ComponentApplicationLifetime.State.get Microsoft.AspNetCore.Components.Lifetime.IComponentApplicationStateStore Microsoft.AspNetCore.Components.Lifetime.IComponentApplicationStateStore.GetPersistedStateAsync() -> System.Threading.Tasks.Task!>! Microsoft.AspNetCore.Components.Lifetime.IComponentApplicationStateStore.PersistStateAsync(System.Collections.Generic.IReadOnlyDictionary! state) -> System.Threading.Tasks.Task! +Microsoft.AspNetCore.Components.RenderHandle.IsHotReloading.get -> bool Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder.Dispose() -> void Microsoft.AspNetCore.Components.Routing.Router.PreferExactMatches.get -> bool Microsoft.AspNetCore.Components.Routing.Router.PreferExactMatches.set -> void diff --git a/src/Components/Components/src/RenderHandle.cs b/src/Components/Components/src/RenderHandle.cs index d6f3f986f5c3..8caa1ac70ae3 100644 --- a/src/Components/Components/src/RenderHandle.cs +++ b/src/Components/Components/src/RenderHandle.cs @@ -44,6 +44,11 @@ public Dispatcher Dispatcher public bool IsInitialized => _renderer != null; + /// + /// Gets a value that determines if the is triggering a render in response to a hot-reload change. + /// + public bool IsHotReloading => _renderer.IsHotReloading; + /// /// Notifies the renderer that the component should be rendered. /// diff --git a/src/Components/Components/src/RenderTree/RenderTreeDiffBuilder.cs b/src/Components/Components/src/RenderTree/RenderTreeDiffBuilder.cs index 674684a65367..5669fdad80fb 100644 --- a/src/Components/Components/src/RenderTree/RenderTreeDiffBuilder.cs +++ b/src/Components/Components/src/RenderTree/RenderTreeDiffBuilder.cs @@ -541,17 +541,12 @@ private static void UpdateRetainedChildComponent( var oldParameters = new ParameterView(ParameterViewLifetime.Unbound, oldTree, oldComponentIndex); var newParametersLifetime = new ParameterViewLifetime(diffContext.BatchBuilder); var newParameters = new ParameterView(newParametersLifetime, newTree, newComponentIndex); - if (!newParameters.DefinitelyEquals(oldParameters) || IsHotReloading(diffContext.Renderer)) + if (!newParameters.DefinitelyEquals(oldParameters) || diffContext.Renderer.IsHotReloading) { componentState.SetDirectParameters(newParameters); } } - /// - /// Intentionally authored as a separate method so we can trim this code. - /// - private static bool IsHotReloading(Renderer renderer) => renderer.HotReloadContext.IsHotReloading; - private static int NextSiblingIndex(in RenderTreeFrame frame, int frameIndex) { switch (frame.FrameTypeField) diff --git a/src/Components/Components/src/RenderTree/Renderer.cs b/src/Components/Components/src/RenderTree/Renderer.cs index 0748d1675754..ddcbf714b453 100644 --- a/src/Components/Components/src/RenderTree/Renderer.cs +++ b/src/Components/Components/src/RenderTree/Renderer.cs @@ -33,7 +33,6 @@ public abstract partial class Renderer : IDisposable, IAsyncDisposable private readonly Dictionary _eventHandlerIdReplacements = new Dictionary(); private readonly ILogger _logger; private readonly ComponentFactory _componentFactory; - private HotReloadContext _hotReloadContext; private HotReloadEnvironment? _hotReloadEnvironment; private List<(ComponentState, ParameterView)>? _rootComponents; @@ -102,8 +101,6 @@ public Renderer(IServiceProvider serviceProvider, ILoggerFactory loggerFactory, private void InitializeHotReload(IServiceProvider serviceProvider) { - _hotReloadContext = new(); - // HotReloadEnvironment is a test-specific feature and may not be available in a running app. We'll fallback to the default instance // if the test fixture does not provide one. _hotReloadEnvironment = serviceProvider.GetService() ?? HotReloadEnvironment.Instance; @@ -131,7 +128,10 @@ private static IComponentActivator GetComponentActivatorOrDefault(IServiceProvid /// protected internal ElementReferenceContext? ElementReferenceContext { get; protected set; } - internal HotReloadContext HotReloadContext => _hotReloadContext; + /// + /// Gets a value that determines if the is triggering a render in response to a hot-reload change. + /// + internal bool IsHotReloading { get; private set; } private async void RenderRootComponentsOnHotReload() { @@ -142,7 +142,7 @@ await Dispatcher.InvokeAsync(() => return; } - HotReloadContext.IsHotReloading = true; + IsHotReloading = true; try { foreach (var (componentState, initialParameters) in _rootComponents) @@ -152,7 +152,7 @@ await Dispatcher.InvokeAsync(() => } finally { - HotReloadContext.IsHotReloading = false; + IsHotReloading = false; } }); } @@ -164,20 +164,7 @@ await Dispatcher.InvokeAsync(() => /// The component instance. protected IComponent InstantiateComponent([DynamicallyAccessedMembers(Component)] Type componentType) { - var component = _componentFactory.InstantiateComponent(_serviceProvider, componentType); - InstatiateComponentForHotReload(component); - return component; - } - - /// - /// Intentionally authored as a separate method call so we can trim this code. - /// - private void InstatiateComponentForHotReload(IComponent component) - { - if (_hotReloadEnvironment is not null && _hotReloadEnvironment.IsHotReloadEnabled && component is IReceiveHotReloadContext receiveHotReloadContext) - { - receiveHotReloadContext.Receive(HotReloadContext); - } + return _componentFactory.InstantiateComponent(_serviceProvider, componentType); } /// From ea9fc78b21f084ae152c7ce0ce33ddc639591af7 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Fri, 26 Mar 2021 08:39:43 -0700 Subject: [PATCH 2/5] Fixup --- src/Components/Components/src/RenderHandle.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Components/Components/src/RenderHandle.cs b/src/Components/Components/src/RenderHandle.cs index 8caa1ac70ae3..23871d34962c 100644 --- a/src/Components/Components/src/RenderHandle.cs +++ b/src/Components/Components/src/RenderHandle.cs @@ -12,7 +12,7 @@ namespace Microsoft.AspNetCore.Components /// public readonly struct RenderHandle { - private readonly Renderer _renderer; + private readonly Renderer? _renderer; private readonly int _componentId; internal RenderHandle(Renderer renderer, int componentId) @@ -22,7 +22,7 @@ internal RenderHandle(Renderer renderer, int componentId) } /// - /// Gets the associated with the component. + /// Gets the associated with the component. /// public Dispatcher Dispatcher { @@ -41,13 +41,12 @@ public Dispatcher Dispatcher /// Gets a value that indicates whether the has been /// initialized and is ready to use. /// - public bool IsInitialized - => _renderer != null; + public bool IsInitialized => _renderer is not null; /// /// Gets a value that determines if the is triggering a render in response to a hot-reload change. /// - public bool IsHotReloading => _renderer.IsHotReloading; + public bool IsHotReloading => _renderer?.IsHotReloading ?? false; /// /// Notifies the renderer that the component should be rendered. @@ -66,7 +65,7 @@ public void Render(RenderFragment renderFragment) [DoesNotReturn] private static void ThrowNotInitialized() { - throw new InvalidOperationException("The render handle is not yet assigned."); + throw new InvalidOperationException("The render handle has not been initialized."); } } } From 8d4f94aeba4a0f2f691696dd3a641bb6ea88dd21 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Fri, 26 Mar 2021 14:01:00 -0700 Subject: [PATCH 3/5] Update src/Components/Components/src/RenderHandle.cs --- src/Components/Components/src/RenderHandle.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Components/Components/src/RenderHandle.cs b/src/Components/Components/src/RenderHandle.cs index 23871d34962c..3b6cf7de5290 100644 --- a/src/Components/Components/src/RenderHandle.cs +++ b/src/Components/Components/src/RenderHandle.cs @@ -65,7 +65,7 @@ public void Render(RenderFragment renderFragment) [DoesNotReturn] private static void ThrowNotInitialized() { - throw new InvalidOperationException("The render handle has not been initialized."); + throw new InvalidOperationException("The render handle is not yet assigned."); } } } From 76934f1044e16de95c73e56ada24f58c52d9ccef Mon Sep 17 00:00:00 2001 From: Pranav K Date: Mon, 29 Mar 2021 16:25:10 -0700 Subject: [PATCH 4/5] Update src/Components/Components/src/Properties/ILLink.Substitutions.xml --- .../Components/src/Properties/ILLink.Substitutions.xml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Components/Components/src/Properties/ILLink.Substitutions.xml b/src/Components/Components/src/Properties/ILLink.Substitutions.xml index 01ec769c3fb9..3727f7aa3d7e 100644 --- a/src/Components/Components/src/Properties/ILLink.Substitutions.xml +++ b/src/Components/Components/src/Properties/ILLink.Substitutions.xml @@ -9,5 +9,9 @@ + + + + From dcffe5ddb2c16105704d24392fdbaa183400dd48 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Mon, 29 Mar 2021 21:49:55 -0700 Subject: [PATCH 5/5] Update src/Components/Components/src/Properties/ILLink.Substitutions.xml --- .../Components/src/Properties/ILLink.Substitutions.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Components/Components/src/Properties/ILLink.Substitutions.xml b/src/Components/Components/src/Properties/ILLink.Substitutions.xml index 3727f7aa3d7e..724381711cb7 100644 --- a/src/Components/Components/src/Properties/ILLink.Substitutions.xml +++ b/src/Components/Components/src/Properties/ILLink.Substitutions.xml @@ -12,6 +12,6 @@ - +