-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Add InvokeNew, GetValue & SetValue to JS interop API #61246
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
Changes from 23 commits
df3822f
bdea46d
8e51e9c
f227e09
9717446
1c525a4
e653a00
4009275
5e273e3
0443be5
951de51
7056120
200d736
e7294ce
42617e4
21caedd
c7f49d6
93d3006
2508129
45260dd
5270dd8
c6934f6
c13223b
2f84ea0
28c8d39
1d8495c
b79f224
bd0ea2b
802f96a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,3 @@ | ||
#nullable enable | ||
Microsoft.AspNetCore.Components.Web.Internal.IInternalWebJSInProcessRuntime.InvokeJS(Microsoft.JSInterop.Infrastructure.JSInvocationInfo! invocationInfo) -> string! | ||
virtual Microsoft.AspNetCore.Components.Routing.NavLink.ShouldMatch(string! uriAbsolute) -> bool |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ | |
using Microsoft.Extensions.DependencyInjection; | ||
using Microsoft.Extensions.Logging; | ||
using Microsoft.JSInterop; | ||
using Microsoft.JSInterop.Infrastructure; | ||
using static Microsoft.AspNetCore.Internal.LinkerFlags; | ||
|
||
namespace Microsoft.AspNetCore.Components.RenderTree; | ||
|
@@ -130,7 +131,16 @@ private void AttachWebRendererInterop(IJSRuntime jsRuntime, JsonSerializerOption | |
newJsonOptions.TypeInfoResolverChain.Add(WebRendererSerializerContext.Default); | ||
newJsonOptions.TypeInfoResolverChain.Add(JsonConverterFactoryTypeInfoResolver<DotNetObjectReference<WebRendererInteropMethods>>.Instance); | ||
var argsJson = JsonSerializer.Serialize(args, newJsonOptions); | ||
inProcessRuntime.InvokeJS(JSMethodIdentifier, argsJson, JSCallResultType.JSVoidResult, 0); | ||
var invocationInfo = new JSInvocationInfo | ||
{ | ||
AsyncHandle = 0, | ||
TargetInstanceId = 0, | ||
Identifier = JSMethodIdentifier, | ||
CallType = JSCallType.FunctionCall, | ||
ResultType = JSCallResultType.JSVoidResult, | ||
ArgsJson = argsJson, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Including the arguments here is going to be a problem. As far as I can tell, JSInvocationInfo gets json serialized, so this means ArgsJson gets "doubly" serialized which is going to result in a bunch of additional unescaped characters. I think we should keep the arguments outside of this struct for that reason. I believe the other implementation does it this way, doesn't it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Regarding Meanwhile the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will check if passing |
||
}; | ||
inProcessRuntime.InvokeJS(invocationInfo); | ||
} | ||
else | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,12 +18,7 @@ internal static partial class InternalCalls | |
public static extern TRes InvokeJS<T0, T1, T2, TRes>(out string exception, ref JSCallInfo callInfo, [AllowNull] T0 arg0, [AllowNull] T1 arg1, [AllowNull] T2 arg2); | ||
|
||
[JSImport("Blazor._internal.invokeJSJson", "blazor-internal")] | ||
public static partial string InvokeJSJson( | ||
string identifier, | ||
[JSMarshalAs<JSType.Number>] long targetInstanceId, | ||
int resultType, | ||
string argsJson, | ||
[JSMarshalAs<JSType.Number>] long asyncHandle); | ||
public static partial string InvokeJSJson(string invocationInfoString); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We shouldn't stringify things here, we need to keep this API the same, including the additional info and passing down the individual members. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the point of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The point is that whenever we need to add things to the public API we don't need to keep adding overloads to JSRuntime and other public classes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, that makes sense. However, I thought it was also to not having to rewrite the three different transport channels whenever we add some parameter to the interop (which based on my recent experience is the part that takes the most work to do). |
||
|
||
[JSImport("Blazor._internal.endInvokeDotNetFromJS", "blazor-internal")] | ||
public static partial void EndInvokeDotNetFromJS( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,3 @@ | ||
#nullable enable | ||
override Microsoft.JSInterop.WebAssembly.WebAssemblyJSRuntime.BeginInvokeJS(Microsoft.JSInterop.Infrastructure.JSInvocationInfo! invocationInfo) -> void | ||
override Microsoft.JSInterop.WebAssembly.WebAssemblyJSRuntime.InvokeJS(Microsoft.JSInterop.Infrastructure.JSInvocationInfo! invocationInfo) -> string? |
Uh oh!
There was an error while loading. Please reload this page.
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.
We shouldn't serialize things to a big string here.
We should keep
_clientProxy.SendAsync
(and the equivalent webassembly method) the same, unpacking the JSInvocationInfo and passing in the individual members.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.
So you want to keep passing individual arguments across the interop bridge? I.e. only add the new
JSCallType
value to the already long list of arguments?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.
Yes, those are not part of the public API, so it's fine, we can change those whenever we want. We can't avoid serializing the arguments as a string, but we don't want to wrap that string into something else and serialize that too.