Address feedback from peer review

Author: captainsafia

src/Components/Components/ref/Microsoft.AspNetCore.Components.netcoreapp.cs

@@ -576,7 +576,7 @@ public Router() { }
         [Microsoft.AspNetCore.Components.ParameterAttribute]
         public Microsoft.AspNetCore.Components.RenderFragment NotFound { [System.Runtime.CompilerServices.CompilerGeneratedAttribute] get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute] set { } }
         [Microsoft.AspNetCore.Components.ParameterAttribute]
-        public System.Func<Microsoft.AspNetCore.Components.Routing.NavigationContext, System.Threading.Tasks.Task> OnNavigateAsync { [System.Runtime.CompilerServices.CompilerGeneratedAttribute] get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute] set { } }
+        public Microsoft.AspNetCore.Components.EventCallback<Microsoft.AspNetCore.Components.Routing.NavigationContext> OnNavigateAsync { [System.Runtime.CompilerServices.CompilerGeneratedAttribute] get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute] set { } }
         public void Attach(Microsoft.AspNetCore.Components.RenderHandle renderHandle) { }
         public void Dispose() { }
         System.Threading.Tasks.Task Microsoft.AspNetCore.Components.IHandleAfterRender.OnAfterRenderAsync() { throw null; }

src/Components/Components/src/Routing/NavigationContext.cs

@@ -1,16 +1,15 @@
 // 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.
 
-using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Text;
 using System.Threading;
-using System.Threading.Tasks;
 
 namespace Microsoft.AspNetCore.Components.Routing
 {
-    public class NavigationContext
+    /// <summary>
+    /// Provides information about the current asynchronous navigation event
+    /// including the target path and the cancellation token.
+    /// </summary>
+    public sealed class NavigationContext
     {
         internal NavigationContext(string path, CancellationToken cancellationToken)
         {

src/Components/Components/src/Routing/Router.cs

@@ -10,7 +10,6 @@
 using System.Reflection;
 using System.Threading;
 using System.Threading.Tasks;
-using Microsoft.AspNetCore.Components.Rendering;
 using Microsoft.Extensions.Logging;
 
 namespace Microsoft.AspNetCore.Components.Routing
@@ -34,6 +33,8 @@ static readonly ReadOnlyDictionary<string, object> _emptyParametersDictionary
 
         private readonly HashSet<Assembly> _assemblies = new HashSet<Assembly>();
 
+        private bool _onNavigateCalled = false;
+
         [Inject] private NavigationManager NavigationManager { get; set; }
 
         [Inject] private INavigationInterception NavigationInterception { get; set; }
@@ -69,7 +70,7 @@ static readonly ReadOnlyDictionary<string, object> _emptyParametersDictionary
         /// <summary>
         /// Gets or sets a handler that should be called before navigating to a new page.
         /// </summary>
-        [Parameter] public Func<NavigationContext, Task> OnNavigateAsync { get; set; }
+        [Parameter] public EventCallback<NavigationContext> OnNavigateAsync { get; set; }
 
         private RouteTable Routes { get; set; }
 
@@ -108,13 +109,20 @@ public async Task SetParametersAsync(ParameterView parameters)
                 throw new InvalidOperationException($"The {nameof(Router)} component requires a value for the parameter {nameof(NotFound)}.");
             }
 
-            await RunOnNavigateAsync(NavigationManager.ToBaseRelativePath(_locationAbsolute), isNavigationIntercepted: false);
+            if (!_onNavigateCalled)
+            {
+                _onNavigateCalled = true;
+                await RunOnNavigateAsync(NavigationManager.ToBaseRelativePath(_locationAbsolute));
+            }
+
+            Refresh(isNavigationIntercepted: false);
         }
 
         /// <inheritdoc />
         public void Dispose()
         {
             NavigationManager.LocationChanged -= OnLocationChanged;
+            _onNavigateCts?.Dispose();
         }
 
         private static string StringUntilAny(string str, char[] chars)
@@ -129,23 +137,14 @@ private void RefreshRouteTable()
         {
             var assemblies = AdditionalAssemblies == null ? new[] { AppAssembly } : new[] { AppAssembly }.Concat(AdditionalAssemblies);
             var assembliesSet = new HashSet<Assembly>(assemblies);
-            if (_assemblies.Count != assembliesSet.Count)
+
+            if (!_assemblies.SetEquals(assembliesSet))
             {
                 Routes = RouteTableFactory.Create(assemblies);
+                _assemblies.Clear();
                 _assemblies.UnionWith(assembliesSet);
             }
-            // If the new assemblies set is the same length as the previous one,
-            // we check if they are equal-by-value and refresh the route table if
-            // not. NOTE: This is an uncommon occurrence and a cold-code path so
-            // we shouldn't end up needing to do an equal-by-value check too frequently.
-            if (_assemblies.Count == assembliesSet.Count)
-            {
-                if (!_assemblies.SetEquals(assembliesSet))
-                {
-                    Routes = RouteTableFactory.Create(assemblies);
-                    _assemblies.UnionWith(assembliesSet);
-                }
-            }
+
         }
 
         private void Refresh(bool isNavigationIntercepted)
@@ -191,33 +190,29 @@ private void Refresh(bool isNavigationIntercepted)
             }
         }
 
-        private async Task RunOnNavigateAsync(string path, bool isNavigationIntercepted)
+        private async Task RunOnNavigateAsync(string path)
         {
             // If this router instance does not provide an OnNavigateAsync parameter
             // then we render the component associated with the route as per usual.
-            if (OnNavigateAsync == null)
+            if (!OnNavigateAsync.HasDelegate)
             {
-                Refresh(isNavigationIntercepted);
                 return;
             }
 
             // If we've already invoked a task and stored its CTS, then
             // cancel the existing task.
-            if (_onNavigateCts != null && !_onNavigateCts.IsCancellationRequested)
-            {
-                _onNavigateCts.Cancel();
-            }
+            _onNavigateCts?.Dispose();
+
             // Create a new cancellation token source for this instance
             _onNavigateCts = new CancellationTokenSource();
-
             var navigateContext = new NavigationContext(path, _onNavigateCts.Token);
-            var task = OnNavigateAsync(navigateContext);
+            var task = OnNavigateAsync.InvokeAsync(navigateContext);
 
             // Create a cancellation task based on the cancellation token
             // associated with the current running task.
             var cancellationTaskSource = new TaskCompletionSource();
             navigateContext.CancellationToken.Register(() =>
-                cancellationTaskSource.TrySetCanceled(navigateContext.CancellationToken));
+                cancellationTaskSource.SetResult());
 
             // If the user provided a Navigating render fragment, then show it.
             if (Navigating != null && task.Status != TaskStatus.RanToCompletion)
@@ -226,7 +221,11 @@ private async Task RunOnNavigateAsync(string path, bool isNavigationIntercepted)
             }
 
             await Task.WhenAny(task, cancellationTaskSource.Task);
+        }
 
+        private async Task RunOnNavigateWithRefreshAsync(string path, bool isNavigationIntercepted)
+        {
+            await RunOnNavigateAsync(path);
             Refresh(isNavigationIntercepted);
         }
 
@@ -235,7 +234,7 @@ private void OnLocationChanged(object sender, LocationChangedEventArgs args)
             _locationAbsolute = args.Location;
             if (_renderHandle.IsInitialized && Routes != null)
             {
-                _ = RunOnNavigateAsync(NavigationManager.ToBaseRelativePath(_locationAbsolute), args.IsNavigationIntercepted);
+                _ = RunOnNavigateWithRefreshAsync(NavigationManager.ToBaseRelativePath(_locationAbsolute), args.IsNavigationIntercepted);
             }
         }
 

src/Components/WebAssembly/WebAssembly/src/Hosting/WebAssemblyHostBuilder.cs

@@ -2,7 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using Microsoft.AspNetCore.Components.Routing;
@@ -11,7 +10,6 @@
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.Configuration.Json;
 using Microsoft.Extensions.DependencyInjection;
-using Microsoft.Extensions.DependencyInjection.Extensions;
 using Microsoft.Extensions.Logging;
 using Microsoft.JSInterop;
 
