[Blazor] Remove obsolete APIs from Components#31
Conversation
Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
WalkthroughThis pull request removes multiple obsolete public APIs and classes across ASP.NET Core Components modules, including deprecation of Router's PreferExactMatches property, EditContext validation extension methods, RemoteBrowserFileStreamOptions class, WebEventCallbackFactoryEventArgsExtensions, SignOutSessionStateManager, and coordinated updates to WebAssembly authentication components. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/Components/WebAssembly/WebAssembly.Authentication/src/Services/AccessTokenNotAvailableException.cs`:
- Around line 38-45: The code currently passes the nullable
_tokenResult.InteractiveRequestUrl directly to _navigation.NavigateToLogin and
uses the null-forgiving operator for _navigation.NavigateTo; update
AccessTokenNotAvailableException handling to first check if
_tokenResult.InteractiveRequestUrl is null and if so throw a clear exception
(e.g., InvalidOperationException/ArgumentNullException) to fail fast, otherwise
call _navigation.NavigateToLogin(_tokenResult.InteractiveRequestUrl) when
_tokenResult.InteractionOptions != null or
_navigation.NavigateTo(_tokenResult.InteractiveRequestUrl) when
InteractionOptions is null; ensure you reference _tokenResult,
InteractionOptions, InteractiveRequestUrl, _navigation.NavigateToLogin and
_navigation.NavigateTo in the fix.
🧹 Nitpick comments (1)
src/Components/WebAssembly/WebAssembly.Authentication/src/RemoteAuthenticatorViewCore.cs (1)
290-292: Clarify the purpose ofTask.Yield()here.The comment states "Ensure we're authenticated before proceeding with sign-out", but
Task.Yield()simply yields control to the scheduler and doesn't directly ensure authentication state readiness. If this is meant to allow pending async operations (like authentication state initialization) to complete, consider adding a more descriptive comment explaining the specific timing issue this addresses.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/Components/Components/src/PublicAPI.Unshipped.txtsrc/Components/Components/src/Routing/Router.cssrc/Components/Forms/src/EditContextDataAnnotationsExtensions.cssrc/Components/Forms/src/PublicAPI.Unshipped.txtsrc/Components/Forms/test/EditContextDataAnnotationsExtensionsTest.cssrc/Components/Web/src/Forms/InputFile/RemoteBrowserFileStreamOptions.cssrc/Components/Web/src/PublicAPI.Unshipped.txtsrc/Components/Web/src/Web/WebEventCallbackFactoryEventArgsExtensions.cssrc/Components/Web/src/WebRenderer.cssrc/Components/WebAssembly/JSInterop/src/InternalCalls.cssrc/Components/WebAssembly/WebAssembly.Authentication/src/PublicAPI.Unshipped.txtsrc/Components/WebAssembly/WebAssembly.Authentication/src/RemoteAuthenticatorViewCore.cssrc/Components/WebAssembly/WebAssembly.Authentication/src/Services/AccessTokenNotAvailableException.cssrc/Components/WebAssembly/WebAssembly.Authentication/src/Services/AccessTokenResult.cssrc/Components/WebAssembly/WebAssembly.Authentication/src/Services/RemoteAuthenticationService.cssrc/Components/WebAssembly/WebAssembly.Authentication/src/Services/SignOutSessionStateManager.cssrc/Components/WebAssembly/WebAssembly.Authentication/src/WebAssemblyAuthenticationServiceCollectionExtensions.cssrc/Components/WebAssembly/WebAssembly.Authentication/test/RemoteAuthenticatorCoreTests.cssrc/Components/WebAssembly/WebAssembly/src/Infrastructure/JSInteropMethods.cssrc/Components/WebAssembly/WebAssembly/src/PublicAPI.Unshipped.txt
💤 Files with no reviewable changes (13)
- src/Components/WebAssembly/WebAssembly.Authentication/src/WebAssemblyAuthenticationServiceCollectionExtensions.cs
- src/Components/Components/src/Routing/Router.cs
- src/Components/Web/src/Forms/InputFile/RemoteBrowserFileStreamOptions.cs
- src/Components/WebAssembly/JSInterop/src/InternalCalls.cs
- src/Components/WebAssembly/WebAssembly.Authentication/src/Services/AccessTokenResult.cs
- src/Components/WebAssembly/WebAssembly.Authentication/test/RemoteAuthenticatorCoreTests.cs
- src/Components/Forms/test/EditContextDataAnnotationsExtensionsTest.cs
- src/Components/WebAssembly/WebAssembly.Authentication/src/Services/RemoteAuthenticationService.cs
- src/Components/Web/src/WebRenderer.cs
- src/Components/Forms/src/EditContextDataAnnotationsExtensions.cs
- src/Components/WebAssembly/WebAssembly/src/Infrastructure/JSInteropMethods.cs
- src/Components/Web/src/Web/WebEventCallbackFactoryEventArgsExtensions.cs
- src/Components/WebAssembly/WebAssembly.Authentication/src/Services/SignOutSessionStateManager.cs
🧰 Additional context used
🪛 LanguageTool
src/Components/Forms/src/PublicAPI.Unshipped.txt
[style] ~3-~3: Using many exclamation marks might seem excessive (in this case: 4 exclamation marks for a text that’s 475 characters long)
Context: ...text! editContext) -> System.IDisposable!
(EN_EXCESSIVE_EXCLAMATION)
🔇 Additional comments (7)
src/Components/WebAssembly/WebAssembly/src/PublicAPI.Unshipped.txt (1)
2-3: LGTM!The removal of the two-parameter
NotifyLocationChangedoverloads is consistent with the PR's objective of removing obsolete APIs. Per the summary, these are replaced with a three-parameter version that includes astateparameter.src/Components/Components/src/PublicAPI.Unshipped.txt (1)
2-4: LGTM!The removal of the
ResourceAssetconstructor overload andRouter.PreferExactMatchesproperty aligns with the PR's objective of removing obsolete APIs. These changes reduce the public API surface as intended.src/Components/WebAssembly/WebAssembly.Authentication/src/RemoteAuthenticatorViewCore.cs (2)
279-286: LGTM on the simplified sign-out validation.The removal of
SignOutSessionStateManagerand direct use ofValidateSignOutRequestState()simplifies the logout flow. The validation correctly rejects sign-out attempts that weren't initiated through proper navigation state (i.e.,GetCachedNavigationState()returnsnullor has a different interaction type).
380-383: LGTM!The
ValidateSignOutRequestState()implementation correctly validates that the sign-out request originated from within the application by checking that the cached navigation state has anInteractionType.SignOutinteraction. The null-conditional operator ensures safe handling when navigation state is absent.src/Components/WebAssembly/WebAssembly.Authentication/src/PublicAPI.Unshipped.txt (1)
2-8: LGTM!The removal of obsolete APIs is consistent with the PR objectives:
AccessTokenResultconstructor andRedirectUrlproperty removed (replaced withInteractiveRequestUrlandInteractionOptions)RemoteAuthenticationServiceconstructor overload removed (now requiresLoggerparameter)SignOutSessionStateManagerclass entirely removed, aligning with the simplified sign-out flow inRemoteAuthenticatorViewCore.cssrc/Components/Web/src/PublicAPI.Unshipped.txt (1)
2-31: LGTM!Comprehensive removal of obsolete public API surface:
RemoteBrowserFileStreamOptionsclass and all its configuration propertiesWebRenderer.RendererIdinit-only setter (now read-only)WebEventCallbackFactoryEventArgsExtensionsand all 20Createoverloads for various event args typesThese removals align with the PR's objective of cleaning up obsolete APIs across the Components modules.
src/Components/Forms/src/PublicAPI.Unshipped.txt (1)
2-3: LGTM!The removal of the obsolete
AddDataAnnotationsValidationand parameterlessEnableDataAnnotationsValidationextension methods is appropriate. The non-obsoleteEnableDataAnnotationsValidation(EditContext, IServiceProvider)overload remains available for proper dependency injection support.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| if (_tokenResult.InteractionOptions != null) | ||
| { | ||
| _navigation.NavigateToLogin(_tokenResult.InteractiveRequestUrl, _tokenResult.InteractionOptions); | ||
| } | ||
| else | ||
| { | ||
| #pragma warning disable CS0618 // Type or member is obsolete | ||
| _navigation.NavigateTo(_tokenResult.RedirectUrl!); | ||
| #pragma warning restore CS0618 // Type or member is obsolete | ||
| _navigation.NavigateTo(_tokenResult.InteractiveRequestUrl!); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify where InteractiveRequestUrl and RedirectUrl are populated and how Redirect() is used.
echo "=== AccessTokenResult class definition ==="
rg -n "class AccessTokenResult\b" -A 15 src/Components/WebAssembly/WebAssembly.Authentication
echo ""
echo "=== InteractiveRequestUrl and RedirectUrl usage ==="
rg -n "InteractiveRequestUrl|RedirectUrl" -B 2 -A 2 src/Components/WebAssembly/WebAssembly.Authentication
echo ""
echo "=== AccessTokenNotAvailableException.Redirect() method ==="
rg -n "void Redirect\(\)" -A 15 src/Components/WebAssembly/WebAssembly.AuthenticationRepository: qodo-benchmark/aspnetcore-combined-coderabbit
Length of output: 14695
🏁 Script executed:
#!/bin/bash
echo "=== Calls to Redirect() method ==="
rg -n "\.Redirect\(\)" -B 3 -A 1 src/Components/WebAssembly/WebAssembly.Authentication
echo ""
echo "=== NavigateToLogin and NavigateTo method signatures ==="
rg -n "NavigateToLogin|NavigateTo" -B 1 -A 3 src/Components/WebAssembly/WebAssembly.Authentication | head -50
echo ""
echo "=== AccessTokenNotAvailableException constructor and full context ==="
rg -n "class AccessTokenNotAvailableException" -A 30 src/Components/WebAssembly/WebAssembly.AuthenticationRepository: qodo-benchmark/aspnetcore-combined-coderabbit
Length of output: 12829
🏁 Script executed:
#!/bin/bash
echo "=== NavigateToLogin extension method definition ==="
rg -n "NavigateToLogin" -B 2 -A 5 src/Components/WebAssembly/WebAssembly.Authentication | grep -A 7 "extension\|method\|void\|public"
echo ""
echo "=== Check for invariants or guards before calling Redirect() ==="
rg -n "\.Redirect\(\)" -B 5 -A 1 --type cs src/
echo ""
echo "=== Check how AccessTokenResult is created ==="
rg -n "new AccessTokenResult" -B 2 -A 2 src/Components/WebAssembly/WebAssembly.AuthenticationRepository: qodo-benchmark/aspnetcore-combined-coderabbit
Length of output: 13537
🏁 Script executed:
#!/bin/bash
echo "=== AccessTokenResult.Status and documentation ==="
rg -n "enum AccessTokenResultStatus|class AccessTokenResult" -A 20 src/Components/WebAssembly/WebAssembly.Authentication | head -60
echo ""
echo "=== Check if there are preconditions/guards for calling Redirect() ==="
cat src/Components/WebAssembly/WebAssembly.Authentication/src/Services/AccessTokenNotAvailableException.csRepository: qodo-benchmark/aspnetcore-combined-coderabbit
Length of output: 9897
Add null checks before navigating with InteractiveRequestUrl.
Line 40 passes InteractiveRequestUrl (which is nullable string?) directly to NavigateToLogin, which expects a non-nullable string. Additionally, line 44 uses a null-forgiving operator without runtime validation. Since InteractiveRequestUrl can be null even when InteractionOptions is null, add explicit null checks or throw an exception to fail fast rather than silently navigating to a null/invalid URL.
Suggested fix
if (_tokenResult.InteractionOptions != null)
{
- _navigation.NavigateToLogin(_tokenResult.InteractiveRequestUrl, _tokenResult.InteractionOptions);
+ if (string.IsNullOrEmpty(_tokenResult.InteractiveRequestUrl))
+ {
+ throw new InvalidOperationException("Interactive request URL is required when interaction options are specified.");
+ }
+ _navigation.NavigateToLogin(_tokenResult.InteractiveRequestUrl, _tokenResult.InteractionOptions);
}
else
{
- _navigation.NavigateTo(_tokenResult.InteractiveRequestUrl!);
+ if (string.IsNullOrEmpty(_tokenResult.InteractiveRequestUrl))
+ {
+ throw new InvalidOperationException("Interactive request URL is required for token provisioning.");
+ }
+ _navigation.NavigateTo(_tokenResult.InteractiveRequestUrl);
}🤖 Prompt for AI Agents
In
`@src/Components/WebAssembly/WebAssembly.Authentication/src/Services/AccessTokenNotAvailableException.cs`
around lines 38 - 45, The code currently passes the nullable
_tokenResult.InteractiveRequestUrl directly to _navigation.NavigateToLogin and
uses the null-forgiving operator for _navigation.NavigateTo; update
AccessTokenNotAvailableException handling to first check if
_tokenResult.InteractiveRequestUrl is null and if so throw a clear exception
(e.g., InvalidOperationException/ArgumentNullException) to fail fast, otherwise
call _navigation.NavigateToLogin(_tokenResult.InteractiveRequestUrl) when
_tokenResult.InteractionOptions != null or
_navigation.NavigateTo(_tokenResult.InteractiveRequestUrl) when
InteractionOptions is null; ensure you reference _tokenResult,
InteractionOptions, InteractiveRequestUrl, _navigation.NavigateToLogin and
_navigation.NavigateTo in the fix.
Benchmark PR from agentic-review-benchmarks#6
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.