Skip to content

[wasm] dotnet.js modularization #38721

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
21 commits merged into from
Jan 11, 2022
Merged

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Nov 30, 2021

Context

After dotnet.js was modularized, Blazor could start using the new API we expose.

This PR updates Blazor to use new API createDotnetRuntime()

The current default output of runtime is CommonJS module, which is at the same time backward compatible with .NET 6.0 globalThis.Module style. In the future ES6 would be default module format. This change also makes Blazor future-compatible with ES6 runtime module.

Changes

  • use dynamic import instead of HTML <script> tag to load dotnet

  • updated createEmscriptenModuleInstance to use the new createDotnetRuntime() API

  • made import of dotnet.js parallel with loading other assets.

  • added src/Components/Web.JS/@types/dotnet temporarily, until we figure out how to pull this out of Microsoft.NETCore.App.Runtime.Mono.browser-wasm.7.0.0-dev.nupkg

  • removed @types/emscripten type definitions

  • fixed usages of addRunDependency, removeRunDependency

  • mapped System_Object and other types to MonoObject etc

  • global Module is no longer needed by runtime

    • therefore removed __wasmmodulecallback__
    • Unfortunately I have no way of testing the platform which had this problem in the past.
  • because it's not ES6 yet

    • there is helper callback __onDotnetRuntimeLoaded to get the API when the CJS module is loaded
  • removed few cwrap calls and replaced them with API usage

  • Implemented DRI for dynamic import via link modulepreload

Feedback

This is also opportunity for Blazor team to give feedback about the new API.
There are more samples of the API usage here dotnet/runtime#62292

Fixes #39283
Fixes: #36805

@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Nov 30, 2021
@pavelsavara pavelsavara force-pushed the wasm_modularization_1 branch 2 times, most recently from c75d9df to ea7f1ae Compare November 30, 2021 18:55
@pavelsavara
Copy link
Member Author

cc @lewing

@pavelsavara pavelsavara force-pushed the wasm_modularization_1 branch 4 times, most recently from 947b7d5 to aa2ce47 Compare December 10, 2021 19:09
@pavelsavara pavelsavara force-pushed the wasm_modularization_1 branch 2 times, most recently from c180d9e to e9bf748 Compare December 18, 2021 10:44
@pavelsavara pavelsavara added this to the .NET 7.0 milestone Dec 18, 2021
@pavelsavara pavelsavara marked this pull request as ready for review December 18, 2021 14:36
@pavelsavara pavelsavara requested review from a team as code owners December 18, 2021 14:36
@pavelsavara pavelsavara changed the title [DRAFT][wasm] dotnet.js modularization [wasm] dotnet.js modularization Dec 18, 2021
@pavelsavara pavelsavara requested a review from wtgodbe as a code owner December 21, 2021 10:53
- updated createEmscriptenModuleInstance to use the new createDotnetRuntime() API

- removed @types/emscripten type definitions
- added src/Components/Web.JS/@types/dotnet temporarily, until we figure out how to pull this out of Microsoft.NETCore.App.Runtime.Mono.browser-wasm.7.0.0-dev.nupkg
- fixed usages of addRunDependency, removeRunDependency
- mapped System_Object and other types to MonoObject etc
- global Module is no longer needed by runtime
- therefore removed __wasmmodulecallback__
- removed few cwrap calls and replaced them with API usage
@pavelsavara pavelsavara force-pushed the wasm_modularization_1 branch from 1229563 to 6ba858e Compare December 21, 2021 10:59
Copy link
Member

@wtgodbe wtgodbe left a comment

Choose a reason for hiding this comment

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

LGTM from a build perspective, but it'd be good to get approval from someone on Blazor too

# Conflicts:
#	src/Components/Web.JS/dist/Release/blazor.server.js
#	src/Components/Web.JS/dist/Release/blazor.webview.js
#	src/Components/Web.JS/src/Platform/Mono/MonoTypes.ts
#	src/Components/Web.JS/src/Platform/Platform.ts
Copy link
Contributor

@TanayParikh TanayParikh left a comment

Choose a reason for hiding this comment

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

Looking good, some minor nits.

Tried resolving the merge conflicts, but I don't have perms to push onto your fork.

@SteveSandersonMS
Copy link
Member

Thanks @pavelsavara - I think we're just about there!

I added a couple of substantive comments about the new link modulepreload code, plus a bunch of trivial syntax tweak suggestions. Hope that's OK. Once that's resolved I think this can be merged!

Copy link
Member

@SteveSandersonMS SteveSandersonMS left a comment

Choose a reason for hiding this comment

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

Awesome, thanks so much for the updates!

@ghost
Copy link

ghost commented Jan 11, 2022

Hello @SteveSandersonMS!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@pavelsavara
Copy link
Member Author

There is some issue with IIS test, I think it's unrelated and it could be merged.

@ghost ghost merged commit 19252d6 into dotnet:main Jan 11, 2022
@pranavkm pranavkm modified the milestones: .NET 7.0, 7.0-preview1 Jan 27, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warnings when running a .NET 7 Blazor app Explore removing inline module script for Blazor Wasm
7 participants