Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions src/Components/Components/src/Routing/Router.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down Expand Up @@ -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.
Expand All @@ -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);

Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/Components/Web.JS/dist/Release/blazor.webassembly.js

Large diffs are not rendered by default.

17 changes: 12 additions & 5 deletions src/Components/Web.JS/src/Platform/Mono/MonoPlatform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noice

}

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be returned?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I don't see how this can work asynchronously without doing so.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We return a promise in L305. This is the same thing we were doing before for this scenario.

Do you mean that we need to return a Promise in the scenarios where we are currently throwing an error?

This is the main change in this commit since we previously returned a dummy Promise.resolve(0) if the parameter was invalid.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, yes I think you're right @captainsafia. There was something about the diff (the removal of line 348) that made it look as if we were now discarding this promise. I didn't read it clearly enough.

.map(assembly => resourceLoader.loadResource(assembly, `_framework/${assembly}`, lazyAssemblies[assembly], 'assembly'))
.map(async resource => (await resource.response).arrayBuffer()));

Expand All @@ -345,8 +354,6 @@ function createEmscriptenModuleInstance(resourceLoader: WebAssemblyResourceLoade
return resourcesToLoad.length;
}));
}
return BINDING.js_to_mono_obj(Promise.resolve(0));
}
});

module.postRun.push(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" />.
Expand Down Expand Up @@ -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));
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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>
Expand All @@ -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,
Expand All @@ -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;
Expand Down
28 changes: 28 additions & 0 deletions src/Components/test/E2ETest/Tests/WebAssemblyLazyLoadTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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);
});
}
}
}
}
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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);
}
}
}
Expand Down