Skip to content
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

In SSR, supply HttpContext as cascading value #50253

Merged
merged 5 commits into from
Aug 23, 2023

Conversation

SteveSandersonMS
Copy link
Member

@SteveSandersonMS SteveSandersonMS commented Aug 22, 2023

Fixes #48769

Usage

[CascadingParameter] public HttpContext? Context { get; set; }

In SSR, this will receive the HttpContext. In other rendering modes where there is no HTTP context, no value will be supplied so the property will remain null.

Alternative design considered

In #48769 I suggested using [SupplyParameterFromHttpContext] but that turns out not to be practical unless we either (a) make ICascadingValueSupplier public, or (b) add an IVT from M.A.Components to .Endpoints.

I'm not keen on doing either of the above on a whim. Plus, use of [CascadingValue] has advantages:

  • It's consistent with the existing pattern for authentication state (we have [CascadingParameter] Task<AuthenticationState> AuthenticationStateTask { get; set; }).
  • Longer term, if we add more things like this, it would be nice not to add a separate special attribute for each one when [CascadingParameter] is already descriptive enough. Special attributes are needed only when the type of thing being supplied might reasonably clash with something else the application is doing (for example, we do need it for form/query, as they supply arbitrary types).

Review notes

It's best to look at the two commits in this PR completely separately:

  1. The first commit fixes an API design problem I discovered while considering how to do this. I realised that when we added CascadingParameterAttributeBase, we made a design mistake:
    • We put the Name property on the abstract base class just because CascadingParameterAttribute, SupplyParameterFromQuery, and SupplyParameterFromForm all have a Name.
    • However, in all three cases there, the Name has completely different meanings. For CascadingParameterAttribute, it's the name associated with <CascadingValue Name=...>, whereas for form it's the Request.Form entry or fall back on property name, and for query it's the Request.Query entry or fall back on property name. In general there's no reason why a CascadingParameterAttributeBase subclass should have a Name at all (SupplyParameterFromHttpContext wasn't going to), and if it does have one, its semantics are specific to it. So these should not be the same properties.
    • The change we made to make CascadingParameterAttribute.Name virtual might even be breaking (see https://learn.microsoft.com/en-us/dotnet/core/compatibility/library-change-rules stating DISALLOWED: Adding the virtual keyword to a member). So it's good we can revert that here.
  2. The second commit is the completely trivial implementation of supplying HttpContext as a cascading value, with an E2E test.

…'t have been in the base because the meaning varies.
@SteveSandersonMS SteveSandersonMS requested a review from a team as a code owner August 22, 2023 09:31
@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Aug 22, 2023
@SteveSandersonMS SteveSandersonMS self-assigned this Aug 22, 2023
@SteveSandersonMS SteveSandersonMS added this to the 8.0-rc2 milestone Aug 22, 2023
@SteveSandersonMS SteveSandersonMS force-pushed the stevesa/supplyparameterfromhttpcontext branch from 2b670d2 to a27e4c4 Compare August 22, 2023 09:40
@SteveSandersonMS SteveSandersonMS force-pushed the stevesa/supplyparameterfromhttpcontext branch from fb52e1a to 44f8b9d Compare August 22, 2023 12:25
@SteveSandersonMS
Copy link
Member Author

@javiercn Thanks for reviewing.

This is worth a quick re-review (at least of the new final commit) because the tests failed in a way that showed we had a missing scenario and I think need some new public API. The "add cascading parameter at root" API only had an Add flavour and not a TryAdd, but our DI setup logic would always want to use TryAdd for this. Application and library developers will also have reasons to want the TryAdd version of DI registration too. So I've added this here. Any concerns?

@@ -63,6 +63,7 @@ public static IRazorComponentsBuilder AddRazorComponents(this IServiceCollection
services.TryAddScoped<EndpointRoutingStateProvider>();
services.TryAddScoped<IRoutingStateProvider>(sp => sp.GetRequiredService<EndpointRoutingStateProvider>());
services.AddSupplyValueFromQueryProvider();
services.TryAddCascadingValue(sp => sp.GetRequiredService<EndpointHtmlRenderer>().HttpContext);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we always register an EndpointHtmlRenderer? Should this be sp.GetService?.HttpContext`?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're running this code, we do always register it - that's on line 53 above.

@SteveSandersonMS SteveSandersonMS requested a review from a team as a code owner August 23, 2023 12:29
@SteveSandersonMS
Copy link
Member Author

@mkArtak Another one to merge for release/8.0. Again this changes PublicAPI.Unshipped.txt so the code check failure is expected and should be merged regardless.

@mkArtakMSFT mkArtakMSFT merged commit 5ad966e into release/8.0 Aug 23, 2023
@mkArtakMSFT mkArtakMSFT deleted the stevesa/supplyparameterfromhttpcontext branch August 23, 2023 15:51
@davidfowl
Copy link
Member

Very nice making this a cascading value

@ghost ghost modified the milestone: 8.0-rc2 Aug 23, 2023
MackinnonBuck added a commit that referenced this pull request Aug 28, 2023
* Remove reflection from KestrelServer constructor (#50272)

* Update dependencies from https://github.com/dotnet/extensions build 20230822.1 (#50288)

[release/8.0] Update dependencies from dotnet/extensions

* Ensure enhanced nav requests have the correct headers (#50263)

* In SSR, supply HttpContext as cascading value (#50253)

Fixes #48769

### Usage

```cs
[CascadingParameter] public HttpContext? Context { get; set; }
```

In SSR, this will receive the `HttpContext`. In other rendering modes where there is no HTTP context, no value will be supplied so the property will remain `null`.

### Alternative design considered

In #48769 I suggested using `[SupplyParameterFromHttpContext]` but that turns out not to be practical unless we either (a) make `ICascadingValueSupplier` public, or (b) add an IVT from `M.A.Components` to `.Endpoints`.

I'm not keen on doing either of the above on a whim. Plus, use of `[CascadingValue]` has advantages:

 * It's consistent with the existing pattern for authentication state (we have `[CascadingParameter] Task<AuthenticationState> AuthenticationStateTask { get; set; }`).
 * Longer term, if we add more things like this, it would be nice not to add a separate special attribute for each one when `[CascadingParameter]` is already descriptive enough. Special attributes are needed only when the type of thing being supplied might reasonably clash with something else the application is doing (for example, we do need it for form/query, as they supply arbitrary types).

## Review notes

It's best to look at the two commits in this PR completely separately:

1. The first commit fixes an API design problem I discovered while considering how to do this. I realised that when we added `CascadingParameterAttributeBase`, we made a design mistake:
   * We put the `Name` property on the abstract base class just because `CascadingParameterAttribute`, `SupplyParameterFromQuery`, and `SupplyParameterFromForm` all have a `Name`.
   * However, in all three cases there, the `Name` has completely different meanings. For `CascadingParameterAttribute`, it's the name associated with `<CascadingValue Name=...>`, whereas for form it's the `Request.Form` entry or fall back on property name, and for query it's the `Request.Query` entry or fall back on property name. In general there's no reason why a `CascadingParameterAttributeBase` subclass should have a `Name` at all (`SupplyParameterFromHttpContext` wasn't going to), and if it does have one, its semantics are specific to it. So these should not be the same properties.
   * The change we made to make `CascadingParameterAttribute.Name` virtual might even be breaking (see https://learn.microsoft.com/en-us/dotnet/core/compatibility/library-change-rules stating *DISALLOWED: Adding the virtual keyword to a member*). So it's good we can revert that here.
2. The second commit is the completely trivial implementation of supplying `HttpContext` as a cascading value, with an E2E test.

* Fix SSR redirections. Fixes #49670 (#50261)

Fixes #49670 trivially by making `HttpNavigationManager`'s redirection logic consistent with the other `NavigationManager` implementations.

You'll be wondering why there are no E2E tests with this PR. The reason is that, surprisingly, we *already* have E2E tests for these kinds of non-root redirections. However, they were falsely passing, which is tracked by #50260 and is outside the scope of this PR. I think we should address that other issue in .NET 9 as there should be no developer/API impact from leaving it as-is for .NET 8.

* API review followups (#50181)

* RazorComponentResult: change namespace, make executor internal, merge test classes

* RazorComponentResult: Nullability and trim annotations

* RazorComponentResult: IStatusCodeHttpResult and IContentTypeHttpResult

* Cleanup

* Further clean up annotations

* Remove HtmlRootComponent.ComponentId as per API review

* Rename SupplyParameterFromFormAttribute.Handler to FormName

* API review: seal

* Clarify RenderModeAttribute inheritance. Fixes #49848

* Rename valueFactory -> initialValueFactory

* Remove unnecessary sequence params

* Make [StreamRendering] default to true

* Support hot reload for Blazor component endpoints (#50031)

* Support hot reload for Blazor component endpoints

* Use alternative navigation exports

* Address feedback

* Make HotReloadService optional

* Address more feedback

* [release/8.0] Update SDK (#50276)

* [release/8.0] Update SDK

* Fix IsAotCompatible warnings

* Update nullability annotation for TemplatePart.Text

* Update CodeGen.proj to account for conditional IsTrimmable metadata

* Update Directory.Build.props.in

---------

Co-authored-by: Stephen Halter <halter73@gmail.com>

* Provide a better error (#50311)

* Remove more setup overhead in RDG benchmarks (#50302)

* Use polling to watch certificate paths (#50251)

* Cache parsable and bindable type info in RDG (#50326)

* Use absolute URLs for confirmation emails (#50297)

This PR updates the MapIdentityApi endpoints that send confirmation emails to use absolute URLs. Previously the emails used relative links which of course do not work for links in emails.

Fixes #50296

* [release/8.0] [Blazor] Close the circuit when all Blazor Server components are disposed (#50170)

# [Blazor] Close the circuit when all Blazor Server components are disposed

Allows a Blazor Server circuit to close when all root Blazor Server components get dynamically removed from the page.

## Description

The overall approach I've taken is:
1. Define what it means for the circuit to be in use (`WebRootComponentManager.hasAnyExistingOrPendingServerComponents()`):
    * There are interactive Blazor Server components on the page, or...
    * The initialization of an interactive Blazor Server component has started, but hasn't completed yet, or...
    * There are SSR'd components on the page that haven't been initialized for interactivity yet (consider stream rendering, where we don't activate new components until the response completes), but they have either a "Server" or "Auto" render mode
2. Determine the cases where a circuit's "in use" status may have changed:
    * After a render batch is applied (see `attachCircuitAfterRenderCallback` in `WebRootComponentManager.ts`)
      * An applied render batch may result in the creation/disposal of a root component
    * After an SSR update occurs, but before the first render batch is applied
      * Consider the case where an SSR'd component with a Server render mode gets removed from the page, but before the circuit has a chance to initialize
3. Decide what to do if the "in use" status may have changed (`WebRootComponentManager.circuitMayHaveNoRootComponents()`):
    * If the circuit is not in use:
      * Start a timeout with some configurable duration (`SsrStartOptions.circuitInactivityTimeoutMs`), if it hasn't started already
      * When the timeout expires, dispose the circuit
    * If the circuit is not in use:
      * Clear any existing timeout

This PR also:
- [X] Addresses a bug preventing Virtualize from working correctly when a WebAssembly and Server instance is present on the page at the same time
- [X] Adds E2E tests

Fixes #48765

* [release/8.0] Update dependencies from dotnet/runtime (#50305)

* Update dependencies from https://github.com/dotnet/runtime build 20230823.11

Microsoft.Bcl.AsyncInterfaces , Microsoft.Bcl.TimeProvider , Microsoft.Extensions.Caching.Abstractions , Microsoft.Extensions.Caching.Memory , Microsoft.Extensions.Configuration , Microsoft.Extensions.Configuration.Abstractions , Microsoft.Extensions.Configuration.Binder , Microsoft.Extensions.Configuration.CommandLine , Microsoft.Extensions.Configuration.EnvironmentVariables , Microsoft.Extensions.Configuration.FileExtensions , Microsoft.Extensions.Configuration.Ini , Microsoft.Extensions.Configuration.Json , Microsoft.Extensions.Configuration.UserSecrets , Microsoft.Extensions.Configuration.Xml , Microsoft.Extensions.DependencyInjection , Microsoft.Extensions.DependencyInjection.Abstractions , Microsoft.Extensions.DependencyModel , Microsoft.Extensions.Diagnostics , Microsoft.Extensions.Diagnostics.Abstractions , Microsoft.Extensions.FileProviders.Abstractions , Microsoft.Extensions.FileProviders.Composite , Microsoft.Extensions.FileProviders.Physical , Microsoft.Extensions.FileSystemGlobbing , Microsoft.Extensions.HostFactoryResolver.Sources , Microsoft.Extensions.Hosting , Microsoft.Extensions.Hosting.Abstractions , Microsoft.Extensions.Http , Microsoft.Extensions.Logging , Microsoft.Extensions.Logging.Abstractions , Microsoft.Extensions.Logging.Configuration , Microsoft.Extensions.Logging.Console , Microsoft.Extensions.Logging.Debug , Microsoft.Extensions.Logging.EventLog , Microsoft.Extensions.Logging.EventSource , Microsoft.Extensions.Logging.TraceSource , Microsoft.Extensions.Options , Microsoft.Extensions.Options.ConfigurationExtensions , Microsoft.Extensions.Options.DataAnnotations , Microsoft.Extensions.Primitives , Microsoft.Internal.Runtime.AspNetCore.Transport , Microsoft.NET.Runtime.MonoAOTCompiler.Task , Microsoft.NET.Runtime.WebAssembly.Sdk , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.AOT.win-x64.Cross.browser-wasm , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.BrowserDebugHost.Transport , Microsoft.NETCore.Platforms , System.Collections.Immutable , System.Composition , System.Configuration.ConfigurationManager , System.Diagnostics.DiagnosticSource , System.Diagnostics.EventLog , System.Diagnostics.PerformanceCounter , System.DirectoryServices.Protocols , System.IO.Hashing , System.IO.Pipelines , System.Net.Http.Json , System.Net.Http.WinHttpHandler , System.Reflection.Metadata , System.Resources.Extensions , System.Runtime.Caching , System.Security.Cryptography.Pkcs , System.Security.Cryptography.Xml , System.Security.Permissions , System.ServiceProcess.ServiceController , System.Text.Encodings.Web , System.Text.Json , System.Threading.AccessControl , System.Threading.Channels , System.Threading.RateLimiting
 From Version 8.0.0-rc.2.23418.14 -> To Version 8.0.0-rc.2.23423.11

* Update dependencies from https://github.com/dotnet/runtime build 20230823.5

Microsoft.Bcl.AsyncInterfaces , Microsoft.Bcl.TimeProvider , Microsoft.Extensions.Caching.Abstractions , Microsoft.Extensions.Caching.Memory , Microsoft.Extensions.Configuration , Microsoft.Extensions.Configuration.Abstractions , Microsoft.Extensions.Configuration.Binder , Microsoft.Extensions.Configuration.CommandLine , Microsoft.Extensions.Configuration.EnvironmentVariables , Microsoft.Extensions.Configuration.FileExtensions , Microsoft.Extensions.Configuration.Ini , Microsoft.Extensions.Configuration.Json , Microsoft.Extensions.Configuration.UserSecrets , Microsoft.Extensions.Configuration.Xml , Microsoft.Extensions.DependencyInjection , Microsoft.Extensions.DependencyInjection.Abstractions , Microsoft.Extensions.DependencyModel , Microsoft.Extensions.Diagnostics , Microsoft.Extensions.Diagnostics.Abstractions , Microsoft.Extensions.FileProviders.Abstractions , Microsoft.Extensions.FileProviders.Composite , Microsoft.Extensions.FileProviders.Physical , Microsoft.Extensions.FileSystemGlobbing , Microsoft.Extensions.HostFactoryResolver.Sources , Microsoft.Extensions.Hosting , Microsoft.Extensions.Hosting.Abstractions , Microsoft.Extensions.Http , Microsoft.Extensions.Logging , Microsoft.Extensions.Logging.Abstractions , Microsoft.Extensions.Logging.Configuration , Microsoft.Extensions.Logging.Console , Microsoft.Extensions.Logging.Debug , Microsoft.Extensions.Logging.EventLog , Microsoft.Extensions.Logging.EventSource , Microsoft.Extensions.Logging.TraceSource , Microsoft.Extensions.Options , Microsoft.Extensions.Options.ConfigurationExtensions , Microsoft.Extensions.Options.DataAnnotations , Microsoft.Extensions.Primitives , Microsoft.Internal.Runtime.AspNetCore.Transport , Microsoft.NET.Runtime.MonoAOTCompiler.Task , Microsoft.NET.Runtime.WebAssembly.Sdk , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.AOT.win-x64.Cross.browser-wasm , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.BrowserDebugHost.Transport , Microsoft.NETCore.Platforms , System.Collections.Immutable , System.Composition , System.Configuration.ConfigurationManager , System.Diagnostics.DiagnosticSource , System.Diagnostics.EventLog , System.Diagnostics.PerformanceCounter , System.DirectoryServices.Protocols , System.IO.Hashing , System.IO.Pipelines , System.Net.Http.Json , System.Net.Http.WinHttpHandler , System.Reflection.Metadata , System.Resources.Extensions , System.Runtime.Caching , System.Security.Cryptography.Pkcs , System.Security.Cryptography.Xml , System.Security.Permissions , System.ServiceProcess.ServiceController , System.Text.Encodings.Web , System.Text.Json , System.Threading.AccessControl , System.Threading.Channels , System.Threading.RateLimiting
 From Version 8.0.0-rc.2.23418.14 -> To Version 8.0.0-rc.2.23423.5

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Update Version.Details.xml

* Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20230824.1 (#50337)

[release/8.0] Update dependencies from dotnet/source-build-reference-packages

* Update Microsoft.Identity.Model to 7.0.0-preview3 (#50218)

* Update Microsoft.Identity.Model to 7.0.0-preview3

* fixing null refs in WsFed

* Add link to tracking issue.

* Add link to tracking issue.

* Update dependencies from https://github.com/dotnet/source-build-externals build 20230824.1

Microsoft.SourceBuild.Intermediate.source-build-externals
 From Version 8.0.0-alpha.1.23421.1 -> To Version 8.0.0-alpha.1.23424.1

* Workaround AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet#2261 in a test.

---------

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Update Versions.props

* Quarantine tests (#50379)

Co-authored-by: Mackinnon Buck <mackinnon.buck@gmail.com>

* [release/8.0] Update dependencies from dotnet/runtime (#50350)

* Update dependencies from https://github.com/dotnet/runtime build 20230825.9

Microsoft.Bcl.AsyncInterfaces , Microsoft.Bcl.TimeProvider , Microsoft.Extensions.Caching.Abstractions , Microsoft.Extensions.Caching.Memory , Microsoft.Extensions.Configuration , Microsoft.Extensions.Configuration.Abstractions , Microsoft.Extensions.Configuration.Binder , Microsoft.Extensions.Configuration.CommandLine , Microsoft.Extensions.Configuration.EnvironmentVariables , Microsoft.Extensions.Configuration.FileExtensions , Microsoft.Extensions.Configuration.Ini , Microsoft.Extensions.Configuration.Json , Microsoft.Extensions.Configuration.UserSecrets , Microsoft.Extensions.Configuration.Xml , Microsoft.Extensions.DependencyInjection , Microsoft.Extensions.DependencyInjection.Abstractions , Microsoft.Extensions.DependencyModel , Microsoft.Extensions.Diagnostics , Microsoft.Extensions.Diagnostics.Abstractions , Microsoft.Extensions.FileProviders.Abstractions , Microsoft.Extensions.FileProviders.Composite , Microsoft.Extensions.FileProviders.Physical , Microsoft.Extensions.FileSystemGlobbing , Microsoft.Extensions.HostFactoryResolver.Sources , Microsoft.Extensions.Hosting , Microsoft.Extensions.Hosting.Abstractions , Microsoft.Extensions.Http , Microsoft.Extensions.Logging , Microsoft.Extensions.Logging.Abstractions , Microsoft.Extensions.Logging.Configuration , Microsoft.Extensions.Logging.Console , Microsoft.Extensions.Logging.Debug , Microsoft.Extensions.Logging.EventLog , Microsoft.Extensions.Logging.EventSource , Microsoft.Extensions.Logging.TraceSource , Microsoft.Extensions.Options , Microsoft.Extensions.Options.ConfigurationExtensions , Microsoft.Extensions.Options.DataAnnotations , Microsoft.Extensions.Primitives , Microsoft.Internal.Runtime.AspNetCore.Transport , Microsoft.NET.Runtime.MonoAOTCompiler.Task , Microsoft.NET.Runtime.WebAssembly.Sdk , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.AOT.win-x64.Cross.browser-wasm , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.BrowserDebugHost.Transport , Microsoft.NETCore.Platforms , System.Collections.Immutable , System.Composition , System.Configuration.ConfigurationManager , System.Diagnostics.DiagnosticSource , System.Diagnostics.EventLog , System.Diagnostics.PerformanceCounter , System.DirectoryServices.Protocols , System.IO.Hashing , System.IO.Pipelines , System.Net.Http.Json , System.Net.Http.WinHttpHandler , System.Reflection.Metadata , System.Resources.Extensions , System.Runtime.Caching , System.Security.Cryptography.Pkcs , System.Security.Cryptography.Xml , System.Security.Permissions , System.ServiceProcess.ServiceController , System.Text.Encodings.Web , System.Text.Json , System.Threading.AccessControl , System.Threading.Channels , System.Threading.RateLimiting
 From Version 8.0.0-rc.2.23423.5 -> To Version 8.0.0-rc.2.23425.9

* Update dependencies from https://github.com/dotnet/runtime build 20230826.4

Microsoft.Bcl.AsyncInterfaces , Microsoft.Bcl.TimeProvider , Microsoft.Extensions.Caching.Abstractions , Microsoft.Extensions.Caching.Memory , Microsoft.Extensions.Configuration , Microsoft.Extensions.Configuration.Abstractions , Microsoft.Extensions.Configuration.Binder , Microsoft.Extensions.Configuration.CommandLine , Microsoft.Extensions.Configuration.EnvironmentVariables , Microsoft.Extensions.Configuration.FileExtensions , Microsoft.Extensions.Configuration.Ini , Microsoft.Extensions.Configuration.Json , Microsoft.Extensions.Configuration.UserSecrets , Microsoft.Extensions.Configuration.Xml , Microsoft.Extensions.DependencyInjection , Microsoft.Extensions.DependencyInjection.Abstractions , Microsoft.Extensions.DependencyModel , Microsoft.Extensions.Diagnostics , Microsoft.Extensions.Diagnostics.Abstractions , Microsoft.Extensions.FileProviders.Abstractions , Microsoft.Extensions.FileProviders.Composite , Microsoft.Extensions.FileProviders.Physical , Microsoft.Extensions.FileSystemGlobbing , Microsoft.Extensions.HostFactoryResolver.Sources , Microsoft.Extensions.Hosting , Microsoft.Extensions.Hosting.Abstractions , Microsoft.Extensions.Http , Microsoft.Extensions.Logging , Microsoft.Extensions.Logging.Abstractions , Microsoft.Extensions.Logging.Configuration , Microsoft.Extensions.Logging.Console , Microsoft.Extensions.Logging.Debug , Microsoft.Extensions.Logging.EventLog , Microsoft.Extensions.Logging.EventSource , Microsoft.Extensions.Logging.TraceSource , Microsoft.Extensions.Options , Microsoft.Extensions.Options.ConfigurationExtensions , Microsoft.Extensions.Options.DataAnnotations , Microsoft.Extensions.Primitives , Microsoft.Internal.Runtime.AspNetCore.Transport , Microsoft.NET.Runtime.MonoAOTCompiler.Task , Microsoft.NET.Runtime.WebAssembly.Sdk , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.AOT.win-x64.Cross.browser-wasm , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.BrowserDebugHost.Transport , Microsoft.NETCore.Platforms , System.Collections.Immutable , System.Composition , System.Configuration.ConfigurationManager , System.Diagnostics.DiagnosticSource , System.Diagnostics.EventLog , System.Diagnostics.PerformanceCounter , System.DirectoryServices.Protocols , System.IO.Hashing , System.IO.Pipelines , System.Net.Http.Json , System.Net.Http.WinHttpHandler , System.Reflection.Metadata , System.Resources.Extensions , System.Runtime.Caching , System.Security.Cryptography.Pkcs , System.Security.Cryptography.Xml , System.Security.Permissions , System.ServiceProcess.ServiceController , System.Text.Encodings.Web , System.Text.Json , System.Threading.AccessControl , System.Threading.Channels , System.Threading.RateLimiting
 From Version 8.0.0-rc.2.23423.5 -> To Version 8.0.0-rc.2.23426.4

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Update Versions.props

---------

Co-authored-by: Stephen Halter <halter73@gmail.com>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Steve Sanderson <SteveSandersonMS@users.noreply.github.com>
Co-authored-by: Safia Abdalla <safia@microsoft.com>
Co-authored-by: Andrew Casey <amcasey@users.noreply.github.com>
Co-authored-by: Mackinnon Buck <mackinnon.buck@gmail.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: jennyf19 <jeferrie@microsoft.com>
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
Co-authored-by: William Godbe <wigodbe@microsoft.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@iphdav
Copy link

iphdav commented Sep 25, 2023

@SteveSandersonMS @MackinnonBuck I like what has been implemented here. However I just want to give the feedback that it will seem strange to most end developers using blazor server (Interactive Server), that they can inject IHttpContextAccessor into a component, and yet get NULL if they have a cascading parameter of type HttpContext.

We all completely understand that at this point in the lifecycle we are on a web socket connection (and cannot for example change the response or set a http header). But lots of "Interactive Server" components still check for maybe the User (on the HttpContext) or check the URL they are on (rather than using NavigationManager); or check a cookie, etc. All read-only stuff, which should be fine.

I am just pointing out the inconsistency with "Interactive Server" of allowing IHttpContextAccessor; yet getting NULL for the Cascading Parameter implementation you have done. It just seems yet another thing that would need explaining to new developers. Why not align it with the behaviour of IHttpContextAccessor for developers that prefer to use "Interactive Server" mode?

@ghost
Copy link

ghost commented Sep 25, 2023

Hi @iphdav. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@Sasino97
Copy link

Sasino97 commented Feb 3, 2024

@SteveSandersonMS @MackinnonBuck I like what has been implemented here. However I just want to give the feedback that it will seem strange to most end developers using blazor server (Interactive Server), that they can inject IHttpContextAccessor into a component, and yet get NULL if they have a cascading parameter of type HttpContext.

We all completely understand that at this point in the lifecycle we are on a web socket connection (and cannot for example change the response or set a http header). But lots of "Interactive Server" components still check for maybe the User (on the HttpContext) or check the URL they are on (rather than using NavigationManager); or check a cookie, etc. All read-only stuff, which should be fine.

I am just pointing out the inconsistency with "Interactive Server" of allowing IHttpContextAccessor; yet getting NULL for the Cascading Parameter implementation you have done. It just seems yet another thing that would need explaining to new developers. Why not align it with the behaviour of IHttpContextAccessor for developers that prefer to use "Interactive Server" mode?

I agree, as a new Blazor dev I'm completely confused. I understand that there's no HTTP context once the initial connection has been made, but why isn't there a straightforward way to get the state of that initial connection, so that for example I can get the request headers, the IP address, and other important values.

@ghost
Copy link

ghost commented Feb 3, 2024

Hi @Sasino97. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants