Breaking change in WebAssembly loadBootResource

[SteveSandersonMS]

The change in PR #89435 (merged a few days ago) appears not to be back-compatible and is probably causing the SDK ingestion dotnet/aspnetcore#49704 to fail.

Specifically, when calling the application's loadBootResource callback, it now passes different values for the type parameter, and imposes new rules that are stricter than we had in the past. Existing applications' loadBootResource implementations may not support these new types/rules.

As a concrete example, see the Blazor docs at https://learn.microsoft.com/en-us/aspnet/core/blazor/fundamentals/startup?view=aspnetcore-7.0. This page contains the following code sample:

<script src="_framework/blazor.webassembly.js" autostart="false"></script>
<script>
  Blazor.start({
    loadBootResource: function (type, name, defaultUri, integrity) {
      if (type == 'dotnetjs') {
        return null;
      } else {
        return fetch(defaultUri, {
          cache: 'no-cache',
          integrity: integrity,
          headers: { 'Custom-Header': 'Custom Value' }
        });
      }
    }
  });
</script>

This works with 7.0, but will break after PR #89435 because the runtime now supplies the new type name js-module-runtime, which is not a documented type. The logic above will perform a fetch request for the resource, since the only documented type that can't be fetched is dotnetjs. Then the runtime will reject the value, giving the exception message For a js-module-runtime resource, custom loaders must supply a URI string. This is one of the build failures at https://github.com/dotnet/aspnetcore/runs/15504558324

Proposed resolution

For any resource type that you don't accept a Promise<Response> value, use the type name dotnetjs for back-compat. Avoid adding new type names that impose stricter rules than existed before.

cc @pavelsavara @maraf @lewing

[SteveSandersonMS]

Note, this isn't necessarily the only build-breaking problem introduced in this SDK update. There are also possibly some different failures, since some other tests instead fail with:

dotnet.js 2:917 "MONO_WASM: Failed to resolve module specifier 'WasmMinimal/_framework/dotnet.runtime.8.0.0-rc.1.23379.8.j5ye6ne55d.js'\nTypeError: Failed to resolve module specifier 'WasmMinimal/_framework/dotnet.runtime.8.0.0-rc.1.23379.8.j5ye6ne55d.js' [Log](https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_apis/build/builds/356713/logs/13)

[SteveSandersonMS]

Actually that indicates there's also a second breaking change. The logic in run.ts's importModules looks wrong because it assumes the URL formats will be compliant with ES dynamic import, but that's not true in general.

For example:

• import('https://some/full/url.js') is legal
• import('./a/b/url.js') is legal
• import('/a/b/url.js') is legal
• import('a/b/url.js') is not legal and will cause the Failed to resolve module specifier error

That is, URLs must be absolute or rooted with / or relative to ..

Proposed resolution

Resolve all URLs into absolute ones using the same technique that Blazor used in .NET 7:

https://github.com/dotnet/aspnetcore/blob/c64d745c863088e6bb6390984b7fe93782e686c3/src/Components/Web.JS/src/Platform/Mono/MonoPlatform.ts#L173-L174

  const absoluteSrc = (new URL(src, document.baseURI)).toString();
  return await import(/* webpackIgnore: true */ absoluteSrc);

Note that this isn't an edge case. It's going to affect just about every existing app that has a custom resource loader, and is causing quite a few test failures at dotnet/aspnetcore#49704

[pavelsavara]

For any resource type that you don't accept a Promise<Response> value, use the type name dotnetjs for back-compat. Avoid adding new type names that impose stricter rules than existed before.

We are calling that loadBootResource extension point in order to allow the 3rd party to customize the location of javascript files. But now there are more JS files than before and so we could not use same dotnetjs to distinguish them.

Edit: we are also sending the name.

The enforcement that it will return non-null is a bug, I will fix it.

[pavelsavara]

* `import('a/b/url.js')` is **not** legal and will cause the _Failed to resolve module specifier_ error

We talking about return values from loadBootResource, right ?

For values in the blazor.boot.json we call locateFile so it should be ok

runtime/src/mono/wasm/runtime/loader/polyfills.ts

Lines 49 to 56 in bd84336

	loaderHelpers.locateFile = (path) => {
	if ("URL" in globalThis && globalThis.URL !== (URLPolyfill as any)) {
	return new URL(path, loaderHelpers.scriptDirectory).toString();
	}
	if (isPathAbsolute(path)) return path;
	return loaderHelpers.scriptDirectory + path;
	};

  const absoluteSrc = (new URL(src, document.baseURI)).toString();

I agree that resolving it to absolute URL is the right thing to do.
But the import has "current script directory" in the locations of the calling script that is ./a.js will resolve to _framework/a.js.
To fix the unqualified case, we have to make assumption about script directory, in which case the document.baseURI doesn't match normal import behavior.

I think somehow <StaticWebAssetBasePath>WasmMinimal</StaticWebAssetBasePath> is causing it, but I need to read more code yet.

The alternative solution is to fail with better message.

[pavelsavara]

To fix the unqualified case, we have to make assumption about script directory, in which case the document.baseURI doesn't match normal import behavior.

It seems we already have this behavior for some time. I will implement it consistently with document.baseURI and document it.

[SteveSandersonMS]

But now there are more JS files than before and so we could not use same dotnetjs to distinguish them.
Edit: we are also sending the name.
The enforcement that it will return non-null is a bug, I will fix it.

I'm not certain exactly what you're saying you'll fix here, but there are existing implementations of the loadBootResource callback out in the wild (such as the one in our docs, shown above) which we need not to break.

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

Issue Details

---

The change in PR #89435 (merged a few days ago) appears not to be back-compatible and is probably causing the SDK ingestion dotnet/aspnetcore#49704 to fail.

Specifically, when calling the application's loadBootResource callback, it now passes different values for the type parameter, and imposes new rules that are stricter than we had in the past. Existing applications' loadBootResource implementations may not support these new types/rules.

As a concrete example, see the Blazor docs at https://learn.microsoft.com/en-us/aspnet/core/blazor/fundamentals/startup?view=aspnetcore-7.0. This page contains the following code sample:

<script src="_framework/blazor.webassembly.js" autostart="false"></script>
<script>
  Blazor.start({
    loadBootResource: function (type, name, defaultUri, integrity) {
      if (type == 'dotnetjs') {
        return null;
      } else {
        return fetch(defaultUri, {
          cache: 'no-cache',
          integrity: integrity,
          headers: { 'Custom-Header': 'Custom Value' }
        });
      }
    }
  });
</script>

This works with 7.0, but will break after PR #89435 because the runtime now supplies the new type name js-module-runtime, which is not a documented type. The logic above will perform a fetch request for the resource, since the only documented type that can't be fetched is dotnetjs. Then the runtime will reject the value, giving the exception message For a js-module-runtime resource, custom loaders must supply a URI string. This is one of the build failures at https://github.com/dotnet/aspnetcore/runs/15504558324

Proposed resolution

For any resource type that you don't accept a Promise<Response> value, use the type name dotnetjs for back-compat. Avoid adding new type names that impose stricter rules than existed before.

cc @pavelsavara @maraf @lewing

Author:	SteveSandersonMS
Assignees:	-
Labels:	arch-wasm, untriaged, area-VM-meta-mono, needs-area-label
Milestone:	-