Follow-ups to lazy-load from API review (#24169)

* Follow-ups to lazy-load from API review

* Address feedback from peer review

Author: captainsafia

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

@@ -68,12 +68,12 @@ static readonly ReadOnlyDictionary<string, object> _emptyParametersDictionary
         /// <summary>
         /// Get or sets the content to display when asynchronous navigation is in progress.
         /// </summary>
-        [Parameter] public RenderFragment Navigating { get; set; }
+        [Parameter] public RenderFragment? Navigating { get; set; }
 
         /// <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 Func<NavigationContext, Task>? OnNavigateAsync { get; set; }
 
         private RouteTable Routes { get; set; }
 
@@ -195,10 +195,6 @@ internal virtual void Refresh(bool isNavigationIntercepted)
 
         private async ValueTask<bool> RunOnNavigateAsync(string path, Task previousOnNavigate)
         {
-            if (OnNavigateAsync == null)
-            {
-                return true;
-            }
 
             // Cancel the CTS instead of disposing it, since disposing does not
             // actually cancel and can cause unintended Object Disposed Exceptions.
@@ -210,6 +206,11 @@ private async ValueTask<bool> RunOnNavigateAsync(string path, Task previousOnNav
             // invocation.
             await previousOnNavigate;
 
+            if (OnNavigateAsync == null)
+            {
+                return true;
+            }
+
             _onNavigateCts = new CancellationTokenSource();
             var navigateContext = new NavigationContext(path, _onNavigateCts.Token);
 
@@ -227,14 +228,12 @@ private async ValueTask<bool> RunOnNavigateAsync(string path, Task previousOnNav
                 if (e.CancellationToken != navigateContext.CancellationToken)
                 {
                     var rethrownException = new InvalidOperationException("OnNavigateAsync can only be cancelled via NavigateContext.CancellationToken.", e);
-                    _renderHandle.Render(builder => ExceptionDispatchInfo.Capture(rethrownException).Throw());
-                    return false;
+                    _renderHandle.Render(builder => ExceptionDispatchInfo.Throw(rethrownException));
                 }
             }
             catch (Exception e)
             {
-                _renderHandle.Render(builder => ExceptionDispatchInfo.Capture(e).Throw());
-                return false;
+                _renderHandle.Render(builder => ExceptionDispatchInfo.Throw(e));
             }
 
             return false;

src/Components/Web.JS/dist/Release/blazor.webassembly.js

@@ -324,9 +324,18 @@ function createEmscriptenModuleInstance(resourceLoader: WebAssemblyResourceLoade
       const assembliesToLoad = BINDING.mono_array_to_js_array<System_String, string>(assembliesToLoadDotNetArray);
       const lazyAssemblies = resourceLoader.bootConfig.resources.lazyAssembly;
 
-      if (lazyAssemblies) {
-        const resourcePromises = Promise.all(assembliesToLoad
-            .filter(assembly => lazyAssemblies.hasOwnProperty(assembly))
+      if (!lazyAssemblies) {
+        throw new Error("No assemblies have been marked as lazy-loadable. Use the 'BlazorWebAssemblyLazyLoad' item group in your project file to enable lazy loading an assembly.");
+      }
+
+      var assembliesMarkedAsLazy = assembliesToLoad.filter(assembly => lazyAssemblies.hasOwnProperty(assembly));
+
+      if (assembliesMarkedAsLazy.length != assembliesToLoad.length) {
+        var notMarked = assembliesToLoad.filter(assembly => !assembliesMarkedAsLazy.includes(assembly));
+        throw new Error(`${notMarked.join()} must be marked with 'BlazorWebAssemblyLazyLoad' item group in your project file to allow lazy-loading.`);
+      }
+
+      const resourcePromises = Promise.all(assembliesMarkedAsLazy
             .map(assembly => resourceLoader.loadResource(assembly, `_framework/${assembly}`, lazyAssemblies[assembly], 'assembly'))
             .map(async resource => (await resource.response).arrayBuffer()));
 
@@ -345,8 +354,6 @@ function createEmscriptenModuleInstance(resourceLoader: WebAssemblyResourceLoade
           return resourcesToLoad.length;
         }));
       }
-      return BINDING.js_to_mono_obj(Promise.resolve(0));
-    }
   });
 
   module.postRun.push(() => {

src/Components/Web.JS/src/Platform/Mono/MonoPlatform.ts

@@ -130,7 +130,7 @@ private WebAssemblyHostEnvironment InitializeEnvironment(WebAssemblyJSRuntimeInv
         /// <summary>
         /// Gets the logging builder for configuring logging services.
         /// </summary>
-        public ILoggingBuilder Logging { get;  }
+        public ILoggingBuilder Logging { get; }
 
         /// <summary>
         /// Registers a <see cref="IServiceProviderFactory{TBuilder}" /> instance to be used to create the <see cref="IServiceProvider" />.
@@ -189,7 +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.AddSingleton(new LazyAssemblyLoader(DefaultWebAssemblyJSRuntime.Instance));
             Services.AddLogging(builder => {
                 builder.AddProvider(new WebAssemblyConsoleLoggerProvider(DefaultWebAssemblyJSRuntime.Instance));
             });

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

@@ -9,7 +9,6 @@
 using System.Runtime.InteropServices;
 using System.Runtime.Loader;
 using System.Threading.Tasks;
-using Microsoft.Extensions.DependencyInjection;
 using Microsoft.JSInterop;
 using Microsoft.JSInterop.WebAssembly;
 
@@ -20,19 +19,18 @@ namespace Microsoft.AspNetCore.Components.WebAssembly.Services
     ///
     /// Supports finding pre-loaded assemblies in a server or pre-rendering context.
     /// </summary>
-    public class LazyAssemblyLoader
+    public sealed class LazyAssemblyLoader
     {
         internal const string GetDynamicAssemblies = "window.Blazor._internal.getLazyAssemblies";
         internal const string ReadDynamicAssemblies = "window.Blazor._internal.readLazyAssemblies";
 
-        private List<Assembly> _loadedAssemblyCache = new List<Assembly>();
+        private readonly IJSRuntime _jsRuntime;
+        private readonly HashSet<string> _loadedAssemblyCache;
 
-        private readonly IServiceProvider _provider;
-
-        public LazyAssemblyLoader(IServiceProvider provider)
+        public LazyAssemblyLoader(IJSRuntime jsRuntime)
         {
-            _provider = provider;
-            _loadedAssemblyCache = AppDomain.CurrentDomain.GetAssemblies().ToList();
+            _jsRuntime = jsRuntime;
+            _loadedAssemblyCache = AppDomain.CurrentDomain.GetAssemblies().Select(a => a.GetName().Name + ".dll").ToHashSet();
         }
 
         /// <summary>
@@ -55,37 +53,45 @@ public async Task<IEnumerable<Assembly>> LoadAssembliesAsync(IEnumerable<string>
 
         private Task<IEnumerable<Assembly>> LoadAssembliesInServerAsync(IEnumerable<string> assembliesToLoad)
         {
-            var loadedAssemblies = _loadedAssemblyCache.Where(assembly =>
-                assembliesToLoad.Contains(assembly.GetName().Name + ".dll"));
+            var loadedAssemblies = new List<Assembly>();
 
-            if (loadedAssemblies.Count() != assembliesToLoad.Count())
+            try
+            {
+                foreach (var assemblyName in assembliesToLoad)
+                {
+                    loadedAssemblies.Add(Assembly.Load(Path.GetFileNameWithoutExtension(assemblyName)));
+                }
+            }
+            catch (FileNotFoundException ex)
             {
-                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.");
+                throw new InvalidOperationException($"Unable to find the following assembly: {ex.FileName}. Make sure that the appplication is referencing the assemblies and that they are present in the output folder.");
             }
 
-            return Task.FromResult(loadedAssemblies);
+            return Task.FromResult<IEnumerable<Assembly>>(loadedAssemblies);
         }
 
         private async Task<IEnumerable<Assembly>> LoadAssembliesInClientAsync(IEnumerable<string> assembliesToLoad)
         {
-            var jsRuntime = _provider.GetRequiredService<IJSRuntime>();
-            // Only load assemblies that haven't already been lazily-loaded
-            var newAssembliesToLoad = assembliesToLoad.Except(_loadedAssemblyCache.Select(a => a.GetName().Name + ".dll"));
+            // Check to see if the assembly has already been loaded and avoids reloading it if so.
+            // Note: in the future, as an extra precuation, we can call `Assembly.Load` and check
+            // to see if it throws FileNotFound to ensure that an assembly hasn't been loaded
+            // between when the cache of loaded assemblies was instantiated in the constructor
+            // and the invocation of this method.
+            var newAssembliesToLoad = assembliesToLoad.Where(assembly => !_loadedAssemblyCache.Contains(assembly));
             var loadedAssemblies = new List<Assembly>();
 
-            var count = (int)await ((WebAssemblyJSRuntime)jsRuntime).InvokeUnmarshalled<string[], object, object, Task<object>>(
-                GetDynamicAssemblies,
-                assembliesToLoad.ToArray(),
-                null,
-                null);
+            var count = (int)await ((WebAssemblyJSRuntime)_jsRuntime).InvokeUnmarshalled<string[], object, object, Task<object>>(
+               GetDynamicAssemblies,
+               newAssembliesToLoad.ToArray(),
+               null,
+               null);
 
             if (count == 0)
             {
                 return loadedAssemblies;
             }
 
-            var assemblies = ((WebAssemblyJSRuntime)jsRuntime).InvokeUnmarshalled<object, object, object, object[]>(
+            var assemblies = ((WebAssemblyJSRuntime)_jsRuntime).InvokeUnmarshalled<object, object, object, object[]>(
                 ReadDynamicAssemblies,
                 null,
                 null,
@@ -99,7 +105,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);
+                _loadedAssemblyCache.Add(loadedAssembly.GetName().Name + ".dll");
             }
 
             return loadedAssemblies;

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

@@ -111,6 +111,21 @@ public void CanLazyLoadAssemblyWithRoutes()
             Assert.True(renderedElement.Displayed);
         }
 
+        [Fact]
+        public void ThrowsErrorForUnavailableAssemblies()
+        {
+            // Navigate to a page with lazy loaded assemblies for the first time
+            SetUrlViaPushState("/Other");
+            var app = Browser.MountTestComponent<TestRouterWithLazyAssembly>();
+
+            // Should've thrown an error for unhandled error
+            var errorUiElem = Browser.Exists(By.Id("blazor-error-ui"), TimeSpan.FromSeconds(10));
+            Assert.NotNull(errorUiElem);
+
+
+            AssertLogContainsCriticalMessages("DoesNotExist.dll must be marked with 'BlazorWebAssemblyLazyLoad' item group in your project file to allow lazy-loading.");
+        }
+
         private string SetUrlViaPushState(string relativeUri)
         {
             var pathBaseWithoutHash = ServerPathBase.Split('#')[0];
@@ -145,5 +160,18 @@ private void AssertLogDoesNotContainCriticalMessages(params string[] messages)
                 });
             }
         }
+
+        void AssertLogContainsCriticalMessages(params string[] messages)
+        {
+            var log = Browser.Manage().Logs.GetLog(LogType.Browser);
+            foreach (var message in messages)
+            {
+                Assert.Contains(log, entry =>
+                {
+                    return entry.Level == LogLevel.Severe
+                    && entry.Message.Contains(message);
+                });
+            }
+        }
     }
 }

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

