Skip to content

Make HybridWebViewHandler throw JS exceptions in C# by default #28086

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

Merged
merged 2 commits into from
Mar 11, 2025

Conversation

mattleibow
Copy link
Member

Description of Change

We could not make exceptions by default in .NET 9 as that would be breaking, but it really is the most sensible.

If there is an exception in .NET 9, then the method never returns.

@Copilot Copilot AI review requested due to automatic review settings February 27, 2025 19:31
@mattleibow mattleibow requested a review from a team as a code owner February 27, 2025 19:31
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This PR updates the logic for determining whether HybridWebViewHandler should throw JavaScript exceptions in C# by default, aligning with the intended behavior for upcoming .NET releases.

  • Inverts the default behavior by returning true when the AppContext switch is not set.
  • Updates the expression to ensure exceptions are thrown by default in .NET 10 while preserving the ability to override.

Reviewed Changes

File Description
src/Core/src/Handlers/HybridWebView/HybridWebViewHandler.cs Inverts the default value for throwing JS exceptions to true, updating the logic accordingly.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/Core/src/Handlers/HybridWebView/HybridWebViewHandler.cs:98

  • The updated logic inverts the default behavior so that exceptions are thrown by default when the switch is not set. Update the accompanying comment (currently mentioning .NET 10) to clearly describe the new default behavior, and ensure that relevant test cases in TestCases.HostApp and TestCases.Shared.Tests are added to validate this behavior.
private static bool IsInvokeJavaScriptThrowsExceptionsEnabled =>
			!AppContext.TryGetSwitch(InvokeJavaScriptThrowsExceptionsSwitch, out var enabled) || enabled;

@mattleibow mattleibow requested a review from Copilot February 27, 2025 19:32
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This PR updates the HybridWebViewHandler to throw JavaScript exceptions in C# by default, while also updating the test configuration to remove redundant explicit switch settings.

  • Changed the default behavior in HybridWebViewHandler.cs to enable JS exception throwing by default.
  • Removed explicit switch setting from MauiProgram.cs in DeviceTests.

Reviewed Changes

File Description
src/Core/src/Handlers/HybridWebView/HybridWebViewHandler.cs Updated logic to default to throwing JS exceptions.
src/Controls/tests/DeviceTests/MauiProgram.cs Removed explicit AppContext switch setting for tests.

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/Core/src/Handlers/HybridWebView/HybridWebViewHandler.cs:97

  • The TODO comment is now outdated since the default behavior has been updated to throw JS exceptions. Please update or remove the comment to reflect the current implementation.
// TODO: .NET 10 flip the default to true for .NET 10

@rmarinho rmarinho added this to the .NET 10.0-preview3 milestone Mar 10, 2025
@rmarinho rmarinho self-assigned this Mar 10, 2025
@rmarinho rmarinho moved this from Todo to Ready To Review in MAUI SDK Ongoing Mar 10, 2025
@rmarinho rmarinho moved this from Ready To Review to Approved in MAUI SDK Ongoing Mar 10, 2025
@rmarinho rmarinho merged commit 3925dee into net10.0 Mar 11, 2025
116 of 123 checks passed
@rmarinho rmarinho deleted the mattleibow-patch-2 branch March 11, 2025 11:48
@github-project-automation github-project-automation bot moved this from Approved to Done in MAUI SDK Ongoing Mar 11, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants