Skip to content

Commit

Permalink
Stop Serializing Return Values for InvokeVoidAsync (dotnet#35807)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
TanayParikh authored Aug 31, 2021
1 parent 1268b20 commit 05303c7
Show file tree
Hide file tree
Showing 17 changed files with 97 additions and 29 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,4 @@ StyleCop.Cache
UpgradeLog.htm
.idea
*.svclog
mono_crash.*.json
4 changes: 3 additions & 1 deletion src/Components/Server/test/ProtectedBrowserStorageTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
using Microsoft.AspNetCore.DataProtection;
using Microsoft.AspNetCore.WebUtilities;
using Microsoft.JSInterop;
using Microsoft.JSInterop.Infrastructure;
using Moq;
using Xunit;

namespace Microsoft.AspNetCore.Components.Server.ProtectedBrowserStorage
Expand Down Expand Up @@ -265,7 +267,7 @@ public async Task ReusesCachedProtectorsByPurpose()
{
// Arrange
var jsRuntime = new TestJSRuntime();
jsRuntime.NextInvocationResult = new ValueTask<object>((object)null);
jsRuntime.NextInvocationResult = new ValueTask<IJSVoidResult>(Mock.Of<IJSVoidResult>());
var dataProtectionProvider = new TestDataProtectionProvider();
var protectedBrowserStorage = new TestProtectedBrowserStorage("testStore", jsRuntime, dataProtectionProvider);

Expand Down
2 changes: 1 addition & 1 deletion src/Components/Web.JS/dist/Release/blazor.server.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/Components/Web.JS/dist/Release/blazor.webview.js

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions src/Components/Web.JS/src/Boot.WebAssembly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ function invokeJSFromDotNet(callInfo: Pointer, arg0: any, arg1: any, arg2: any):
const streamReference = DotNet.createJSStreamReference(result);
const resultJson = JSON.stringify(streamReference);
return BINDING.js_string_to_mono_string(resultJson);
case DotNet.JSCallResultType.JSVoidResult:
return null;
default:
throw new Error(`Invalid JS call result type '${resultType}'.`);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ internal TResult InvokeUnmarshalled<T0, T1, T2, TResult>(string identifier, T0 a
switch (resultType)
{
case JSCallResultType.Default:
case JSCallResultType.JSVoidResult:
var result = InternalCalls.InvokeJS<T0, T1, T2, TResult>(out exception, ref callInfo, arg0, arg1, arg2);
return exception != null
? throw new JSException(exception)
Expand Down
2 changes: 2 additions & 0 deletions src/Components/test/E2ETest/Tests/InteropTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ public void CanInvokeDotNetMethods()
["asyncGenericInstanceMethod"] = @"""Updated value 1""",
["requestDotNetStreamReferenceAsync"] = @"""Success""",
["requestDotNetStreamWrapperReferenceAsync"] = @"""Success""",
["invokeVoidAsyncReturnsWithoutSerializing"] = "Success",
["invokeVoidAsyncReturnsWithoutSerializingInJSObjectReference"] = "Success",
};

var expectedSyncValues = new Dictionary<string, string>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,36 @@
ReturnValues["returnPrimitive"] = ((IJSInProcessRuntime)JSRuntime).Invoke<int>("returnPrimitive").ToString();
ReturnValues["returnArray"] = string.Join(",", ((IJSInProcessRuntime)JSRuntime).Invoke<Segment[]>("returnArray").Select(x => x.Source).ToArray());
}

try
{
// Triger a non-serializable WindowProxy (https://developer.mozilla.org/en-US/docs/Web/API/Window)
// InvokeVoidAsync shouldn't serialize return values, and thus there shouldn't be any exception thrown.
await JSRuntime.InvokeVoidAsync("eval", "window");
ReturnValues["invokeVoidAsyncReturnsWithoutSerializing"] = "Success";
}
catch (Exception ex)
{
ReturnValues["invokeVoidAsyncReturnsWithoutSerializing"] = $"Failure: {ex.Message}";
}

var jsObjectReference = await JSRuntime.InvokeAsync<IJSObjectReference>("returnJSObjectReference");
ReturnValues["jsObjectReference.identity"] = await jsObjectReference.InvokeAsync<string>("identity", "Invoked from JSObjectReference");
ReturnValues["jsObjectReference.nested.add"] = (await jsObjectReference.InvokeAsync<int>("nested.add", 2, 3)).ToString();
ReturnValues["addViaJSObjectReference"] = (await JSRuntime.InvokeAsync<int>("addViaJSObjectReference", jsObjectReference, 2, 3)).ToString();

try
{
// Fetch a non-serializable Window (https://developer.mozilla.org/en-US/docs/Web/API/Window)
// InvokeVoidAsync shouldn't serialize return values, and thus there shouldn't be any exception thrown.
await jsObjectReference.InvokeVoidAsync("getWindow");
ReturnValues["invokeVoidAsyncReturnsWithoutSerializingInJSObjectReference"] = "Success";
}
catch (Exception ex)
{
ReturnValues["invokeVoidAsyncReturnsWithoutSerializingInJSObjectReference"] = $"Failure: {ex.Message}";
}

try
{
await jsObjectReference.InvokeAsync<object>("nonFunction");
Expand Down Expand Up @@ -201,7 +225,7 @@

var dotNetStreamReferenceWrapper = DotNetStreamReferenceInterop.GetDotNetStreamWrapperReference();
ReturnValues["dotNetToJSReceiveDotNetStreamWrapperReferenceAsync"] = await JSRuntime.InvokeAsync<string>("jsInteropTests.receiveDotNetStreamWrapperReference", dotNetStreamReferenceWrapper);

Invocations = invocations;
DoneWithInterop = true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,9 @@ function returnJSObjectReference() {
identity: function (value) {
return value;
},
getWindow: function() {
return window;
},
nonFunction: 123,
nested: {
add: function (a, b) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ export module DotNet {
Default = 0,
JSObjectReference = 1,
JSStreamReference = 2,
JSVoidResult = 3,
}

/**
Expand Down Expand Up @@ -566,6 +567,8 @@ export module DotNet {
return createJSObjectReference(returnValue);
case JSCallResultType.JSStreamReference:
return createJSStreamReference(returnValue);
case JSCallResultType.JSVoidResult:
return null;
default:
throw new Error(`Invalid JS call result type '${resultType}'.`);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
using System.ComponentModel;

namespace Microsoft.JSInterop.Infrastructure
{
/// <summary>
/// Represents a void result from a JavaScript call.
/// This property is public to support cross-assembly accessibility for WebAssembly and should not be used by user code.
/// </summary>
[EditorBrowsable(EditorBrowsableState.Never)]
public interface IJSVoidResult
{
}
}
5 changes: 5 additions & 0 deletions src/JSInterop/Microsoft.JSInterop/src/JSCallResultType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,10 @@ public enum JSCallResultType : int
/// Indicates that the returned value is to be treated as a JS data reference.
/// </summary>
JSStreamReference = 2,

/// <summary>
/// Indicates a void result type.
/// </summary>
JSVoidResult = 3,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Diagnostics.CodeAnalysis;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.JSInterop.Infrastructure;
using static Microsoft.AspNetCore.Internal.LinkerFlags;

namespace Microsoft.JSInterop
Expand All @@ -28,7 +29,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);
}

/// <summary>
Expand Down Expand Up @@ -93,7 +94,7 @@ public static async ValueTask InvokeVoidAsync(this IJSObjectReference jsObjectRe
throw new ArgumentNullException(nameof(jsObjectReference));
}

await jsObjectReference.InvokeAsync<object>(identifier, cancellationToken, args);
await jsObjectReference.InvokeAsync<IJSVoidResult>(identifier, cancellationToken, args);
}

/// <summary>
Expand Down Expand Up @@ -135,7 +136,7 @@ public static async ValueTask InvokeVoidAsync(this IJSObjectReference jsObjectRe
using var cancellationTokenSource = timeout == Timeout.InfiniteTimeSpan ? null : new CancellationTokenSource(timeout);
var cancellationToken = cancellationTokenSource?.Token ?? CancellationToken.None;

await jsObjectReference.InvokeAsync<object>(identifier, cancellationToken, args);
await jsObjectReference.InvokeAsync<IJSVoidResult>(identifier, cancellationToken, args);
}
}
}
7 changes: 4 additions & 3 deletions src/JSInterop/Microsoft.JSInterop/src/JSRuntimeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Diagnostics.CodeAnalysis;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.JSInterop.Infrastructure;
using static Microsoft.AspNetCore.Internal.LinkerFlags;

namespace Microsoft.JSInterop
Expand All @@ -28,7 +29,7 @@ public static async ValueTask InvokeVoidAsync(this IJSRuntime jsRuntime, string
throw new ArgumentNullException(nameof(jsRuntime));
}

await jsRuntime.InvokeAsync<object>(identifier, args);
await jsRuntime.InvokeAsync<IJSVoidResult>(identifier, args);
}

/// <summary>
Expand Down Expand Up @@ -93,7 +94,7 @@ public static async ValueTask InvokeVoidAsync(this IJSRuntime jsRuntime, string
throw new ArgumentNullException(nameof(jsRuntime));
}

await jsRuntime.InvokeAsync<object>(identifier, cancellationToken, args);
await jsRuntime.InvokeAsync<IJSVoidResult>(identifier, cancellationToken, args);
}

/// <summary>
Expand Down Expand Up @@ -135,7 +136,7 @@ public static async ValueTask InvokeVoidAsync(this IJSRuntime jsRuntime, string
using var cancellationTokenSource = timeout == Timeout.InfiniteTimeSpan ? null : new CancellationTokenSource(timeout);
var cancellationToken = cancellationTokenSource?.Token ?? CancellationToken.None;

await jsRuntime.InvokeAsync<object>(identifier, cancellationToken, args);
await jsRuntime.InvokeAsync<IJSVoidResult>(identifier, cancellationToken, args);
}
}
}
2 changes: 2 additions & 0 deletions src/JSInterop/Microsoft.JSInterop/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ Microsoft.JSInterop.DotNetStreamReference.DotNetStreamReference(System.IO.Stream
Microsoft.JSInterop.DotNetStreamReference.LeaveOpen.get -> bool
Microsoft.JSInterop.DotNetStreamReference.Stream.get -> System.IO.Stream!
Microsoft.JSInterop.Infrastructure.DotNetInvocationResult.ResultJson.get -> string?
Microsoft.JSInterop.Infrastructure.IJSVoidResult
Microsoft.JSInterop.JSCallResultType.JSStreamReference = 2 -> Microsoft.JSInterop.JSCallResultType
Microsoft.JSInterop.JSCallResultType.JSVoidResult = 3 -> Microsoft.JSInterop.JSCallResultType
Microsoft.JSInterop.JSRuntime.Dispose() -> void
static Microsoft.JSInterop.Implementation.JSObjectReferenceJsonWorker.ReadJSObjectReferenceIdentifier(ref System.Text.Json.Utf8JsonReader reader) -> long
static Microsoft.JSInterop.Implementation.JSObjectReferenceJsonWorker.WriteJSObjectReference(System.Text.Json.Utf8JsonWriter! writer, Microsoft.JSInterop.Implementation.JSObjectReference! objectReference) -> void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.JSInterop.Infrastructure;
using Moq;
using Xunit;

Expand Down Expand Up @@ -65,7 +66,7 @@ public async Task InvokeVoidAsync_WithoutCancellationToken()
var method = "someMethod";
var args = new[] { "a", "b" };
var jsRuntime = new Mock<IJSRuntime>(MockBehavior.Strict);
jsRuntime.Setup(s => s.InvokeAsync<object>(method, args)).Returns(new ValueTask<object>(new object()));
jsRuntime.Setup(s => s.InvokeAsync<IJSVoidResult>(method, args)).Returns(new ValueTask<IJSVoidResult>(Mock.Of<IJSVoidResult>()));

// Act
await jsRuntime.Object.InvokeVoidAsync(method, args);
Expand All @@ -80,7 +81,7 @@ public async Task InvokeVoidAsync_WithCancellationToken()
var method = "someMethod";
var args = new[] { "a", "b" };
var jsRuntime = new Mock<IJSRuntime>(MockBehavior.Strict);
jsRuntime.Setup(s => s.InvokeAsync<object>(method, It.IsAny<CancellationToken>(), args)).Returns(new ValueTask<object>(new object()));
jsRuntime.Setup(s => s.InvokeAsync<IJSVoidResult>(method, It.IsAny<CancellationToken>(), args)).Returns(new ValueTask<IJSVoidResult>(Mock.Of<IJSVoidResult>()));

// Act
await jsRuntime.Object.InvokeVoidAsync(method, new CancellationToken(), args);
Expand Down Expand Up @@ -142,14 +143,14 @@ public async Task InvokeVoidAsync_WithTimeout()
var method = "someMethod";
var args = new[] { "a", "b" };
var jsRuntime = new Mock<IJSRuntime>(MockBehavior.Strict);
jsRuntime.Setup(s => s.InvokeAsync<object>(method, It.IsAny<CancellationToken>(), args))
jsRuntime.Setup(s => s.InvokeAsync<IJSVoidResult>(method, It.IsAny<CancellationToken>(), args))
.Callback<string, CancellationToken, object[]>((method, cts, args) =>
{
// There isn't a very good way to test when the cts will cancel. We'll just verify that
// it'll get cancelled eventually.
Assert.True(cts.CanBeCanceled);
})
.Returns(new ValueTask<object>(new object()));
.Returns(new ValueTask<IJSVoidResult>(Mock.Of<IJSVoidResult>()));

// Act
await jsRuntime.Object.InvokeVoidAsync(method, TimeSpan.FromMinutes(5), args);
Expand All @@ -164,13 +165,13 @@ public async Task InvokeVoidAsync_WithInfiniteTimeout()
var method = "someMethod";
var args = new[] { "a", "b" };
var jsRuntime = new Mock<IJSRuntime>(MockBehavior.Strict);
jsRuntime.Setup(s => s.InvokeAsync<object>(method, It.IsAny<CancellationToken>(), args))
jsRuntime.Setup(s => s.InvokeAsync<IJSVoidResult>(method, It.IsAny<CancellationToken>(), args))
.Callback<string, CancellationToken, object[]>((method, cts, args) =>
{
Assert.False(cts.CanBeCanceled);
Assert.True(cts == CancellationToken.None);
})
.Returns(new ValueTask<object>(new object()));
.Returns(new ValueTask<IJSVoidResult>(Mock.Of<IJSVoidResult>()));

// Act
await jsRuntime.Object.InvokeVoidAsync(method, Timeout.InfiniteTimeSpan, args);
Expand Down
31 changes: 18 additions & 13 deletions src/Shared/JSInterop/JSCallResultTypeHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Reflection;
using Microsoft.JSInterop.Infrastructure;

namespace Microsoft.JSInterop
{
Expand All @@ -12,21 +13,25 @@ internal static class JSCallResultTypeHelper

public static JSCallResultType FromGeneric<TResult>()
{
if (typeof(TResult).Assembly == _currentAssembly
&& (typeof(TResult) == typeof(IJSObjectReference)
|| typeof(TResult) == typeof(IJSInProcessObjectReference)
|| typeof(TResult) == typeof(IJSUnmarshalledObjectReference)))
if (typeof(TResult).Assembly == _currentAssembly)
{
return JSCallResultType.JSObjectReference;
}
else if (typeof(TResult).Assembly == _currentAssembly && typeof(TResult) == typeof(IJSStreamReference))
{
return JSCallResultType.JSStreamReference;
}
else
{
return JSCallResultType.Default;
if (typeof(TResult) == typeof(IJSObjectReference) ||
typeof(TResult) == typeof(IJSInProcessObjectReference) ||
typeof(TResult) == typeof(IJSUnmarshalledObjectReference))
{
return JSCallResultType.JSObjectReference;
}
else if (typeof(TResult) == typeof(IJSStreamReference))
{
return JSCallResultType.JSStreamReference;
}
else if (typeof(TResult) == typeof(IJSVoidResult))
{
return JSCallResultType.JSVoidResult;
}
}

return JSCallResultType.Default;
}
}
}

0 comments on commit 05303c7

Please sign in to comment.