@@ -191,6 +189,7 @@ internal void InitializeDefaultServices()
             Services.AddSingleton<IJSRuntime>(DefaultWebAssemblyJSRuntime.Instance);
             Services.AddSingleton<NavigationManager>(WebAssemblyNavigationManager.Instance);
             Services.AddSingleton<INavigationInterception>(WebAssemblyNavigationInterception.Instance);
+            Services.AddSingleton(provider => new LazyAssemblyLoader(provider));
             Services.AddLogging(builder => {
                 builder.AddProvider(new WebAssemblyConsoleLoggerProvider(DefaultWebAssemblyJSRuntime.Instance));
             });

src/Components/WebAssembly/WebAssembly/src/Services/LazyAssemblyLoader.cs

@@ -2,33 +2,47 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.IO;
 using System.Collections.Generic;
+using System.IO;
 using System.Linq;
 using System.Reflection;
-using System.Threading.Tasks;
 using System.Runtime.InteropServices;
 using System.Runtime.Loader;
-using Microsoft.JSInterop.WebAssembly;
+using System.Threading.Tasks;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.JSInterop;
+using Microsoft.JSInterop.WebAssembly;
 
 namespace Microsoft.AspNetCore.Components.WebAssembly.Services
 {
+    /// <summary>
+    /// Provides a service for loading assemblies at runtime in a browser context.
+    ///
+    /// Supports finding pre-loaded assemblies in a server or pre-rendering context.
+    /// </summary>
     public class LazyAssemblyLoader
     {
         internal const string GetDynamicAssemblies = "window.Blazor._internal.getLazyAssemblies";
         internal const string ReadDynamicAssemblies = "window.Blazor._internal.readLazyAssemblies";
 
-        private List<string> _loadedAssemblyCache = new List<string>();
+        private List<Assembly> _loadedAssemblyCache = new List<Assembly>();
 
         private readonly IServiceProvider _provider;
 
         public LazyAssemblyLoader(IServiceProvider provider)
         {
             _provider = provider;
+            _loadedAssemblyCache = AppDomain.CurrentDomain.GetAssemblies().ToList();
         }
 
+        /// <summary>
+        /// In a browser context, calling this method will fetch the assemblies requested
+        /// via a network call and load them into the runtime. In a server or pre-rendered
+        /// context, this method will look for the assemblies already loaded in the runtime
+        /// and return them.
+        /// </summary>
+        /// <param name="assembliesToLoad">The names of the assemblies to load (e.g. "MyAssembly.dll")</param>
+        /// <returns>A list of the loaded <see cref="Assembly"/></returns>
         public async Task<IEnumerable<Assembly>> LoadAssembliesAsync(IEnumerable<string> assembliesToLoad)
         {
             if (RuntimeInformation.IsOSPlatform(OSPlatform.Browser))
@@ -41,26 +55,26 @@ public async Task<IEnumerable<Assembly>> LoadAssembliesAsync(IEnumerable<string>
 
         private Task<IEnumerable<Assembly>> LoadAssembliesInServerAsync(IEnumerable<string> assembliesToLoad)
         {
-            var assemblies = AppDomain.CurrentDomain.GetAssemblies().Where(assembly =>
+            var loadedAssemblies = _loadedAssemblyCache.Where(assembly =>
                 assembliesToLoad.Contains(assembly.GetName().Name + ".dll"));
 
-            if (assemblies.Count() != assembliesToLoad.Count())
+            if (loadedAssemblies.Count() != assembliesToLoad.Count())
             {
-                var unloadedAssemblies = assembliesToLoad.Except(assemblies.Select(assembly => assembly.GetName().Name + ".dll"));
-                throw new FileNotFoundException($"Unable to find the following assemblies: {string.Join(",", unloadedAssemblies)}");
+                var unloadedAssemblies = assembliesToLoad.Except(loadedAssemblies.Select(a => a.GetName().Name + ".dll"));
+                throw new InvalidOperationException($"Unable to find the following assemblies: {string.Join(",", unloadedAssemblies)}. Make sure that the appplication is referencing the assemblies and that they are present in the output folder.");
             }
 
-            return Task.FromResult(assemblies);
+            return Task.FromResult(loadedAssemblies);
         }
 
         private async Task<IEnumerable<Assembly>> LoadAssembliesInClientAsync(IEnumerable<string> assembliesToLoad)
         {
-            var _jsRuntime = _provider.GetRequiredService<IJSRuntime>();
+            var jsRuntime = _provider.GetRequiredService<IJSRuntime>();
             // Only load assemblies that haven't already been lazily-loaded
-            var newAssembliesToLoad = assembliesToLoad.Where(assembly => !_loadedAssemblyCache.Contains(assembly));
+            var newAssembliesToLoad = assembliesToLoad.Except(_loadedAssemblyCache.Select(a => a.GetName().Name + ".dll"));
             var loadedAssemblies = new List<Assembly>();
 
-            var count = (int)await ((WebAssemblyJSRuntime)_jsRuntime).InvokeUnmarshalled<string[], object, object, Task<object>>(
+            var count = (int)await ((WebAssemblyJSRuntime)jsRuntime).InvokeUnmarshalled<string[], object, object, Task<object>>(
                 GetDynamicAssemblies,
                 assembliesToLoad.ToArray(),
                 null,
@@ -71,7 +85,7 @@ private async Task<IEnumerable<Assembly>> LoadAssembliesInClientAsync(IEnumerabl
                 return loadedAssemblies;
             }
 
-            var assemblies = ((WebAssemblyJSRuntime)_jsRuntime).InvokeUnmarshalled<object, object, object, object[]>(
+            var assemblies = ((WebAssemblyJSRuntime)jsRuntime).InvokeUnmarshalled<object, object, object, object[]>(
                 ReadDynamicAssemblies,
                 null,
                 null,
@@ -85,7 +99,7 @@ private async Task<IEnumerable<Assembly>> LoadAssembliesInClientAsync(IEnumerabl
                 // into the default app context.
                 var loadedAssembly = AssemblyLoadContext.Default.LoadFromStream(new MemoryStream(assembly));
                 loadedAssemblies.Add(loadedAssembly);
-                _loadedAssemblyCache.Add(loadedAssembly.GetName().Name + ".dll");
+                _loadedAssemblyCache.Add(loadedAssembly);
             }
 
             return loadedAssemblies;

src/Components/test/E2ETest/Tests/WebAssemblyLazyLoadTest.cs

@@ -65,6 +65,10 @@ public void CanLazyLoadOnFirstVisit()
             SetUrlViaPushState("/WithLazyAssembly");
             var app = Browser.MountTestComponent<TestRouterWithLazyAssembly>();
 
+            // Wait for the page to finish loading
+            new WebDriverWait(Browser, TimeSpan.FromSeconds(2)).Until(
+                driver => driver.FindElement(By.Id("use-package-button")) != null);
+
             var button = app.FindElement(By.Id("use-package-button"));
 
             // We should have requested the DLL

src/Components/test/testassets/TestServer/ClientStartup.cs

@@ -1,5 +1,4 @@
 using Microsoft.AspNetCore.Builder;
-using Microsoft.AspNetCore.Components.WebAssembly.Services;
 using Microsoft.AspNetCore.Hosting;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
@@ -24,7 +23,6 @@ public void ConfigureServices(IServiceCollection services)
         {
             services.AddMvc();
             services.AddServerSideBlazor();
-            services.AddSingleton(s => new LazyAssemblyLoader(s));
         }
 
         // This method gets called by the runtime. Use this method to configure the HTTP request pipeline.

src/Components/test/testassets/TestServer/PrerenderedStartup.cs

@@ -5,6 +5,7 @@
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Hosting;
 using Microsoft.AspNetCore.Components.WebAssembly.Services;
+using Microsoft.JSInterop;
 
 namespace TestServer
 {
@@ -23,7 +24,7 @@ public void ConfigureServices(IServiceCollection services)
             services.AddMvc();
             services.AddServerSideBlazor();
             services.AddAuthentication(CookieAuthenticationDefaults.AuthenticationScheme).AddCookie();
-            services.AddSingleton(s => new LazyAssemblyLoader(s));
+            services.AddSingleton<LazyAssemblyLoader>();
         }
 
         // This method gets called by the runtime. Use this method to configure the HTTP request pipeline.