-
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,18 +3,26 @@ | |
|
||
using System.Buffers; | ||
using System.Collections.Immutable; | ||
using Microsoft.Build.Graph; | ||
using Microsoft.CodeAnalysis.ExternalAccess.Watch.Api; | ||
|
||
namespace Microsoft.DotNet.Watch | ||
{ | ||
internal sealed class BlazorWebAssemblyDeltaApplier(IReporter reporter, BrowserRefreshServer browserRefreshServer, Version? targetFrameworkVersion) : SingleProcessDeltaApplier(reporter) | ||
internal sealed class BlazorWebAssemblyDeltaApplier(IReporter reporter, BrowserRefreshServer browserRefreshServer, ProjectGraphNode project) : SingleProcessDeltaApplier(reporter) | ||
{ | ||
private const string DefaultCapabilities60 = "Baseline"; | ||
private const string DefaultCapabilities70 = "Baseline AddMethodToExistingType AddStaticFieldToExistingType NewTypeDefinition ChangeCustomAttributes"; | ||
private const string DefaultCapabilities80 = "Baseline AddMethodToExistingType AddStaticFieldToExistingType NewTypeDefinition ChangeCustomAttributes AddInstanceFieldToExistingType GenericAddMethodToExistingType GenericUpdateMethod UpdateParameters GenericAddFieldToExistingType"; | ||
private static readonly ImmutableArray<string> s_defaultCapabilities60 = | ||
["Baseline"]; | ||
|
||
private static readonly ImmutableArray<string> s_defaultCapabilities70 = | ||
["Baseline", "AddMethodToExistingType", "AddStaticFieldToExistingType", "NewTypeDefinition", "ChangeCustomAttributes"]; | ||
|
||
private static readonly ImmutableArray<string> s_defaultCapabilities80 = | ||
["Baseline", "AddMethodToExistingType", "AddStaticFieldToExistingType", "NewTypeDefinition", "ChangeCustomAttributes", | ||
"AddInstanceFieldToExistingType", "GenericAddMethodToExistingType", "GenericUpdateMethod", "UpdateParameters", "GenericAddFieldToExistingType"]; | ||
|
||
private static readonly ImmutableArray<string> s_defaultCapabilities90 = | ||
s_defaultCapabilities80; | ||
|
||
private ImmutableArray<string> _cachedCapabilities; | ||
private readonly SemaphoreSlim _capabilityRetrievalSemaphore = new(initialCount: 1); | ||
private int _updateId; | ||
|
||
public override void Dispose() | ||
|
@@ -31,109 +39,31 @@ public override async Task WaitForProcessRunningAsync(CancellationToken cancella | |
// Alternatively, we could inject agent into blazor-devserver.dll and establish a connection on the named pipe. | ||
=> await browserRefreshServer.WaitForClientConnectionAsync(cancellationToken); | ||
|
||
public override async Task<ImmutableArray<string>> GetApplyUpdateCapabilitiesAsync(CancellationToken cancellationToken) | ||
public override Task<ImmutableArray<string>> GetApplyUpdateCapabilitiesAsync(CancellationToken cancellationToken) | ||
{ | ||
var cachedCapabilities = _cachedCapabilities; | ||
if (!cachedCapabilities.IsDefault) | ||
{ | ||
return cachedCapabilities; | ||
} | ||
var capabilities = project.GetWebAssemblyCapabilities(); | ||
|
||
await _capabilityRetrievalSemaphore.WaitAsync(cancellationToken); | ||
try | ||
{ | ||
if (_cachedCapabilities.IsDefault) | ||
{ | ||
_cachedCapabilities = await RetrieveAsync(cancellationToken); | ||
} | ||
} | ||
finally | ||
if (capabilities.IsEmpty) | ||
{ | ||
_capabilityRetrievalSemaphore.Release(); | ||
} | ||
var targetFramework = project.GetTargetFrameworkVersion(); | ||
|
||
return _cachedCapabilities; | ||
|
||
async Task<ImmutableArray<string>> RetrieveAsync(CancellationToken cancellationToken) | ||
{ | ||
var buffer = ArrayPool<byte>.Shared.Rent(32 * 1024); | ||
Reporter.Verbose($"Using capabilities based on target framework: '{targetFramework}'."); | ||
|
||
try | ||
capabilities = targetFramework?.Major switch | ||
{ | ||
Reporter.Verbose("Connecting to the browser."); | ||
|
||
await browserRefreshServer.WaitForClientConnectionAsync(cancellationToken); | ||
|
||
string capabilities; | ||
if (browserRefreshServer.Options.TestFlags.HasFlag(TestFlags.MockBrowser)) | ||
{ | ||
// When testing return default capabilities without connecting to an actual browser. | ||
capabilities = GetDefaultCapabilities(targetFrameworkVersion); | ||
} | ||
else | ||
{ | ||
string? capabilityString = null; | ||
|
||
await browserRefreshServer.SendAndReceiveAsync( | ||
request: _ => default(JsonGetApplyUpdateCapabilitiesRequest), | ||
response: (value, reporter) => | ||
{ | ||
var str = Encoding.UTF8.GetString(value); | ||
if (str.StartsWith('!')) | ||
{ | ||
reporter.Verbose($"Exception while reading WASM runtime capabilities: {str[1..]}"); | ||
} | ||
else if (str.Length == 0) | ||
{ | ||
reporter.Verbose($"Unable to read WASM runtime capabilities"); | ||
} | ||
else if (capabilityString == null) | ||
{ | ||
capabilityString = str; | ||
} | ||
else if (capabilityString != str) | ||
{ | ||
reporter.Verbose($"Received different capabilities from different browsers:{Environment.NewLine}'{str}'{Environment.NewLine}'{capabilityString}'"); | ||
} | ||
}, | ||
cancellationToken); | ||
|
||
if (capabilityString != null) | ||
{ | ||
capabilities = capabilityString; | ||
} | ||
else | ||
{ | ||
capabilities = GetDefaultCapabilities(targetFrameworkVersion); | ||
Reporter.Verbose($"Falling back to default WASM capabilities: '{capabilities}'"); | ||
} | ||
} | ||
|
||
// Capabilities are expressed a space-separated string. | ||
// e.g. https://github.com/dotnet/runtime/blob/14343bdc281102bf6fffa1ecdd920221d46761bc/src/coreclr/System.Private.CoreLib/src/System/Reflection/Metadata/AssemblyExtensions.cs#L87 | ||
return capabilities.Split(' ').ToImmutableArray(); | ||
} | ||
catch (Exception e) when (!cancellationToken.IsCancellationRequested) | ||
{ | ||
Reporter.Error($"Failed to read capabilities: {e.Message}"); | ||
|
||
// Do not attempt to retrieve capabilities again if it fails once, unless the operation is canceled. | ||
return []; | ||
} | ||
finally | ||
{ | ||
ArrayPool<byte>.Shared.Return(buffer); | ||
} | ||
9 => s_defaultCapabilities90, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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. |
||
8 => s_defaultCapabilities80, | ||
7 => s_defaultCapabilities70, | ||
6 => s_defaultCapabilities60, | ||
_ => [], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think we have condition elsewhere that requires net6+ |
||
}; | ||
} | ||
else | ||
{ | ||
Reporter.Verbose($"Project specifies capabilities."); | ||
} | ||
|
||
static string GetDefaultCapabilities(Version? targetFrameworkVersion) | ||
=> targetFrameworkVersion?.Major switch | ||
{ | ||
>= 8 => DefaultCapabilities80, | ||
>= 7 => DefaultCapabilities70, | ||
>= 6 => DefaultCapabilities60, | ||
_ => string.Empty, | ||
}; | ||
return Task.FromResult(capabilities); | ||
} | ||
|
||
public override async Task<ApplyStatus> Apply(ImmutableArray<WatchHotReloadService.Update> updates, CancellationToken cancellationToken) | ||
|
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.