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

[browser][MT] support for NodeJS #92853

Open
elringus opened this issue Sep 30, 2023 · 6 comments
Open

[browser][MT] support for NodeJS #92853

elringus opened this issue Sep 30, 2023 · 6 comments
Assignees
Labels
arch-wasm WebAssembly architecture area-VM-threading-mono os-browser Browser variant of arch-wasm
Milestone

Comments

@elringus
Copy link

elringus commented Sep 30, 2023

With <WasmEnableThreads>true</WasmEnableThreads> generated dotnet.js has assert to only be used in browser environment. When imported (w/o actually creating the runtime), following error is thrown:

This build of dotnet is multi-threaded, it doesn't support shell environments like V8 or NodeJS.

This breaks unit testing in node when the dotnet APIs are mocked and the runtime is not initalized, but still has to be imported. After removing the assertion unit testing in node works fine.

Is it possible to remove the assertion or move it to the runtime creation function?

@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 Sep 30, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Sep 30, 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 labels Oct 1, 2023
@ghost
Copy link

ghost commented Oct 1, 2023

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

Issue Details

With <WasmEnableThreads>true</WasmEnableThreads> generated dotnet.js has assert to only be used in browser environment. When imported (w/o actually creating the runtime), following error is thrown:

This build of dotnet is multi-threaded, it doesn't support shell environments like V8 or NodeJS.

This breaks unit testing in node when the dotnet APIs are mocked and the runtime is not initalized, but still has to be imported. After removing the assertion unit testing in node works fine.

Is it possible to remove the assertion or move it to the runtime creation function?

Author: Elringus
Assignees: -
Labels:

arch-wasm, area-System.Runtime.InteropServices.JavaScript, needs-area-label, os-browser

Milestone: -

@maraf maraf added this to the 9.0.0 milestone Oct 1, 2023
@pavelsavara
Copy link
Member

pavelsavara commented Oct 2, 2023

This is by design. What are the use cases which you need to address
Oh, I miss-read the description

@pavelsavara
Copy link
Member

yeah, MT on NodeJS is not even close to ready. Why are you importing the module when you mock it anyway ?

@elringus
Copy link
Author

elringus commented Oct 2, 2023

I mock individual bindings, but not the whole module. Here is an example: https://github.com/Elringus/DotNetJS/blob/feat/revamp/Samples/App/test/computer.test.tsx (backend is the dotnet module).

@vcsjones vcsjones removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Oct 3, 2023
@pavelsavara pavelsavara changed the title [browser] Oveprotective MT assert breaking fair usage in unit tests [browser][MT] support for NodeJS Nov 7, 2023
@pavelsavara pavelsavara modified the milestones: 9.0.0, Future Nov 9, 2023
@pavelsavara
Copy link
Member

This needs emscripten bump to 3.1.52

@lewing
Copy link
Member

lewing commented Mar 28, 2024

cc @radekdoulik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-wasm WebAssembly architecture area-VM-threading-mono os-browser Browser variant of arch-wasm
Projects
None yet
Development

No branches or pull requests

5 participants