Support calling void and "no return" functions in JS #27094
Conversation
This change just makes adding new tests easier
…d-return # Conflicts: # src/Controls/tests/DeviceTests/Elements/HybridWebView/HybridWebViewTests.cs
There was a problem hiding this comment.
Copilot reviewed 4 out of 20 changed files in this pull request and generated 2 comments.
Files not reviewed (16)
- src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt: Language not supported
- src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt: Language not supported
- src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt: Language not supported
- src/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txt: Language not supported
- src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt: Language not supported
- src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt: Language not supported
- src/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txt: Language not supported
- src/Controls/tests/DeviceTests/Resources/Raw/HybridTestRoot/index.html: Language not supported
- src/Core/src/PublicAPI/net-android/PublicAPI.Unshipped.txt: Language not supported
- src/Core/src/PublicAPI/net-ios/PublicAPI.Unshipped.txt: Language not supported
- src/Core/src/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt: Language not supported
- src/Core/src/PublicAPI/net-tizen/PublicAPI.Unshipped.txt: Language not supported
- src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt: Language not supported
- src/Core/src/PublicAPI/net/PublicAPI.Unshipped.txt: Language not supported
- src/Core/src/PublicAPI/netstandard/PublicAPI.Unshipped.txt: Language not supported
- src/Core/src/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt: Language not supported
Comments suppressed due to low confidence (1)
src/Controls/src/Core/HybridWebView/HybridWebView.cs:84
- Ensure that the new method InvokeJavaScriptAsync in HybridWebView is covered by tests.
public async Task InvokeJavaScriptAsync(string methodName, object?[]? paramValues = null, JsonTypeInfo?[]? paramJsonTypeInfos = null)
| { | ||
| invokeJavaScriptRequest.SetResult(null); | ||
| } | ||
| // if there is no result or if the result was null/undefined, then reat it as null |
There was a problem hiding this comment.
The word 'reat' should be 'treat'.
| // if there is no result or if the result was null/undefined, then reat it as null | |
| // if there is no result or if the result was null/undefined, then treat it as null |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
| @@ -3,11 +3,11 @@ | |||
|
|
|||
| namespace Microsoft.Maui | |||
| { | |||
| public class HybridWebViewInvokeJavaScriptRequest(string methodName, JsonTypeInfo returnTypeJsonTypeInfo, object?[]? paramValues, JsonTypeInfo?[]? paramJsonTypeInfos) | |||
| public class HybridWebViewInvokeJavaScriptRequest(string methodName, JsonTypeInfo? returnTypeJsonTypeInfo, object?[]? paramValues, JsonTypeInfo?[]? paramJsonTypeInfos) | |||
There was a problem hiding this comment.
The change in the constructor signature to accept nullable JsonTypeInfo could be a breaking change. Ensure this is documented appropriately.
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
| public async Task InvokeJavaScriptAsync( | ||
| string methodName, | ||
| object?[]? paramValues = null, | ||
| JsonTypeInfo?[]? paramJsonTypeInfos = null) |
There was a problem hiding this comment.
This is the new API that makes using it more intuitive and does not have unused parameters.
If we do not want to break ABI with .NET 9.0 GA, we can hide this and maybe the community toolkit an provide some extension methods. The existing method will work, but we have to provide an unused generic argument.
| public class HybridWebViewInvokeJavaScriptRequest(string methodName, JsonTypeInfo returnTypeJsonTypeInfo, object?[]? paramValues, JsonTypeInfo?[]? paramJsonTypeInfos) | ||
| public class HybridWebViewInvokeJavaScriptRequest(string methodName, JsonTypeInfo? returnTypeJsonTypeInfo, object?[]? paramValues, JsonTypeInfo?[]? paramJsonTypeInfos) |
There was a problem hiding this comment.
This is technically not breaking, so we can do this for .NET 9. Since this change is attribute-only, we could just secretly allow nulls and make this change later. But, since it is not breaking and the number of people creating their own handler for the hybrid web view is probably 0, we can do this and just make everything easier in the future.
| if (stringResult is null) | ||
| // if we are not looking for a return object, then return null | ||
| if (invokeJavaScriptRequest.ReturnTypeJsonTypeInfo is null) | ||
| { | ||
| invokeJavaScriptRequest.SetResult(null); | ||
| } | ||
| // if there is no result or if the result was null/undefined, then reat it as null | ||
| else if (stringResult is null || stringResult == "null" || stringResult == "undefined") | ||
| { | ||
| invokeJavaScriptRequest.SetResult(null); | ||
| } | ||
| // if we are expecting a result, then deserialize what we have | ||
| else |
There was a problem hiding this comment.
This is the real fix, if no type info is provided and/or the return type is null/undefined (from JS) then we just return null in .NET.
Description of Change
Currently, the HybridWebView assumes that all functions return a value and immediately tries to deserialize the result.
This PR changes that to first check to see if the value is "null" / "undefined" and then skips the deserialization since those values represent null in .NET.
There is also a new API in the HybridWebView control that provides a way to avoid having to provide a type and type info that will just be ignored.
After this PR, there will be 3 ways to invoke a void functino:
hybridWebView.InvokeJavaScriptAsync<TReturn>(name, TReturn info, ...)This is the current way, we just no longer crash if the return value is null.
hybridWebView.InvokeJavaScriptAsync<TReturn>(name, null, ...)This is a better way, we allow passing null as the type info, but we still need the generic argument - which can be anything, object, string, etc
hybridWebView.InvokeJavaScriptAsync(name, ...)This is the new API where it is not generic and does not have a return type info parameter
Issues Fixed
Fixes #26766