@@ -1,6 +1,6 @@
 @using Microsoft.AspNetCore.Components.Routing
 @using System.Reflection
-@using Microsoft.AspNetCore.Components.WebAssembly.Services 
+@using Microsoft.AspNetCore.Components.WebAssembly.Services
 
 @inject LazyAssemblyLoader lazyLoader
 
@@ -31,25 +31,24 @@
 
     private async Task LoadAssemblies(string uri)
     {
-        try
+        if (uri.EndsWith("WithLazyAssembly"))
         {
-            if (uri.EndsWith("WithLazyAssembly"))
-            {
-                Console.WriteLine($"Loading assemblies for WithLazyAssembly...");
-                var assemblies = await lazyLoader.LoadAssembliesAsync(new List<string>() { "Newtonsoft.Json.dll" });
-                lazyLoadedAssemblies.AddRange(assemblies);
-            }
-
-            if (uri.EndsWith("WithLazyLoadedRoutes"))
-            {
-                Console.WriteLine($"Loading assemblies for WithLazyLoadedRoutes...");
-                var assemblies = await lazyLoader.LoadAssembliesAsync(new List<string>() { "LazyTestContentPackage.dll" });
-                lazyLoadedAssemblies.AddRange(assemblies);
-            }
+            Console.WriteLine($"Loading assemblies for WithLazyAssembly...");
+            var assemblies = await lazyLoader.LoadAssembliesAsync(new List<string>() { "Newtonsoft.Json.dll" });
+            lazyLoadedAssemblies.AddRange(assemblies);
         }
-        catch (Exception e)
+
+        if (uri.EndsWith("WithLazyLoadedRoutes"))
         {
-            Console.WriteLine($"Error when loading assemblies: {e}");
+            Console.WriteLine($"Loading assemblies for WithLazyLoadedRoutes...");
+            var assemblies = await lazyLoader.LoadAssembliesAsync(new List<string>() { "LazyTestContentPackage.dll" });
+            lazyLoadedAssemblies.AddRange(assemblies);
+        }
+
+        if (uri.EndsWith("Other")) {
+            Console.WriteLine($"Loading assemblies for Other...");
+            var assemblies = await lazyLoader.LoadAssembliesAsync(new List<string>() { "DoesNotExist.dll" });
+            lazyLoadedAssemblies.AddRange(assemblies);
         }
     }
 }