Api proposal for `NotFoundEventArgs`

[ilonatommy]

Background and Motivation

Change mostly for internal use. Router has the information about path to NotFoundPage. It has to pass this information to the Renderer. If it's not passed, we assume the event was "unhandled" and we should re-execute if the re-execution middleware is set. The most efficient way to inform the Renderer about this path is attaching it to NotFoundEventArgs.

NotFoundEventArgs is a class added together with NavigationManager.NotFound() feature. It was approved with default constructor and without any properties. This proposal is adding nullable Path with setter and getter. Both are used internally:

• setter in Router.cs - role: passing the path to the not found page + marking the event as handled;
• getter in EndpointHtmlRenderer.EventDispatch.cs - role: rendering the not found contents;
• getter in RazorComponentEndpointInvoker.cs - role: checking if the event was handled to decide if allow re-execution.

Proposed API

namespace Microsoft.AspNetCore.Components.Routing
{
    public sealed class NotFoundEventArgs
    {
-       public NotFoundEventArgs(string path);
+       public NotFoundEventArgs();
+       public string? Path { get; set; }
    }
}

Usage Examples

By desing for internal use. Customers could use the getter. Using the setter won't cause rendering the cont found contents. This might be misleading from customer's point of view, see "risks".

Alternative Designs

• Previously, NotFoundEventArgs did not provide the not-found path, so we were always using a fixed path "not-found", forcing users to define not found component with exactly this route, without a way to customize it.

Risks

• The path can be confused with unmatched path of component that called NavigationManager.NotFound(). From this reason, better naming could be used.
• The setter could be misunderstood as: customer can subscribe to NavigationManager.OnNotFound(), set the Path like and the page under this path will get rendered when I call NavigationManager.NotFound(). Yes, it will but only in specific cases and they should not rely on it.

// CustomRouter.cs
    public Task SetParametersAsync(ParameterView parameters)
    {
          navigationManager.OnNotFound += OnNotFoundEvent;
     }

    private void OnNotFoundEvent(object sender, NotFoundEventArgs e)
    {
        var type = typeof(CustomNotFoundPage);
        var routeAttributes = type.GetCustomAttributes(typeof(RouteAttribute), inherit: true);
        if (routeAttributes.Length == 0)
        {
            throw new InvalidOperationException($"The type {type.FullName} " +
                $"does not have a {typeof(RouteAttribute).FullName} applied to it.");
        }

        var routeAttribute = (RouteAttribute)routeAttributes[0];
        if (routeAttribute.Template != null)
        {
            e.Path = routeAttribute.Template;
        }
    }

Because:

• In case we have no router and the request has not started, only re-execution can cause rendering. This would not work.
• In case we have no router and the request has started, we're relying on fetching the page on client and we will be able to render it. This would work.
• In case we have blazor's router and the request has not started, we allow the Router to render only if it has NotFoundPage parameter passed. It's by design and we don't want to change it. This would not work.
• In case we have blazor's router and the request has started, we're relying on fetching the page on client and we will be able to render it. This would work.

[dotnet-policy-service]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[halter73]

API Review Notes:

• We had a long debate over whether RenderPath would be clearer than Path or if we should add both RenderPath and NotFoundPath, but decided against it.
• We also considered adding a NotFoundEventArgs.Handled(string path), but decided against it as well.

API Approved as proposed!

namespace Microsoft.AspNetCore.Components.Routing
{
    public sealed class NotFoundEventArgs
    {
-       public NotFoundEventArgs(string path);
+       public NotFoundEventArgs();
+       public string? Path { get; set; }
    }
}

[javiercn]

No changes