[browser] JS interop fix marshalling of completed task of long #123366
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dotnet-policy-service
bot
added
the
community-contribution
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the marshalling of completed Task<long> to JavaScript through the JSExport/JSImport API. When a Task<long> was already completed, a shortcut path in the marshalling code would call a generic ToJS(object) method instead of the proper type-specific marshaler delegate. Since the ToJS(object) method explicitly throws a NotSupportedException for long types (because longs can be marshaled as either JS number or bigint), this caused runtime errors.
Changes:
- Fixed the marshaller to use the proper delegate (
marshaler(ref this, result)) instead of the generic object marshaller for completed tasks - Added comprehensive test coverage for both incomplete and completed
Task<long>scenarios - Added supporting JSImport and JSExport methods to facilitate the new tests
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| JSMarshalerArgument.Task.cs | Fixed line 360 to use the marshaler delegate instead of ToJS(object) for completed task results |
| JavaScriptTestHelper.cs | Added JSImport invoke1_TaskOfLong and JSExport AwaitTaskOfInt64 for testing Task marshalling with BigInt |
| JSExportTest.cs | Added two test methods: JsExportTaskOfLong (incomplete task) and JsExportCompletedTaskOfLong (completed task - the bug scenario) |
...teropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/JSExportTest.cs
Outdated
Show resolved
Hide resolved
pavelsavara
added
arch-wasm
Contributor
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
pavelsavara
approved these changes
Jan 20, 2026
pavelsavara
changed the title
This was referenced Jan 21, 2026
This was referenced Jan 21, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A 'shortcut' is taken in the marshalling process to JS when a
Task<T>is already completed to just marshall the value directy. I found that the task result marshaller would differ from the one used in the path where the task is not yet completed. For most types this is fine, however forTask<long>there are options (number vs bigint) and the marshaller used in the shortcut path (the ToJS(object) function) couldnt decide which js type should be used. If this path was hit the end user would get an error likeToJSNotSupported, Int64.The changes include two new tests that marshall
Task<long>, one completing after crossing the interop boundary and one before crossing. The latter test would fail before the fix. The fix is using the marshaller delegate (provided by codegen) which indeed is able to marshall as the type configured by the user.