-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Use WebAssemblyHotReloadCapabilities project property #45055
base: release/9.0.2xx
Are you sure you want to change the base?
Conversation
... to determine WASM capabilities instead of querying the runtime, which might not be loaded yet.
Thanks for your PR, @tmat. |
@phil-allen-msft ptal |
{ | ||
ArrayPool<byte>.Shared.Return(buffer); | ||
} | ||
9 => s_defaultCapabilities90, |
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 looks like this is the first time that "9" got a value. Is there a way to get the newest major version TFM to flag something? Is there a consistent comment people put on code to mark that it needs an update whenever a new TFM major version is introduced?
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 won't need updating anymore (this is the last update) since the new ASP.NET packages will specify the capabilities using the msbuild property.
{ | ||
string? capabilityString = null; | ||
|
||
await browserRefreshServer.SendAndReceiveAsync( |
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.
Here's my read: In the past, the runtime would add a new hot reload capability, and we would detect it with this code; there was a fallback based on TFM alone if the runtime was unreachable. In the future, the project (has access to SDK/build targets/build props) has the main info, and there is a fallback based on TFM alone in case the SDK/build targets don't return everything.
Does this sound right?
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.
Yes, the TFM based fallback is for projects referencing older packages that do not set the msbuild property.
8 => s_defaultCapabilities80, | ||
7 => s_defaultCapabilities70, | ||
6 => s_defaultCapabilities60, | ||
_ => [], |
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.
Is the minimum TFM for BlazorWasm net6?
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.
Yes, I think we have condition elsewhere that requires net6+
... to determine WASM capabilities instead of querying the runtime, which might not be loaded yet.
Contributes to a fix of dotnet/aspnetcore#59027
Related: dotnet/aspnetcore#59094