Skip to content

BLAZOR API review #4050

New issue
New issue
Closed
Closed
BLAZOR API review#4050
Assignees
rynowak
Labels
DoneThis issue has been fixedarea-blazorIncludes: Blazor, Razor Componentstask
Milestone
 
3.0.0-preview9
@danroth27

Description

@danroth27
danroth27
opened on Nov 16, 2018

Edit: @rynowak hijacking top post for great justice

Framing Blazor API reviews

I'm suggesting that we bucket API review work into areas based on the user-impact and expected usage of APIs, and from there prioritize and time-box some of the decision making. I'm hoping to use this document to paint a high-level picture rather than "do the work" for everything at once.

I don't think it's super reasonable to have us sit in a room and "do a pass" given how many of these APIs we've built up over time - the discussion would quickly get unfocused. I want everyone's input and involvement in making the first release of Blazor awesome - so I'm going to suggest that we do initial reviews and discussions on github or email and then have meetings as-needed.

Here's a quick shout-out for all of the ideas that were explicitly mentioned in the github issue.

  • Review all of our startup API for both client-side and server-side
  • Startup Code (various suggestions)
  • Mark all attributes sealed (and review AttributeUsage)
  • Develop a style-guide for components and make the built-in ones conform
  • Figure out what to do with our HttpClient extension methods
  • Do we want to develop a naming convention for types that are intended for JS interop only (like is commonly done with P/Invoke)

It's been a long time since some of us started work on Blazor (over a year and a half for Steve). We should look at the details of everything important with fresh eyes, and make sure it's as good as it can be. We've gone up until this point doing the best we can right now, but we're about to cross the threshold where all of these decisions become permanent. It's scary! But at least we're doing it together.

What are we doing?

I think there are probably two beneficiaries of an API review - us, and unsurprisingly our users. While it might seem like we want different things, I think we usually want the same things in this area. If we can craft APIs with good ergonomics, it's easy to talk about with customers, or write documentation for. If we create good APIs it's easy to scale them up as we work on the framework.

  • Simplicity (few namespaces, overloads you want are easy to find, avoid tricky patterns)
  • Consistency (predictability, portability)
  • Horizontal Scalability (API and overloads can grow as functionality is added)

Often this means that we need to do hard work so that users have a good experience. If you've worked on MVC for a while I'm sure you can think of a few examples where we wish we did better 😉.

Areas of focus

Areas are in rough order of priority and attempt to call out of some of things that are important to review and decide. Please feel free to suggest your own ideas or disagreements - but keep in mind that we will have to time-box this work (both reviewing and changing).

I'm not attempting to make a list of everything that has to be reviewed.

Here's a rough categorization and catalog of all of the APIs that exist as of a week ago.

https://microsoft.sharepoint.com/:x:/t/DotNetTeam/ERLn4oNI3ZNHvyqRdklMoBUBKTxzD4jOCbmEO0u50p-zqA?e=4vdrl1

Use: \\fxcore\tools\apps\ApiReviewer to generate a spreadsheet like this.

Work Items

Packages/assemblies

There was a proposal circulated via email to try and align and packages around a single name, as well as clean up some unclarities.

image

The immediate action item here is to rename M.A.C.Browser and M.A.C.Browser.JS.

  • Rename M.A.C.Browser -> M.A.C.Web
  • Rename M.A.C.Browser.JS -> M.A.C.Web.JS

ComponentBase and related types

  • ComponentBase
  • IComponent
  • IHandleEvent and other lifecycle APIs
  • RenderHandle
  • ParameterAttribute
  • CascadingParameterAttribute
  • IComponentContext
  • InjectAttribute
  • RouteAttribute

What we're looking for here is the consistency, naming and predictability of the APIs on ComponentBase.

Things I'm personally interested in:

  • Do we have too many lifecycle events?
  • Do we anticipate adding more over time?
  • Do we have fragile-base-class extensibility points?

I'm concerned about the naming of things like InjectAttribute and RouteAttribute - we're parking these names in the components namespace, without an attempt to reconcile them with existing or future types.

Built-in components

  • InputText and other form types
  • EditForm
  • AuthorizeDisplay
  • Router
  • NavLink

We may choose to break some of these off into their own discussions depending on the size.

On my mind here is coming up with a vocabulary, or a style-guide for component naming and trying to apply it to these and see if it fits.

Startup experience

The startup experience includes everything Blazor adds to the Startup.cs file:

  • Extensions on IServiceCollection
  • Extensions on IEndpointRouteBuilder

I'm leaving aside client-side Blazor for 3.0 which has a lot more startup code, but doesn't need to be locked down.

What we're looking for here is consistency with the rest of the stack and horizontal scalability. Do we expose the right options? Do we expose details of underlying components like SignalR where important?

JS Interface

