Skip to content
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

[API Proposal]: WASM DotnetHostBuilder assembly loading progress callback #93941

Open
JakeYallop opened this issue Oct 24, 2023 · 9 comments
Open
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration os-browser Browser variant of arch-wasm
Milestone

Comments

@JakeYallop
Copy link
Contributor

JakeYallop commented Oct 24, 2023

Background and motivation

The WASM runtime can take a while to load for certain kinds of apps. To provide a better experience for the user using the application, it can be useful to show a loading indicator.

ASP.NET Core Blazor did this (at least it did in .NET 7 when I was last investigating this, although looks like it may not be the case now), but it had to manually instantiate the web assembly module in order to achieve this.

Currently, there is no public API available on the DotnetHostBuilder type that enables this scenario. The public, documented approach would require manually looking up the blazor.boot.json file, and then calling createDotnetRuntime using that configuration. However, for less advanced scenarios, it would be useful to have this functionality available on the host builder.

This is already possible by using the internal withModuleConfig API, and passing in a value to the onDownloadResourceProgress callback, however this method is internal and not exposed in the typescript type files for the DotnetHostBuilder, making for a bad IDE experience. This method is mentioned in one of the advanced samples here, which is how I discovered it in the first place.

API Proposal

interface DotnetHostBuilder { 
  //exact naming TDB
  //parameters currently match those provided to the `onDownloadResourceProgress` callback available when calling `createDotnetRuntime`   
  withDownloadResourceProgress(callback: (resourcesLoaded: number, totalResources: number) => void): DotnetHostBuilder;
}

Currently, using onDownloadResourceProgress, the total parameter does not actually reflect the total number of resources that the runtime needs to load - I suggest that for this user-friendly API we provide that information rather than making the user calculate this number themselves directly from the blazor.boot.json file.

API Usage

const runtime = await dotnet
  .withDownloadResourceProgress(updateProgress)
  //...other configuration
  .create();


function updateProgress(loaded: number, total: number) {
  //do something with the information
}

Alternative Designs

  • Expose withModuleConfig publicly
    This is a much more advanced API, allowing access to lower-level emscripten and more control over how the WASM module is instantiated.
  • Alternative names
    • withDownloadResourceProgressCallback
    • withDownloadProgress
    • withInitializationProgress
    • etc.

Risks

Low - the API already exists, this would just be exposing it for use in a more user friendly manner.

@JakeYallop JakeYallop added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Oct 24, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Oct 24, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Oct 24, 2023
@maraf maraf added arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm and removed untriaged New issue has not been triaged by the area owner needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Oct 25, 2023
@maraf maraf added this to the 9.0.0 milestone Oct 25, 2023
@ghost
Copy link

ghost commented Oct 25, 2023

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

The WASM runtime can take a while to load for certain kinds of apps. To provide a better experience for the user using the application, it can be useful to show a loading indicator.

ASP.NET Core Blazor did this (at least it did in .NET 7 when I was last investigating this, although looks like it may not be the case now), but it had to manually instantiate the web assembly module in order to achieve this.

Currently, there is no public API available on the DotnetHostBuilder type that enables this scenario. The public, documented approach would require manually looking up the blazor.boot.json file, and then calling createDotnetRuntime using that configuration. However, for less advanced scenarios, it would be useful to have this functionality available on the host builder.

This is already possible by using the internal withModuleConfig API, and passing in a value to the onDownloadResourceProgress callback, however this method is internal and not exposed in the typescript type files for the DotnetHostBuilder, making for a bad IDE experience. This method is mentioned in one of the advanced samples here, which is how I discovered it in the first place.

API Proposal

interface DotnetHostBuilder { 
  //exact naming TDB
  //parameters currently match those provided to the `onDownloadResourceProgress` callback available when calling `createDotnetRuntime`   
  withDownloadResourceProgress(loaded: number, total: number): DotnetHostBuilder;
}

Currently, using onDownloadResourceProgress, the total parameter does not actually reflect the total number of resources that the runtime needs to load - I suggest that for this user-friendly API we provide that information rather than making the user calculate this number themselves directly from the blazor.boot.json file.

API Usage

const runtime = await dotnet
  .withDownloadResourceProgress(updateProgress)
  //...other configuration
  .create();


function updateProgress(loaded: number, total: number) {
  //do something with the information
}

