Stop Serializing Return Values for InvokeVoidAsync
#35807
Conversation
…et/aspnetcore into taparik/invokeVoidReturnVal
|
API Proposal: #35929 |
|
/ping @dotnet/aspnet-blazor-eng for review 😄 |
src/JSInterop/Microsoft.JSInterop/src/Infrastructure/IJSVoidResult.cs
Outdated
Show resolved
Hide resolved
ghost
added this to the
|
Based on the label applied it looks like we’re going to have to wait another year to get this fix? |
|
Hi @StevenRasmussen. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context. |
It'll be going into .NET 6 RC2. |
|
/backport to release/6.0 |
|
Started backporting to release/6.0: https://github.com/dotnet/aspnetcore/actions/runs/1184532941 |
|
@TanayParikh backporting to release/6.0 failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Stop Serializing `InvokeVoidAsync` Return Value
Applying: Fix E2E Test
.git/rebase-apply/patch:56: trailing whitespace.
warning: 1 line adds whitespace errors.
warning: Cannot merge binary files: src/Components/Web.JS/dist/Release/blazor.webview.js (HEAD vs. Fix E2E Test)
warning: Cannot merge binary files: src/Components/Web.JS/dist/Release/blazor.server.js (HEAD vs. Fix E2E Test)
Using index info to reconstruct a base tree...
M src/Components/Web.JS/dist/Release/blazor.server.js
M src/Components/Web.JS/dist/Release/blazor.webview.js
Falling back to patching base and 3-way merge...
Auto-merging src/Components/Web.JS/dist/Release/blazor.webview.js
CONFLICT (content): Merge conflict in src/Components/Web.JS/dist/Release/blazor.webview.js
Auto-merging src/Components/Web.JS/dist/Release/blazor.server.js
CONFLICT (content): Merge conflict in src/Components/Web.JS/dist/Release/blazor.server.js
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 Fix E2E Test
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
| @@ -28,7 +28,7 @@ public static async ValueTask InvokeVoidAsync(this IJSObjectReference jsObjectRe | |||
| throw new ArgumentNullException(nameof(jsObjectReference)); | |||
| } | |||
|
|
|||
| await jsObjectReference.InvokeAsync<object>(identifier, args); | |||
| await jsObjectReference.InvokeAsync<IJSVoidResult>(identifier, args); | |||
There was a problem hiding this comment.
If it's a void type result why do we need to keep it? also what's the difference between InvokeVoidAsync vs InvokeAsync ??
There was a problem hiding this comment.
If it's a void type result why do we need to keep it?
We still need to have JS return so that we know when to stop awaiting. We could build a separate pipeline which doesn't return any value but that'd add unnecessary complexity / increase long term maintenance.
also what's the difference between InvokeVoidAsync vs InvokeAsync ??
InvokeVoidAsync is used for calls not requiring a return result, whereas InvokeAsync<T> expects a generic T result. Note, InvokeAsync<IJSVoidResult> should not be used in developer code (it's for framework internal use).
* Stop Serializing `InvokeVoidAsync` Return Value * Fix E2E Test * Add JSObjectReferenceTest * Remove `IJSVoidResult` from the public API * release .js files * Revert "Remove `IJSVoidResult` from the public API" This reverts commit e754872. * Add monocrash to gitignore * Fix Tests * Update src/JSInterop/Microsoft.JSInterop/src/PublicAPI.Unshipped.txt * PR Feedback @pranavkm
* Stop Serializing `InvokeVoidAsync` Return Value * Fix E2E Test * Add JSObjectReferenceTest * Remove `IJSVoidResult` from the public API * release .js files * Revert "Remove `IJSVoidResult` from the public API" This reverts commit e754872. * Add monocrash to gitignore * Fix Tests * Update src/JSInterop/Microsoft.JSInterop/src/PublicAPI.Unshipped.txt * PR Feedback @pranavkm
Fixes: #16632
Fixes: #35929