From 0388cdeb8297cb6672aea4033990acb6b2432431 Mon Sep 17 00:00:00 2001 From: PureWeen <223556219+Copilot@users.noreply.github.com> Date: Fri, 21 Aug 2026 15:24:10 -0500 Subject: [PATCH 1/2] [Blazor] Document Components design invariants Adds a Design invariants section to src/Components/AGENTS.md covering render mode agnosticism, HttpContext access, cross-render-mode service registration, rendering and the synchronization context, IHandleEvent, trim/AOT annotations, dependencies, and async coordination in tests. Each rule was checked against current source, and the legitimate exceptions are recorded alongside it so the guidance does not produce false review findings. Also clarifies that Selenium is the E2E framework for this area and that the Playwright references are the Playwright MCP browser tool used for manual validation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/Components/AGENTS.md | 125 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 124 insertions(+), 1 deletion(-) diff --git a/src/Components/AGENTS.md b/src/Components/AGENTS.md index fd579b2a3447..5a5ef0397e8f 100644 --- a/src/Components/AGENTS.md +++ b/src/Components/AGENTS.md @@ -66,6 +66,126 @@ Together these cover every interactivity platform (Server/WebAssembly/Auto/None) 6. **Clean up sample code** - After your E2E tests are passing, remove the sample code you added to the Samples projects. The sample was only for development and interactive testing; the E2E tests now provide the permanent test coverage. Use `git checkout -- src/Components/Samples` and `git clean -df -- src/Components/Samples` to remove the sample code. +## Design invariants + +These are long-standing design rules for framework code in this area. Each one lists its real +exceptions - respect those, because the exceptions are legitimate existing patterns, not +violations to clean up. + +### Render modes + +- **Framework components must be render-mode agnostic** so they work when consumed from a + Razor Class Library. Framework components never use `@rendermode` to force a mode - it is a + choice for the app that consumes the component, not for the component itself. +- **Exception:** a deliberately small set of render-mode-specific components ships in + `Microsoft.AspNetCore.Components.Endpoints` for static SSR - `CacheView`, `BasePath`, + `ImportMap`, and `ResourcePreloader`. Keep that set small; do not add to it without a strong + reason. +- Public APIs should work in every render mode. The exception is APIs defined in a concrete + render-mode assembly (`Endpoints`, `Server`, `WebAssembly`, `WebView`), which are not + intended for class-library consumption. Anything meant to be consumed broadly belongs in an + agnostic assembly such as `Components`, `Web`, or `Forms`. + +### Accessing `HttpContext` + +- **Do not use `IHttpContextAccessor`** in framework components or framework services. It does + not exist in any render mode other than static SSR. +- Components receive it as a cascading parameter instead - note the consuming attribute is + `[CascadingParameter]`: + + ```csharp + [CascadingParameter] public HttpContext? HttpContext { get; set; } + ``` + + See `Endpoints/src/Assets/ImportMap.cs` and `Endpoints/src/CacheView/CacheView.cs`. +- Services take `HttpContext` as a **method parameter**, never as a constructor dependency. + `EndpointAntiforgeryStateProvider` is the reference: it takes `IAntiforgery` in its + constructor and receives the context through `SetRequestContext(HttpContext)`, then falls + back to persisted state when there is no context. + +### Registering services across render modes + +New functionality must register the services it needs for **every** render mode it supports. +A feature consumed by class libraries needs a render-mode-agnostic abstraction that each +hosting environment implements. Antiforgery is the reference pattern: + +- `AntiforgeryStateProvider` - abstract, in `Web/src/Forms/` +- `DefaultAntiforgeryStateProvider` - shared default, in `Shared/src/` +- `EndpointAntiforgeryStateProvider` - SSR implementation, in `Endpoints/src/Forms/` +- registered once per host: `Endpoints` (`RazorComponentsServiceCollectionExtensions`), + `Server` (`ComponentServiceCollectionExtensions`), and `WebAssembly` + (`WebAssemblyHostBuilder`), with `AddPersistentServiceRegistration` covering `InteractiveAuto` + +Form binding follows the same shape: the `IFormValueMapper` abstraction lives in +`Web/src/Forms/Mapping/`, and `Endpoints` supplies `HttpContextFormValueMapper`. Copy this +pattern rather than inventing a new one. + +### Rendering and the synchronization context + +- **Do not call `StateHasChanged()` where `ComponentBase` already re-renders for you.** Inside + `OnInitialized[Async]`, `OnParametersSet[Async]`, and event-callback handlers, a call before + the first `await` or after the last `await` is redundant, because `ComponentBase` renders + after those anyway. This is enforced by analyzer **`BL0012`** + (`Analyzers/src/StateHasChangedAnalyzer.cs`). +- **That scope is narrow, and calls outside it are usually required.** `BL0012` deliberately + targets only those methods, so before flagging a call, check whether it is one of these + legitimate cases: + - `OnAfterRender[Async]`, which does **not** trigger an automatic re-render - see + `ComponentBase.IHandleAfterRender.OnAfterRenderAsync`, and `Web/src/Head/HeadOutlet.cs` + for a component that relies on this + - ordinary helper methods, such as those in `Web/src/Virtualization/Virtualize.cs` + - handlers for external events, such as `Web/src/Routing/NavLink.cs` reacting to + `LocationChanged` + - lambdas and local functions, which the analyzer explicitly skips because they have their + own execution timing - see `Web/src/Forms/ValidationMessage.cs` +- **Do not call `InvokeAsync` to re-enter a synchronization context you are already on.** + Framework code enters the renderer's context at well-defined dispatch points and should not + leave it on its own. +- **Exception:** marshalling an **external** callback back onto the renderer's context is + exactly what `InvokeAsync` is for. See `Authorization/src/CascadingAuthenticationState.razor`, + where `AuthenticationStateChanged` arrives off-context, and the QuickGrid components. +- `Dispatcher.InvokeAsync` in hosting infrastructure (`CircuitHost`, `EndpointHtmlRenderer`, + `WebViewManager`, `RemoteRenderer`) *is* the well-defined dispatch point, not a violation. + `EventCallback.InvokeAsync` is an unrelated API for invoking a callback - it is not affected + by this rule. + +### Components and events + +- **Do not implement `IHandleEvent` in new components.** `ComponentBase` already implements it + and provides the automatic re-render after event handlers; implementing it yourself opts out + of that behavior. Tests that implement it directly are exercising that opt-out deliberately. + +### Trimming and AOT + +Trim and AOT safety is handled with **annotations**, not by banning reflection: +`[DynamicallyAccessedMembers]`, `[RequiresUnreferencedCode]`, and `[RequiresDynamicCode]`. +Annotate new reflection-based code rather than avoiding reflection outright. Do not propose a +blanket no-reflection or source-generated-serialization-only rule - JS interop itself depends +on annotated reflection. `JsonSerializerContext` is still the right tool where it fits, such as +`Endpoints/src/Assets/ImportMapSerializerContext.cs`. + +### Dependencies + +**No new dependencies as a default.** Almost every shipping project in this area has zero +`` entries and uses `` for framework references +instead. The few exceptions are inherent to what the project is - the analyzer needs Roslyn, +and `Gateway` needs OpenTelemetry. Adding a package reference to a shipping Components project +needs an explicit justification. + +### Coordinating async work in tests + +Prefer `TaskCompletionSource` to coordinate async work; it is the dominant pattern in this +area. Avoid `Task.Delay` and `Thread.Sleep` as a way to wait for something to happen, since +that is what makes tests flaky. + +**Legitimate exceptions - do not "fix" these:** + +- E2E/Selenium polling and retry loops, which must wait on a real browser +- tests that assert on measured duration, such as the metrics tests +- analyzer test fixtures, where `await Task.Delay(1)` is the *source text being analyzed* + rather than test coordination +- deliberately simulating slow async work + ## Build Tips ### Efficient Build Strategy @@ -224,7 +344,10 @@ E2E tests are located in `src/Components/test/E2ETest`. ### Running E2E Tests -The E2E tests use Selenium. To build and run tests: +The E2E tests use Selenium. Selenium is the E2E framework for this area - do not introduce a +second one. The Playwright references in this guide are the Playwright MCP **browser tool**, +used for manual interactive validation of a sample while developing; it is not a test framework +here and is not referenced by the E2E test project. To build and run tests: ```bash # Build the E2E test project and its dependencies From 6e5366cdaea07f7f2b677707a7ac62bce95af37c Mon Sep 17 00:00:00 2001 From: PureWeen <223556219+Copilot@users.noreply.github.com> Date: Fri, 21 Aug 2026 16:02:25 -0500 Subject: [PATCH 2/2] [Blazor] Condense Components design invariants Keep the source-grounded rules while removing implementation inventories, file-path walkthroughs, and duplicated analyzer scope from the nearest-wins guidance. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/Components/AGENTS.md | 157 ++++++++++++--------------------------- 1 file changed, 47 insertions(+), 110 deletions(-) diff --git a/src/Components/AGENTS.md b/src/Components/AGENTS.md index 5a5ef0397e8f..46d71464dc9e 100644 --- a/src/Components/AGENTS.md +++ b/src/Components/AGENTS.md @@ -68,123 +68,61 @@ Together these cover every interactivity platform (Server/WebAssembly/Auto/None) ## Design invariants -These are long-standing design rules for framework code in this area. Each one lists its real -exceptions - respect those, because the exceptions are legitimate existing patterns, not -violations to clean up. +Apply these rules to framework code in this area. Before flagging code, check the current +analyzer, host-assembly boundary, and nearby pattern. Examples are anchors, not exhaustive +inventories. ### Render modes -- **Framework components must be render-mode agnostic** so they work when consumed from a - Razor Class Library. Framework components never use `@rendermode` to force a mode - it is a - choice for the app that consumes the component, not for the component itself. -- **Exception:** a deliberately small set of render-mode-specific components ships in - `Microsoft.AspNetCore.Components.Endpoints` for static SSR - `CacheView`, `BasePath`, - `ImportMap`, and `ResourcePreloader`. Keep that set small; do not add to it without a strong - reason. -- Public APIs should work in every render mode. The exception is APIs defined in a concrete - render-mode assembly (`Endpoints`, `Server`, `WebAssembly`, `WebView`), which are not - intended for class-library consumption. Anything meant to be consumed broadly belongs in an - agnostic assembly such as `Components`, `Web`, or `Forms`. - -### Accessing `HttpContext` - -- **Do not use `IHttpContextAccessor`** in framework components or framework services. It does - not exist in any render mode other than static SSR. -- Components receive it as a cascading parameter instead - note the consuming attribute is - `[CascadingParameter]`: +- Framework components must be render-mode agnostic so they work when consumed from a Razor + Class Library. The app chooses the render mode; framework components do not force one with + `@rendermode`. +- Host-specific APIs and components may live in concrete host assemblies, but that set is + deliberately small - adding to it needs a strong reason. Anything intended for broad app or + library consumption needs a render-mode-agnostic abstraction. + +### `HttpContext` and host services + +- Do not use `IHttpContextAccessor` in framework components or services. Components receive + `HttpContext` as a cascading parameter: ```csharp [CascadingParameter] public HttpContext? HttpContext { get; set; } ``` - See `Endpoints/src/Assets/ImportMap.cs` and `Endpoints/src/CacheView/CacheView.cs`. -- Services take `HttpContext` as a **method parameter**, never as a constructor dependency. - `EndpointAntiforgeryStateProvider` is the reference: it takes `IAntiforgery` in its - constructor and receives the context through `SetRequestContext(HttpContext)`, then falls - back to persisted state when there is no context. - -### Registering services across render modes - -New functionality must register the services it needs for **every** render mode it supports. -A feature consumed by class libraries needs a render-mode-agnostic abstraction that each -hosting environment implements. Antiforgery is the reference pattern: - -- `AntiforgeryStateProvider` - abstract, in `Web/src/Forms/` -- `DefaultAntiforgeryStateProvider` - shared default, in `Shared/src/` -- `EndpointAntiforgeryStateProvider` - SSR implementation, in `Endpoints/src/Forms/` -- registered once per host: `Endpoints` (`RazorComponentsServiceCollectionExtensions`), - `Server` (`ComponentServiceCollectionExtensions`), and `WebAssembly` - (`WebAssemblyHostBuilder`), with `AddPersistentServiceRegistration` covering `InteractiveAuto` - -Form binding follows the same shape: the `IFormValueMapper` abstraction lives in -`Web/src/Forms/Mapping/`, and `Endpoints` supplies `HttpContextFormValueMapper`. Copy this -pattern rather than inventing a new one. - -### Rendering and the synchronization context - -- **Do not call `StateHasChanged()` where `ComponentBase` already re-renders for you.** Inside - `OnInitialized[Async]`, `OnParametersSet[Async]`, and event-callback handlers, a call before - the first `await` or after the last `await` is redundant, because `ComponentBase` renders - after those anyway. This is enforced by analyzer **`BL0012`** - (`Analyzers/src/StateHasChangedAnalyzer.cs`). -- **That scope is narrow, and calls outside it are usually required.** `BL0012` deliberately - targets only those methods, so before flagging a call, check whether it is one of these - legitimate cases: - - `OnAfterRender[Async]`, which does **not** trigger an automatic re-render - see - `ComponentBase.IHandleAfterRender.OnAfterRenderAsync`, and `Web/src/Head/HeadOutlet.cs` - for a component that relies on this - - ordinary helper methods, such as those in `Web/src/Virtualization/Virtualize.cs` - - handlers for external events, such as `Web/src/Routing/NavLink.cs` reacting to - `LocationChanged` - - lambdas and local functions, which the analyzer explicitly skips because they have their - own execution timing - see `Web/src/Forms/ValidationMessage.cs` -- **Do not call `InvokeAsync` to re-enter a synchronization context you are already on.** - Framework code enters the renderer's context at well-defined dispatch points and should not - leave it on its own. -- **Exception:** marshalling an **external** callback back onto the renderer's context is - exactly what `InvokeAsync` is for. See `Authorization/src/CascadingAuthenticationState.razor`, - where `AuthenticationStateChanged` arrives off-context, and the QuickGrid components. -- `Dispatcher.InvokeAsync` in hosting infrastructure (`CircuitHost`, `EndpointHtmlRenderer`, - `WebViewManager`, `RemoteRenderer`) *is* the well-defined dispatch point, not a violation. - `EventCallback.InvokeAsync` is an unrelated API for invoking a callback - it is not affected - by this rule. - -### Components and events - -- **Do not implement `IHandleEvent` in new components.** `ComponentBase` already implements it - and provides the automatic re-render after event handlers; implementing it yourself opts out - of that behavior. Tests that implement it directly are exercising that opt-out deliberately. - -### Trimming and AOT - -Trim and AOT safety is handled with **annotations**, not by banning reflection: -`[DynamicallyAccessedMembers]`, `[RequiresUnreferencedCode]`, and `[RequiresDynamicCode]`. -Annotate new reflection-based code rather than avoiding reflection outright. Do not propose a -blanket no-reflection or source-generated-serialization-only rule - JS interop itself depends -on annotated reflection. `JsonSerializerContext` is still the right tool where it fits, such as -`Endpoints/src/Assets/ImportMapSerializerContext.cs`. - -### Dependencies - -**No new dependencies as a default.** Almost every shipping project in this area has zero -`` entries and uses `` for framework references -instead. The few exceptions are inherent to what the project is - the analyzer needs Roslyn, -and `Gateway` needs OpenTelemetry. Adding a package reference to a shipping Components project -needs an explicit justification. +- Services accept `HttpContext` as a method parameter, never as a constructor dependency. +- Features consumed by class libraries use a render-mode-agnostic abstraction with an + implementation and service registration for each supported host. Antiforgery is the + reference pattern. + +### Rendering and event dispatch + +- Analyzer `BL0012` is authoritative for redundant `StateHasChanged()` calls in + `OnInitialized[Async]`, `OnParametersSet[Async]`, and event-callback handlers. Calls from + after-render logic, helper methods, or external callbacks may be required; check the + analyzer's current scope before flagging them. +- Do not use `InvokeAsync` to re-enter a synchronization context that framework code is already + on. Use it to marshal external callbacks onto the renderer's context. + This rule covers `ComponentBase.InvokeAsync` only: `Dispatcher.InvokeAsync` in hosting + infrastructure is the intended dispatch point, and `EventCallback.InvokeAsync` is an + unrelated callback-invocation API. +- Do not implement `IHandleEvent` in new components. `ComponentBase` already provides automatic + rendering after event handlers. + +### Trimming and dependencies + +- Handle reflection with trim/AOT annotations such as `[DynamicallyAccessedMembers]`, + `[RequiresUnreferencedCode]`, and `[RequiresDynamicCode]`. Do not impose a blanket reflection + ban or source-generated-serialization-only rule; JS interop itself uses annotated reflection. +- Do not add package dependencies to shipping Components projects by default. Any addition + needs explicit justification against the nearby project pattern. ### Coordinating async work in tests -Prefer `TaskCompletionSource` to coordinate async work; it is the dominant pattern in this -area. Avoid `Task.Delay` and `Thread.Sleep` as a way to wait for something to happen, since -that is what makes tests flaky. - -**Legitimate exceptions - do not "fix" these:** - -- E2E/Selenium polling and retry loops, which must wait on a real browser -- tests that assert on measured duration, such as the metrics tests -- analyzer test fixtures, where `await Task.Delay(1)` is the *source text being analyzed* - rather than test coordination -- deliberately simulating slow async work +Prefer `TaskCompletionSource` to coordinate async work. Do not use `Task.Delay` or +`Thread.Sleep` to wait for state changes. Legitimate exceptions include real-browser polling, +duration assertions, analyzer fixture source text that is the input being analyzed, and +deliberate slow-work simulation. ## Build Tips @@ -344,10 +282,9 @@ E2E tests are located in `src/Components/test/E2ETest`. ### Running E2E Tests -The E2E tests use Selenium. Selenium is the E2E framework for this area - do not introduce a -second one. The Playwright references in this guide are the Playwright MCP **browser tool**, -used for manual interactive validation of a sample while developing; it is not a test framework -here and is not referenced by the E2E test project. To build and run tests: +The E2E tests use Selenium. Playwright references in this guide mean the Playwright MCP browser +tool for manual interactive validation, not the E2E framework. Adding another E2E framework +requires agreement from Components maintainers. To build and run tests: ```bash # Build the E2E test project and its dependencies