Alternative Designs

  • Expose withModuleConfig publicly
    This is a much more advanced API, allowing access to lower-level emscripten and more control over how the WASM module is instantiated.
  • Alternative names
    • withDownloadResourceProgressCallback
    • withDownloadProgress
    • withInitializationProgress
    • etc.

Risks

Low - the API already exists, this would just be exposing it for use in a more user friendly manner.

Author: JakeYallop
Assignees: -
Labels:

api-suggestion, arch-wasm, area-System.Runtime.InteropServices.JavaScript, os-browser

Milestone: -

@maraf
Copy link
Member

maraf commented Oct 25, 2023

Thank you for the proposal. It is reasonable to expose the callback with public API.

I have fixed the propsal API typescript definition so the parameter is a callback.

@maraf
Copy link
Member

maraf commented Oct 25, 2023

cc @pavelsavara

@JakeYallop
Copy link
Contributor Author

I'd be happy to contribute a PR if it would be accepted, unless this need to go through API review first?

@maraf
Copy link
Member

maraf commented Oct 25, 2023

PR is welcome! Actually, we don't need API review for JavaScript changes

@JakeYallop
Copy link
Contributor Author

I'm thinking it would be much simpler if we had the same behaviour for this method vs the onDownloadResourceProgress callback specified in withModuleConfig.

The proposal above suggests making totalResources actually be the total number of resources that need to be loaded during initialization, however at the moment, it seems these 2 values represent

  • resourcesLoaded - the number of completed requests for loading resources
  • totalResources - the number of started requests for loading resources

mono_assert(asset.resolvedUrl, "Request's resolvedUrl must be set");
const fetchResponse = download_resource_with_cache(asset);
const response = { name: asset.name, url: asset.resolvedUrl, response: fetchResponse };
totalResources.add(asset.name!);
response.response.then(() => {
if (asset.behavior == "assembly") {
loaderHelpers.loadedAssemblies.push(asset.resolvedUrl!);
}
resourcesLoaded++;
if (loaderHelpers.onDownloadResourceProgress)
loaderHelpers.onDownloadResourceProgress(resourcesLoaded, totalResources.size);
});
return response;

What do we want to do here? Using the withModuleConfig callback, its not a big deal to ask the user to compute the number of resources that need to be loaded, as its an advanced API, however, with this new API - I'm not sure that same assumption holds. The developer using this API shouldn't need to know what the blazor.boot.json looks like, which would be the only way to compute this number. Additionally, the only way to actually get the config before loading the runtime so that this total value can be computed would mean calling withModuleConfig anyway - which defeats whole point of the proposal.

We have a few options:

  1. Unify onDownloadResourceProgress and withDownloadResourceProgress, and make totalResources return the total number of resources the runtime needs to load whilst its initialising. This would be a low-impact breaking change.
  2. Maintain 2 separate callbacks that have different behaviour - this seems kind of complicated given how the code looks so far, and would be weird, unexpected thing for the code to do.
  3. Some kind of third parameter to the callbacks, representing the total resources being loaded - this would also affect withModuleConfig setting.

@maraf
Copy link
Member

maraf commented Oct 26, 2023

Sorry, I missed the part about total number. We should definitely have just one callback, so withDownloadResourceProgress should reuse onDownloadResourceProgress. Current behavior can be considered as a bug / not ideal. IIRC it's not straightforward to get total number of resources to download, because not everything from the boot config will be downloaded (pdbs are used only in debug mode, only one icu is downloaded, memory snapshot skip dlls, etc).

We're ok accepting a PR with just the new API and opening an issue for better "total resources" computation

@JakeYallop
Copy link
Contributor Author

Ah - I did wonder about the reasoning behind it - makes a bit more sense now. Will leave the existing behaviour as is.

@stephentoub stephentoub modified the milestones: 9.0.0, 10.0.0 Jul 22, 2024
@pavelsavara pavelsavara added the needs-author-action An issue or pull request that requires more info or actions from the author. label Dec 12, 2024
@JakeYallop
Copy link
Contributor Author

I won't be working on this any time soon, but I don't think "needs author action" is the right label either - should this just be marked as "help wanted"?

@dotnet-policy-service dotnet-policy-service bot added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed needs-author-action An issue or pull request that requires more info or actions from the author. labels Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration os-browser Browser variant of arch-wasm
Projects
None yet
Development

No branches or pull requests

4 participants