MockJSRuntime: Consider using TryAddSingleton in AddMockJSRuntime()

[egil]

To avoid confusion when a IJSRuntime has already been registered with the TestServiceProvider, lets use TryAddSingleton instead and throw an exception if it or some other mock has already been registered.

[joro550]

Hey there! I wanted to pick up this issue but I'm a little confused by the level of detail so I hope you don't mind me rambling a bit:

The TestContext seems to add a PlaceholderJSRuntime which means that any call to TryAddSingleton will fail as it already has a service that is associated with IJsRuntime. From some testing I see that if you provide multiple associative classes to a service provider and ask for the service then it will give you the last associated class - this is what I assume you mean by "to avoid confusion".

[egil]

Hey Mark,

You raise a good point. To be honest, I didn't think of that before I added this issue, so this might not be a good idea after all.

What's your take?

I guess my idea was that if two competing helper methods both register a IJSRuntime, the built in one should throw if added. But users should really be aware of what they are doing at this level, so it might not be the right thing to do. Users might indeed want to override an existing registration.

[joro550]

Yeah - it's a difficult one because you would hope that users should be aware of what they are doing if they get to these lower levels.

You could remove the PlaceholderJSRuntime from the TestContext so that when a user injects the IJsRuntime into a page it forces them into registering a runtime, prompting them to look up the documentation on the extension methods, which could use the TryAddSingleton which again could be pointed out in the documentation by saying something to the affect of

This will only add an instance of IJsRuntime if one doesn't all ready exist

This would also decouple you from the (I guess you could call it a) side effect of the di always injecting the last known class, meaning that if a consumer is brave enough to register their own they would have to know these things by looking at the documentaiton of the service provider that Microsoft owns.

Alternatively you could leave it as is and rely on the fact that people doing this sort of lower level injection are aware of what they are doing.

I think either way some notes on this sort of thing in the documentation would be great - so that consumers know the potential pitfalls that can happen here.

Just as a side note, not really that relevant is the kind of funny thing of users doing something like @inject IEnumerable<IJsRuntime> runtime which would give them all registrations 😄

[egil]

Just as a side note, not really that relevant is the kind of funny thing of users doing something like @inject IEnumerable<IJsRuntime> runtime which would give them all registrations 😄

LOL, didnt know.

Anyway, I think this issue should be parked for now, and I will reopen it if turns out to be big problem.

The point with PlaceholderJSRuntime is make it easier for users to discover missing registrations, and provide an exception that points them to the documentation that shows how to solve their problem, and it seems that it works very well from what I can gather.

If you want to work on something related to this, then this issue obvious: #115

Thanks for taking the time Mark!