Add support for JSObjectReference/IJSObjectReference

[egil]

The new way of loading JavaScript modules into a Blazor app and then receiving a JSObjectReference after the call should be eksplicit supported by the mock js runtime in bUnit, such that the resulting JSObjectReference just works.

More info here: https://devblogs.microsoft.com/aspnet/asp-net-core-updates-in-net-5-release-candidate-1/#blazor-javascript-isolation-and-object-references

Related: dotnet/aspnetcore#26981

[KristofferStrube]

Hey Egil,
As I see it, the MockJSObjectReference that would implement IJSObjectReference would look very similar to the MockJSRuntime which uses the MockJSRuntimeInvokeHandler, but it seems like a lot of duplicated code if we were to make a MockJSObjectReferenceInvokeHandler as well so maybe the MockJSObjectReference should just get parsed the same InvokeHandler, but then the name should maybe change to something more general like MockJSInvokeHandler. This would of cause require some refactoring since we have the JSRuntime specific method ToJSRuntime() in that class as well. What do you think or have you already thought about the design?

A problem that I'm facing (that is not directly related) is that I can't reference IJSObjectReference in the MockJSRuntimeInvokeHandler when I've tried sketching a solution. But I can inherit from JSObjectReference. I guess that it might not have been added to netstandard 2.1 yet, but I don't know. I suppose some of the people from the dotnet foundation who are working on Blazor would argue that you indeed should extend JSObjectReference not IJSObjectReference, but that leaves us with a constructor that can't take e.g. a MockJSInvokeHandler. Any thoughts on this?

[egil]

Hey Kristoffer

My initial design thoughts on this are as follows (assuming users have called var mockJs = Services.AddMockJSRuntime()):

1. Two new "setup" methods should be added to MockJSRuntimeInvokeHandler:

   • SetupModule(string moduleUrl, JSRuntimeMockMode? mode = null)
   • Setup<IJSObjectReference>(string identifier, string moduleUrl, JSRuntimeMockMode? mode = null)

2. Both new setup methods does the same thing:

   • return a MockJSObjectReferenceInvokeHandler
   • add a planned invocation to the MockJSRuntimeInvokeHandler planned invocations list, that will return the MockJSObjectReferenceInvokeHandler's mock IJSRuntime to the caller.
   • set the JSRuntimeMockMode on the MockJSObjectReferenceInvokeHandler if one is provided, otherwise use the one from the MockJSRuntimeInvokeHandler. This allows users to have different modes for different modules, something that could be useful for 3rd party libraries that want to provide a fake JSInterop experience.

3. These new setup methods are only available when targeting .NET 5.

4. MockJSObjectReferenceInvokeHandler could inherits from MockJSRuntimeInvokeHandler or a common base class, with the addition of a property called Module, that is set to the name of the module provided in the setup methods.

5. MockJSObjectReferenceInvokeHandler should add any invocations it handles to the "root" mockJs's Invocations list, the MockJSRuntimeInvokeHandler that created it, as well as its own, such that users can query both invocations list to verify invocations.

6. There should be a special JSRuntimePlannedInvocationBase type for planned invocations for modules, that also contains the Module property for the invocation, to avoid conflicts with modules that have the same functions.

What do you think?

Btw. in addition to this, I plan to change how bUnit's JSInterop/mock JSRuntime is surfaced to the user. I want to add a property public MockJSRuntimeInvokeHandler JSInterop { get; } to the TestContext type. This will also mean that JSInterop will be registered in the services collection by default in strict mode. This is because .NET5 will bring 1st party components with Blazor that uses JSInterop, and bUnit should support those out of the box. But that discussion is for a different issue (#237) and doesn't really influence this.

[KristofferStrube]

Hey again,

Nice to see that you've already had a lot of ideas. 👍

1. Looks good. Like the idea of using the JSRuntimeMockMode again. Apropos, should setting the MockJSRuntimeInvokeHandler's JSRuntimeMockMode to Loose make all InvokeAsync<IJSObjectReference>("import", someUrl) calls return a MockJSObjectReference by default or should you have to call SetupModule explicitly to use modules in a test? Some might not be interested in having to call SetupModule for all their modules if they have a lot.

