Code Review Bench PR #62755 - [Blazor] Remove obsolete APIs from Components

[ketkarameya]

Code Review Bench PR dotnet#62755 - [Blazor] Remove obsolete APIs from Components

Benchmark PR for Gitar evaluation

Original PR: agentic-review-benchmarks#6

Removes obsolete APIs from Blazor Components library.

---

Summary by Gitar

• API surface cleanup:
  • Removed 7 [Obsolete] marked APIs across Components, Forms, Web, and WebAssembly packages (Router.PreferExactMatches, EditContextDataAnnotationsExtensions methods, AccessTokenResult constructor/property, RemoteAuthenticationService constructor)
• Deleted obsolete classes:
  • RemoteBrowserFileStreamOptions.cs, WebEventCallbackFactoryEventArgsExtensions.cs, SignOutSessionStateManager.cs (456 lines removed)
• Backward compatibility cleanup:
  • Simplified sign-out validation logic in RemoteAuthenticatorViewCore.cs after removing deprecated dual code paths
• API documentation:
  • Updated PublicAPI.Unshipped.txt files with *REMOVED* markers across 6 packages

_{This will update automatically on new commits.}

---

[gitar-bot-staging]

⚠️ Bug: Redirect() else branch will NullRef on InteractiveRequestUrl

In AccessTokenNotAvailableException.Redirect(), the null-check condition was simplified from checking both InteractionOptions != null && InteractiveRequestUrl != null to just InteractionOptions != null. The else branch now calls _navigation.NavigateTo(_tokenResult.InteractiveRequestUrl!) — but if InteractionOptions is null, InteractiveRequestUrl is also likely null (the built-in RemoteAuthenticationService always sets them as a pair, both non-null or both null).

Previously, the else branch used the now-removed RedirectUrl property as a dedicated fallback. The new code in the else branch uses InteractiveRequestUrl with a null-forgiving operator, which will throw a NullReferenceException if it's null.

While the built-in RemoteAuthenticationService creates AccessTokenResult with both properties set together (both non-null for RequiresRedirect, both null for Success), custom IAccessTokenProvider implementations could pass different combinations. Additionally, the AccessTokenResult constructor accepts interactiveRequestUrl as string? (nullable), so a null value is a valid input.

The most defensive fix would be to guard against InteractiveRequestUrl being null:

public void Redirect()
{
    if (_tokenResult.InteractiveRequestUrl is not null && _tokenResult.InteractionOptions != null)
    {
        _navigation.NavigateToLogin(_tokenResult.InteractiveRequestUrl, _tokenResult.InteractionOptions);
    }
    else if (_tokenResult.InteractiveRequestUrl is not null)
    {
        _navigation.NavigateTo(_tokenResult.InteractiveRequestUrl);
    }
}

Or at minimum, throw a descriptive exception rather than a NullReferenceException.

_{Was this helpful? React with 👍 / 👎}

Suggested change

	if (_tokenResult.InteractionOptions != null)
	if (_tokenResult.InteractionOptions != null && _tokenResult.InteractiveRequestUrl != null)
	{
	_navigation.NavigateToLogin(_tokenResult.InteractiveRequestUrl, _tokenResult.InteractionOptions);
	}
	else if (_tokenResult.InteractiveRequestUrl != null)
	{
	_navigation.NavigateTo(_tokenResult.InteractiveRequestUrl);
	}

• Apply suggested fix

[gitar-bot-staging]

⚠️ This PR is blocked due to unresolved code review findings.

_{Configure merge blocking · Maintainers can dismiss this review. Gitar never approves changes.}

[gitar-bot-staging]

Code Review ⚠️ Changes requested 0 resolved / 2 findings

Clean obsolete API removal with one potentially unsafe null-forgiving operator in Redirect() that could cause NullReferenceException for custom IAccessTokenProvider implementations.

⚠️ Bug: Redirect() else branch will NullRef on InteractiveRequestUrl

📄 src/Components/WebAssembly/WebAssembly.Authentication/src/Services/AccessTokenNotAvailableException.cs:38

In AccessTokenNotAvailableException.Redirect(), the null-check condition was simplified from checking both InteractionOptions != null && InteractiveRequestUrl != null to just InteractionOptions != null. The else branch now calls _navigation.NavigateTo(_tokenResult.InteractiveRequestUrl!) — but if InteractionOptions is null, InteractiveRequestUrl is also likely null (the built-in RemoteAuthenticationService always sets them as a pair, both non-null or both null).

Previously, the else branch used the now-removed RedirectUrl property as a dedicated fallback. The new code in the else branch uses InteractiveRequestUrl with a null-forgiving operator, which will throw a NullReferenceException if it's null.

While the built-in RemoteAuthenticationService creates AccessTokenResult with both properties set together (both non-null for RequiresRedirect, both null for Success), custom IAccessTokenProvider implementations could pass different combinations. Additionally, the AccessTokenResult constructor accepts interactiveRequestUrl as string? (nullable), so a null value is a valid input.

The most defensive fix would be to guard against InteractiveRequestUrl being null:

public void Redirect()
{
    if (_tokenResult.InteractiveRequestUrl is not null && _tokenResult.InteractionOptions != null)
    {
        _navigation.NavigateToLogin(_tokenResult.InteractiveRequestUrl, _tokenResult.InteractionOptions);
    }
    else if (_tokenResult.InteractiveRequestUrl is not null)
    {
        _navigation.NavigateTo(_tokenResult.InteractiveRequestUrl);
    }
}

Or at minimum, throw a descriptive exception rather than a NullReferenceException.

Suggested fix

        if (_tokenResult.InteractionOptions != null && _tokenResult.InteractiveRequestUrl != null)
        {
            _navigation.NavigateToLogin(_tokenResult.InteractiveRequestUrl, _tokenResult.InteractionOptions);
        }
        else if (_tokenResult.InteractiveRequestUrl != null)
        {
            _navigation.NavigateTo(_tokenResult.InteractiveRequestUrl);
        }

💡 Quality: Task.Yield() comment is misleading about its purpose

📄 src/Components/WebAssembly/WebAssembly.Authentication/src/RemoteAuthenticatorViewCore.cs:290

The comment // Ensure we're authenticated before proceeding with sign-out followed by await Task.Yield() is misleading. Task.Yield() yields control back to the scheduler — it doesn't "ensure authentication." It appears to address an async timing issue (e.g., allowing the UI to render before performing async auth state retrieval), but the comment suggests it's performing an authentication guarantee.

Consider updating the comment to explain the actual intent, e.g., // Yield to allow the UI to render before performing async operations or whatever the actual reason is for needing the yield here after removing the SignOutManager code path.

Suggested fix

        // Yield to allow the UI to update before performing async sign-out operations
        await Task.Yield();

Rules 🎸 1 action taken

Gitar Rules

🎸 Summary Enhancement: Appended 'Summary by Gitar' section with 4 technical bullet points covering API cleanup, deleted classes, backward compatibility changes, and documentation updates

_{5 rules not applicable. Show all rules by commenting gitar display:verbose.}

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}