-
-
Notifications
You must be signed in to change notification settings - Fork 104
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
#361: Allow untyped JSInterop.Setup methods, that matches with any type #451
Conversation
Thanks @jgoday. I hope I can get some time tonight or tomorrow between all the moving to look at this. |
So sorry @jgoday. We're very busy packing up the house so it looks like I wont have time to look at this before some time the weekend. |
There's no hurry :) |
Finally had some time to look through the code. There is a scenario that the current solution does not support: [Fact(DisplayName = "Untyped supports setting result after invocation")]
public async Task Test062()
{
var sut = CreateSut(JSRuntimeMode.Strict);
var identifier = "func";
var jsRuntime = sut.JSRuntime;
var handler = sut.Setup(i => i.Identifier == identifier);
var i1 = jsRuntime.InvokeAsync<bool>(identifier);
handler.SetResult(_ => false);
(await i1).ShouldBe(false);
} Here, the
|
Added last example to tests (Test062), It should work now. UntypedJSRuntimeInvocationHandler :
Is this a valid solution ? |
Hmm I am starting to get cold feet on this one tbh. What troubles me is that it should be possible to add/register one or more responses, each matching different invocations and return types, right? Or maybe it should just be possible to provide a single Does this make sense? |
Hi @egil, Notice that, probably, I am not fully aware of the scope of the problem. Here are the possible steps:
|
Let me elaborate a bit more on my general concerns (was on a phone last time, so was keystroke limited). Supposed we set up a untyped handler for every invocation whose identifier starts with var handler = Setup(i => i.Identifier,StartsWith("myJsLib")); Its not a completely unreasonable scenario, since you might want to handle all calls to a js lib with the following functions:
How should the API for specifying the return type (or exception) look? handler.SetResult(inv =>
{
if(inv.Identifier.EndsWith("getText")
return "foo";
if(inv.Identifier.EndsWith("getAnotherText")
return "bar";
if(inv.Identifier.EndsWith("getNum")
return 42;
}); This wont work unless If we instead just allow the handler to receive multiple calls: handler.SetResult<string>(inv =>
{
if(inv.Identifier.EndsWith("getText")
return "foo";
if(inv.Identifier.EndsWith("getAnotherText")
return "bar";
return ???;
})
handler.SetResult<int>(inv => 42); This looks a bit weird to me. Also, what if I want e.g. What I think the key is, is that there is a duality between Setup and SetXXXX calls, and they cannot have different number of branches, e.g. one setup call that matches 10 JS functions, but only 5 SetXXXX calls specifying results. If there is, its going to lead to too much "magic" for the end-user. The original use case was to make it easy for 3rd party component vendors to do a one liner that would set up handling for all their js calls, so that users of the 3rd party component lib could write a test of a component that uses a component from the 3rd party lib without having to worry about complex jsinterop setup logic. However, with the above in mind, its pretty clear that a one liner would be pretty hard to achieve in many cases. What do you think? |
Thank you @egil :) Sorry for the late response, I will try to look and work on this starting the week. |
I look forward to seeing your solution. Ps. Going camping next week, so might not be able to review your code before after that. |
hey @jgoday, im going to close this PR. If you want to continue with this, just reopen it/resubmit it. |
No worries. I understand! |
Pull request description
Fixes #361.
It defines new non-generic Setup methods on BunitJSInteropSetupExtensions, that returns an UntypedJSRuntimeInvocationHandler, who is responsible to create typed JSRuntimeInvocationHandler (JSRuntimeInvocationHandlerFactory) from factory functions (SetResult/SetException/SetCanceled).
Factory functions are defined as Func<JSRuntimeInvocation, T>.
A review of UntypedJSruntimeInvocationHandler/JSRuntimeInvocationHandlerFactory correct names would be necessary.
UntypedJSruntimeInvocationHandler is created from the non-generic setup extension method,
and allow us to return a typed handler from a factory function.
JSRuntimeInvocationHandlerFactory is based on JSRuntimeInvocationHandler,
is created from UntypedJSRuntimeInvocationHandler.SetResult(FactoryFunction) or UntypedJSRuntimeInvocationHandler.SetException(FactoryFunction),
his only responsibility is handling a js invocation and set the specific result or exception on the base class.
PR meta checklist
main
branch for codeor targeted at
stable
branch for documentation that is live on bunit.dev.Code PR specific checklist