2. Sounds good.

3. Makes sense.

4. It should probably not inherit from MockJSRuntimeInvokeHandler since that would mean that a module could load another module, which I don't think is possible. A common base class like MockJSInvokeHandler sounds like the way to go.

5. You won't be able to have multiple layers of MockJSRuntimeInvokeHandlers so just two Invocation lists or is "root" something else than a MockJSRuntimeInvokeHandler? But sounds like a good idea to be able to check it in both places.

6. Good idea. Keeping the modules separate matches the actual behaviour well.

In total it seems like a good plan.

If there were to be added an on-by-default result for an invoked import then that might be the easiest to add by simply checking in the MockJSRuntime.TryHandlePlannedInvocation method: If JSRuntimeMockMode is loose, the TValue equals IJSObjectReference, and the identifier equals "import" and then returning a new MockJSObjectReference with a new loose MockJSObjectReferenceInvokeHandler parsed. This would though open up the question of what the Module property should be set to. "default" or does it even matter when it is loose anyway? There probably exists a more elegant solution than this.

Sounds interesting to add JSRuntime to the TestContext. Will that mean that the ToJSRuntime() method will be removed?

[egil]

Thanks for the input Kristoffer.

Regarding 1: As part of the "always on" issue #237, planned invocations for InvokeAsync<IJSObjectReference>("import", someUrl) will be added to handle that invocation type out of the box.

Regarding 4: Agreed, some base type/interface is probably needed. But are we sure that IJSObjectReference modules are not allowed to load other IJSObjectReference modules?

Regarding 5: I see two layers, the "root" jsruntime mock that can return zero or more child IJSObjectReference mock jsruntimes. The idea of having the childs add their recorded invocations to the root invocationslist, is the same as with planned invocations, that also share their received invocations with the root invocation list.

If there were to be added an on-by-default result for an invoked import then that might be the easiest to add by simply checking in the MockJSRuntime.TryHandlePlannedInvocation method: If JSRuntimeMockMode is loose, the TValue equals IJSObjectReference, and the identifier equals "import" and then returning a new MockJSObjectReference with a new loose MockJSObjectReferenceInvokeHandler parsed. This would though open up the question of what the Module property should be set to. "default" or does it even matter when it is loose anyway? There probably exists a more elegant solution than this.

With the always on, we will just register planned handlers for that case. I do not think an special override is needed.

Sounds interesting to add JSRuntime to the TestContext. Will that mean that the ToJSRuntime() method will be removed?

Probably some tweaks to the public API, but lets keep those discussions in that issue.

[KristofferStrube]

Hey again,
I've made a project that shows how modules can be imported in Blazor: github.com/KristofferStrube/MinimalIJSObjectReferenceExample
You can't call top-level functions like import from a module, but you can let a javascript function that is exported call import and return that. That's pretty much the take away from the project.

[egil]

@KristofferStrube if you want to take a crack at this issue, feel free to go ahead. Do check out the contributor guidelines first thought.

[KristofferStrube]

I will try putting something together. I haven't done any pull requests before, so might need some help through the process, but I'm up for the challenge.
I'm still facing the problem that I can't reference IJSObjectReference, but that might be fixed when we set it so that it can only be used when using .NET5.0.

[egil]

That's fair. This is a pretty complex one. If you want to get your feet wet first then there are also other issues with the good first issue label that you can look at.

[egil]

Hey @KristofferStrube, you can check out the PR #247 to see what I am planned to change in bUnit s JSInterop.

[egil]

Hi @KristofferStrube, im going to start looking at this now. Let me know if you want to contribute.

[KristofferStrube]

Hey @egil, sounds great. I don't think I have the time currently to contribute a lot, but I would love to ping-pong if you have any decisions that you need sparring for or other minor tasks.

[egil]

Here is my design/progress so far:

1. Calling JSInterop.Setup<IJSObjectReference>("import", "FOO.js") or JSInterop.Setup<JSObjectReference>("import", "FOO.js") will throw an exception telling the user to use the SetupModule method instead.
2. There are three SetupModule methods:
   1. SetupModule(string moduleName) - this will return a handler that only matches a module with the specified name.
   2. SetupModule(InvocationMatcher invocationMatcher) - this will return a handler that matches any module that passes the InvocationMatcher predicate.
   3. SetupModule() - this will return a handler that matches any InvokeAsync<IJSObjectReference>("import", xxxx) module invocation.
3. The SetupModule methods all return a special handler, JSObjectReferenceInvocationHandler, that can be configured to handle calls using the normal Setup and SetupVoid methods.
4. The JSObjectReferenceInvocationHandler has its own Mode property, which it by default inherits its value from, from the root JSInterop. If the JSObjectReferenceInvocationHandler Mode property is explicitly set by the user, it will stop inheriting from the root. This allows individual modules to e.g. be in strict mode, while the root JSInterop is in loose mode.
5. When a module invocation is received, its identifier is set to <moduleName>#<invocation identifier>, e.g. foo.js#helloWorld.
6. Invocations to received by module handlers are stored in their own Invocations dictinoary, as well as that of the root JSInterop.

[KristofferStrube]

Looks good @egil,

1. Good idea to not make it possible to call import manually.
2. Seems to cover use cases. Will we be able to call invoke("import", xxxx) if the JSInterop is in loose mode without setting up a module explicitly?
3. Great, good to point out that JSObjectReferenceInvocationHandler should not have a SetupModule method.
4. Sounds great that it inherits the mode. I was thinking if it should inherit the _handlers from its root JSInterop as well, but it's probably pretty niche that a user wants to have the module strict but doesn't want to define its handler specifically for each module. So I think it's good to only inherit the mode.
5. Fine to use # as a separator as it cannot be part of the name of a method. The same probably goes for js-file names, but I haven't checked.
6. Good 👍

Extra thing:
What about making it so JSObjectReferenceInvocationHandler returns a JSObjectReferenceInvocationHandler if a method of signature Setup<IJSObjectReference>(XXX, args?) is called? This way we support that exported functions can return an IJSObjectReference but this might make the solution more complex. Any thoughts on this?

[egil]

2. Seems to cover use cases. Will we be able to call invoke("import", xxxx) if the JSInterop is in loose mode without setting up a module explicitly?

Yes. There is a default "loose mode" JSObjectReferenceInvocationHandler registered that will be used in this case, and it will simply return the default value for calls it handles, like regular loose mode calls.

Extra thing:
What about making it so JSObjectReferenceInvocationHandler returns a JSObjectReferenceInvocationHandler if a method of signature Setup<IJSObjectReference>(XXX, args?) is called? This way we support that exported functions can return an IJSObjectReference but this might make the solution more complex. Any thoughts on this?

Are you even allowed to call module.InvokeAsync<IJSObjectReference>(...)? As in importing a module from a module from Blazor code.

[KristofferStrube]

Yep. I have an example here where I do it: here

[egil]

Hmm, so does...

var module1 = await jsRuntime.InvokeAsync<IJSObjectReference>("import", "./js/script1.js");
var module2 = await module1.InvokeAsync<IJSObjectReference>("importScript", "./script2.js");
await module2.InvokeVoidAsync("hello2", "Importing indirectly works");

1. actually call the hello2 method in script2.js?
2. and if yes, why are using the importScript instead of import?
3. is that actually calling a method in script1.js named importScript?

[KristofferStrube]

script1.js contains:

export function importScript(Url) {
    return import(Url);
}

and script2.js contains:

export function hello2(message) {
    console.log(message);
}

1. It actually calls hello2.
2. We can't call non-exported functions on a module and there might be more to the function like some logic about what file it imports (not just parsing the Url directly).
3. Yep. I didn't want to use import as I feared that it might be a protected keyword and to say implicitly that it is another method than import.

[KristofferStrube]

This might just be a very specific use case so might not be necessary to handle for now?

[egil]

This might just be a very specific use case so might not be necessary to handle for now?

Yes and no. Its probably easier to design for this now than later.

3. Yep. I didn't want to use import as I feared that it might be a protected keyword and to say implicitly that it is another method than import.

My impression is that calls that follow the pattern jsRuntime.InvokeAsync<IJSObjectReference>("import", ...); will call a Blazor JavaScript function which does more or less what your importScript function does.

If this is the case, then I need to, in general, return a JSObjectReferenceInvocationHandler whenever a call to jsRuntime.InvokeAsync<TValue>() has IJSObjectReference as the TValue, independent of the following arguments.

If a user creates their own import function, where the second argument is not script name as it is on your case, but the import is hardcoded, e.g. export function importExtras() { return import("extras.js"); } then the handler doesn't know the name of the module based on the file name. I guess, in that case, I could say, that if the first argument is not "import", then the first argument is used as the module name.

[KristofferStrube]

Even though we handle jsRuntime.InvokeAsync<TValue>() with IJSObjectReference by returning JSObjectReferenceInvocationHandler then we should probably still make the warning if they call invoke("import", xxxx). We should probably either do this or just completely remove the SetupModule methods.

I think another way to name the module would be to add together all arguments concatenated since all arguments identify which module is imported (this of course depends on the implementation).

[egil]

Even though we handle jsRuntime.InvokeAsync<TValue>() with IJSObjectReference by returning JSObjectReferenceInvocationHandler then we should probably still make the warning if they call invoke("import", xxxx). We should probably either do this or just completely remove the SetupModule methods.

I like the SetupModule methods because they can have a specific return type of JSObjectReferenceInvocationHandler, which Setup<IJSObjectReference> cannot, forcing users to cast from JSRuntimeInvocationHandler<TResult>, which Setup<IJSObjectReference> returns, to JSObjectReferenceInvocationHandler to get access to all the features of it.

The need for casting could obviously be worked around by adding extension methods to JSRuntimeInvocationHandler<TResult> that mirror the Setup methods added to JSObjectReferenceInvocationHandler, making the need to cast almost moot. We are missing the Mode property though, and since extension properties is not a thing (yet) in C#, that would be missing.

The alternative, which I am leaning towards, is to continue to disallow setting up IJSObjectReference with the Setup method. Instead I would allow users to specify a custom "import" identifier if that is needed, using your example, this would be:

SetupModule(moduleName: "./js/script1.js");
SetupModule(identifier: "importScript", moduleName: "./script2.js");

That would mean we have the following overloads:

1. SetupModule(string moduleName);
2. SetupModule(string identifier, string moduleName);
3. SetupModule(string identifier, params object[] arguments);
4. SetupModule(InvocationMatcher invocationMatcher);
5. SetupModule(string identifier, InvocationMatcher invocationMatcher);
6. SetupModule(); // catch all

These basically mirror the Setup methods.

I think another way to name the module would be to add together all arguments concatenated since all arguments identify which module is imported (this of course depends on the implementation).

Since we cannot consistently get the name module, I wonder if it is just better to not prefix the invocation identifiers with the module name. Users can always inspect the JSObjectReferenceInvocationHandler.Invocations to see invocations that were captured by that handler, so they have the context that way. Not prefixing makes the experience consistent across the different styles, which is means users wont have their test break when switching between styles -- something that I find hughly important.

[KristofferStrube]

I like this approach. Covers all use cases and is very flexible while still using the JSObjectReferenceInvocationHandler. 😃

I agree that it should be fine to not prefix the invocation identifiers.

[KristofferStrube]

Something I worked with today while writing an article is IJSInProcessObjectReference.
Supporting cast to that should probably be its own issue, but it's probably easier to formulate once this issue is closed.

[egil]

Something I worked with today while writing an article is IJSInProcessObjectReference.
Supporting cast to that should probably be its own issue, but it's probably easier to formulate once this issue is closed.

How is it possible to cast/get a IJSInProcessObjectReference? I think I would like to add that support in the PR right away.

[egil]

Nevermind, figured it out. It seems it is only partially supported in Blazor, depending on how you setup your app. Anyway, its pretty easy to add support for it in bUnit, so Ill just do it now.

[KristofferStrube]

Cool.
What makes it partially supported? I couldn't find anything about that in the docs.

[egil]

Cool.
What makes it partially supported? I couldn't find anything about that in the docs.

I found an issue related to it, it is something that might make it into the next version. It is covered in the draft for Steves performance guidelines, so I think it is safe to assume it will be supported. Either way, it is not a problem for us to have it in now.