-
Notifications
You must be signed in to change notification settings - Fork 840
Make JSRuntime.Current non-public #1118
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
Conversation
| /// Gets the current <see cref="IJSRuntime"/>, if any. | ||
| /// </summary> | ||
| public static IJSRuntime Current => _currentJSRuntime.Value; | ||
| internal static IJSRuntime Current => _currentJSRuntime.Value; |
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.
It says remove. But from looking at it, I can see that it is not removed.
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.
Indeed. Could the entire JSRuntime.cs file be removed?
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.
Not in any trivial way. It's used from a static context by DotnetNetDispatcher to notify of completion:
- https://github.com/aspnet/Extensions/blob/master/src/JSInterop/Microsoft.JSInterop/src/DotNetDispatcher.cs#L99
- https://github.com/aspnet/Extensions/blob/master/src/JSInterop/Microsoft.JSInterop/src/DotNetDispatcher.cs#L192-L193
In addition, there's also some pretty gnarly coupling of the runtime \ SimpleJSON's serialization and object tracking. I was hoping not to mess around with the code here if it's meant to be cleaned up soon.
3a67975 to
3b46605
Compare
|
Bump |
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.
This accomplishes our goals for preview 3, we can revise later if necessary.
Fixes dotnet/aspnetcore#6828