-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[browser] [wasm] Make response streaming opt-out #111680
Changes from all commits
4b9af8e
1b42cf6
1f96a83
80cd37d
d93ad3c
8a68c96
019b411
770f69a
0930086
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 |
---|---|---|
|
@@ -11,11 +11,26 @@ namespace System.Net.Http | |
{ | ||
internal static partial class BrowserHttpInterop | ||
{ | ||
private static bool? _SupportsStreamingRequest; | ||
private static bool? _SupportsStreamingResponse; | ||
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. Since this is now cached in .NET we don't need to cache it in JS anymore. Does not hurt though and might still be relevant for MT. 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. I removed it there, thanks. I believe that the webworker has the same feature set as UI thread. |
||
|
||
public static bool SupportsStreamingRequest() | ||
{ | ||
_SupportsStreamingRequest ??= SupportsStreamingRequestImpl(); | ||
return _SupportsStreamingRequest.Value; | ||
} | ||
|
||
public static bool SupportsStreamingResponse() | ||
{ | ||
_SupportsStreamingResponse ??= SupportsStreamingResponseImpl(); | ||
return _SupportsStreamingResponse.Value; | ||
} | ||
|
||
[JSImport("INTERNAL.http_wasm_supports_streaming_request")] | ||
public static partial bool SupportsStreamingRequest(); | ||
public static partial bool SupportsStreamingRequestImpl(); | ||
|
||
[JSImport("INTERNAL.http_wasm_supports_streaming_response")] | ||
public static partial bool SupportsStreamingResponse(); | ||
public static partial bool SupportsStreamingResponseImpl(); | ||
|
||
[JSImport("INTERNAL.http_wasm_create_controller")] | ||
public static partial JSObject CreateController(); | ||
|
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.
What exactly is
value="false"
/featurevalue="false"
? The default value should betrue
or is this something else?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 means: if you set
System.Net.Http.WebAssemblyEnableStreamingResponse
tofalse
then ILLink will stub theget_FeatureEnableStreamingResponse
to returnfalse
instead of reading the config/env.We don't want to also hardcode
true
do we ?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 behavior here the same in development as in publish?
We have something similar in Blazor routing and we had to do extra work for the development case
https://github.com/dotnet/aspnetcore/blob/main/src/Components/Components/src/Properties/ILLink.Substitutions.xml#L6-L8 for routing constraints, and it doesn't work in development unless you specifically set the AppContextSwitch manually
https://github.com/dotnet/aspnetcore/blob/f25dc7be397b496cc1c71f8720b2b7d67fb18649/src/Components/WebAssembly/WebAssembly/src/Hosting/WebAssemblyHostBuilder.cs#L98-L112
My understanding is that
runtimeConfig.json
stuff doesn't flow to wasm at the moment.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.
#97449