The blazor.*.js files define an API for use in bootstrapping Blazor applications, as well as configuring features like reconnection. We need to do a full pass over this since this API has grown organically.

Top-level features

Authentication

  • AuthenticationState
  • AuthenticationStateProvider
  • AuthenticationStateChangedHandler

These APIs are relatively recent, and seem pretty polished.

Parameter Infrastructure

  • Parameter
  • ParameterCollection
  • ParameterEnumerator
  • ParameterCollectionExtensions

I think the design of these types and naming of Parameter needs a review. ParameterCollectionExtensions probably has no reason to be an extension method.

HttpClient extensions

  • HttpClientJsonExtensions

Can we ship this in a Blazor-specific (non-ASP.NET Core Shared Framework assembly)? I have serious concerns about us shipping HttpClient extensibility in the components assembly, and addressing those concerns will break our present our future.

IUriHelper and navigation

  • IUriHelper
  • UriHelperBase
  • NavigationException

I'm concerned about the design of this API surface, the naming and its future maintainability. This should probably be an abstract class (or broken up into separate concerns). The naming is too similar to IUrlHelper in MVC, and the naming feels outdated (referential of helpers in Rails).

  • INavigationInterception
  • LocationChangedEventArgs
  • NavLinkMatch

These types are in Microsoft.AspNetCore.Components.Routing and probably don't belong there unless they are oriented around something more central. This is a namespace with few types.

ElementRef

  • ElementRef

Should this be renamed to ElementReference? More formal. Should this be in Microsoft.AspNetCore.Components.Browser?

Forms

Everything in Microsoft.AspNetCore.Components.Forms - this is a top level API for extensibility and integration with forms, we need to review all of this.

  • EditContext
  • EditContextDataAnnotationsExtensions
  • EditContextExpressionExtensions
  • EditContextFieldClassExtensions
  • FieldChangedEventArgs
  • FieldIdentifier
  • ValidationMessageStore
  • ValidationMessageStoreExpressionExtensions
  • ValidationRequestedEventArgs
  • ValidationStateChangedEventArgs

Layouts

  • LayoutAttribute
  • LayoutComponentBase

EventCallback and Event Handler types

  • EventCallback
  • UIEventArgs
  • UIMouseEventArgs and other browser/DOM specific APIs like DataTransfer
  • EventCallbackFactory and extension methods

Should UIMouseEventArgs and other DOM-related types life in Microsoft.AspNetCore.Components.Browser?

We should review the EventCallbackFactory extensions and make sure they aren't too gross.

Public infrastructure

These are infrastructure-ish things that need to be public, either to preserve layering/extensibility or to because they are useful to users.

RenderTreeBuilder

The RenderTreeBuilder needs to be public so it can be targeted by the compiler. It's also useful for really advanced component authors that need to author in C# for any reason.

We should do a pass over these APIs for consistency and naming, but I think largely this is pretty good.

We should consider whether we can make GetFrames() internal.

The namespace Microsoft.AspNetCore.Components.RenderTree probably is too specific. This should be in Microsoft.AspNetCore.Components.Rendering.

IDispatcher

The IDispatcher is hiding out here in the .Rendering namespace. We should consider moving it somewhere more top-level unless we fill up .Rendering with other stuff. I'm making this suggestion because IDispatcher doesn't touch any of our other APIs in this namespace.

Compiler infrastructure

There's a variety of types that are in the top-level namespace that should almost certainly not be featured so prominently. These are implementation details of what the compiler does, and usually don't make sense for a user to call. We can't really remove anything from these names, so they will likely become more mess over time. We should put all of these types in a dedicated namespace so they are hidden.

  • BindAttributes
  • BindElementAttribute
  • BindInputElementAttribute
  • BindMethods
  • EventHandlerAttribute
  • EventHandlers
  • RuntimeHelpers

Implementation Details

I'm really concerned about the number and kinds of things that are public. We haven't done a pass yet and trying to hide implementation details yet, but I think it's warranted.

The big risk here is the Renderer, RenderBatch and everything else in .Rendering (besides IDispatcher). We could definitely tell people not to use these types, but the reality is that won't stop them. We need to preserve our ability to work on the render tree format. I don't think we have a critical scenario in 3.0 where anyone but us writes a rendering engine.

Appendix A: Links to reference sources

Special mention: no ref assembly for .js Browser.JS

Appendix B: Totally ignorable

  • Microsoft.AspNetCore.Blazor.Build has no APIs
  • Microsoft.AspNetCore.Blazor.DevServer has no APIs
  • Microsoft.AspNetCore.Blazor.Templates has no APIs
  • Microsoft.VisualStudio.BlazorExtension has no APIs

Metadata

Metadata

Assignees

Labels

DoneThis issue has been fixedarea-blazorIncludes: Blazor, Razor Componentstask

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions