-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[browser] fix JSExport in lazy loaded assembly #117890
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
Conversation
eadc4d4 to
db3f51c
Compare
|
expected BTW output |
|
@maraf hints ? |
|
My first guess would be that the assembly was trimmed away. Binlog would help to validate it EDIT: But a) that should not happen if there is JSExport b) it shouldn't scream with both assemblies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes JSExport functionality in lazy loaded assemblies by addressing several critical issues: removing the DLL suffix when calling GetAssemblyExport, preventing stack restoration when the runtime is not running, and adding comprehensive test coverage.
Key Changes:
- Fixed GetAssemblyExport to use assembly name without ".dll" suffix
- Added runtime state checks before calling Module.stackRestore to prevent crashes
- Added new test infrastructure for lazy loading JSExport functionality
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| JSHostImplementation.cs | Removes ".dll" suffix from assembly name passed to GetAssemblyExport |
| memory.ts | Adds runtime state check before stack restoration in withStackAlloc |
| managed-exports.ts | Adds runtime state checks before all Module.stackRestore calls |
| invoke-cs.ts | Adds runtime state checks before Module.stackRestore in binding functions |
| GenerateWasmBootJson.cs | Fixes variable reference in error message generation |
| LazyLibrary/* | New test library with JSExport functionality for lazy loading tests |
| main.js | Adds test case for JSExport in lazy loaded assembly |
| WasmBasicTestApp.csproj | Adds LazyLibrary reference and lazy load configuration |
| LazyLoadingTest.cs | Adds DynamicDependency attribute for lazy loaded assembly |
Co-authored-by: Copilot <[email protected]>
Fixes #117780