Skip to content

Conversation

@pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Oct 16, 2021

  • encapsulate emscripten js runtime into IFFE, so that it doesn't leak into global namespace
  • keep Module object in the global namespace
  • keep MONO, BINDING, cwrap, addRunDependency, removeRunDependency in the global namespace with deprecation warning
  • improve test loading script

@pavelsavara pavelsavara added the arch-wasm WebAssembly architecture label Oct 16, 2021
@pavelsavara pavelsavara added this to the 7.0.0 milestone Oct 16, 2021
@pavelsavara pavelsavara requested review from kg and lewing October 16, 2021 14:18
@ghost
Copy link

ghost commented Oct 16, 2021

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

Issue Details
  • encapsulate emscripten js runtime into IFFE, so that it doesn't leak into global namespace
  • keep Module object in the global namespace
  • keep MONO,BINDING,cwrap,addRunDependency,removeRunDependency in the global namespace with deprecation warning
  • improve test loading script
Author: pavelsavara
Assignees: -
Labels:

arch-wasm

Milestone: 7.0.0

@kg
Copy link
Member

kg commented Oct 16, 2021

Changing everything from INTERNAL to Module.INTERNAL feels like an enormous step backward, what are we getting out of this?

@pavelsavara
Copy link
Member Author

pavelsavara commented Oct 18, 2021

This all is in preparation for ES6 modularization and no variable in global namespace. Intermediate step.

Both are good thing (tm). One of possible motivations is to be able to instantiate dotnet multiple times per web page as independent components. Perhaps WebComponents.

We could have CommonJS module and thin ES6 wrapper. Or we could switch completely.
We could keep global variables in place, for consumers who don't care for multiple instances. We actually already have Module.no_global_exports flag for that which is false by default right now.

If/When we switch to ES6 module it would be breaking change for sure.
It would be also opportunity to flatten some of the MONO, BINDING into top-most export.
Also possibly nest the emscripten C runtime into WASM namespace.

But I'm not that far with prototyping yet. There are following challenges:

  • the initialization of emscripten runtime expects Module object with various extension points. We could present that configurability multiple ways. But key concern is that import could not accept parameters. That's why modularized emscripten exports a factory instead of module's static functions.
  • the other concern which needs to be explored is startup sequence
    • there is our current startup/configuration pattern with loading config and Mono.config.loaded_cb()
    • Blazor startup sequence, based on mono_wasm_invoke_js_blazor and Module.instantiateWasm. Parallel loading, custom manifest with files hash. Customized icu setup.
    • the default ES6 promise which emscripten offer. We don't have to take it, but perhaps it's there for good reasons. It seems to resolve only when whole application is ready.
  • our own code needs to be able to find the non-global runtime instance. For example in debugger code and in C# of JS interop.

So maybe we will have

import { createRuntime } from 'dotnet-runtime';

const runtimeModule = ​await createRuntimeModule({
   ​onRuntimeInitialized: function () { ... }
});

const { mono_wasm_setenv, mono_bind_static_method, INTERNAL, Module } = runtimeModule;

mono_wasm_setenv('NAME','value');
INTERNAL.mono_wasm_detach_debugger();
Module.cwrap('something');

@kg
Copy link
Member

kg commented Oct 18, 2021

Your example of the final state looks good, but this interim state feels unacceptable. It churns all this code and we'll just have to change it all again later. Is there some way we can stage this without making everything gross in the interim?

@pavelsavara
Copy link
Member Author

Yes, it could be in the branch the whole time :-D

@kg
Copy link
Member

kg commented Oct 19, 2021

If that's unavoidable, so be it. Is there some way we could auto-export these symbols from Module into the global scope as a default, so that old code doesn't break? Then we smoothly transition stuff to the clean new approach one at a time?

Squashed commit:

[7790da7c125] - unwraped mono_wasm_invoke_js from C into ts
- replaced eval with Function constructor and closure

[df70b86f69c] wip

[e5b9ddceeaa] remove obsolete blazor

[2d742949f3a] wip

[abed5e9db3e] - encapsulate emscripten js runtime into IFFE, so that it doesn't leak into global namespace
- keep Module object in the global namespace
- keep MONO,BINDING,cwrap,addRunDependency,removeRunDependency in the global namespace with deprecation warning
- improve test loading script
@pavelsavara pavelsavara closed this Nov 1, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Dec 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasm WebAssembly architecture

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants