Skip to content

Blazor API Review: Public JS APIs #12229

Closed
@SteveSandersonMS

Description

@SteveSandersonMS

Our JavaScript-side APIs are much less clearly organized than the .NET-side ones, which is largely because the public API surface is so tiny compared to everything we expose in .NET.

We have a lot more flexibility to change these in back-compatible ways in the future, since by nature JavaScript doesn't require method signatures to line up at compile-time (and we're not publishing TypeScript definitions). It's possible to vary the behavior at runtime in pretty arbitrary ways, so if we wanted to make small changes in the future it's likely we still could.

JS interop

Here's Microsoft.JSInterop.d.ts, excluding comments:

declare module DotNet {
    type JsonReviver = ((key: any, value: any) => any);
    function attachDispatcher(dispatcher: DotNetCallDispatcher): void;
    function attachReviver(reviver: JsonReviver): void;
    function invokeMethod<T>(assemblyName: string, methodIdentifier: string, ...args: any[]): T;
    function invokeMethodAsync<T>(assemblyName: string, methodIdentifier: string, ...args: any[]): Promise<T>;
    interface DotNetCallDispatcher {
        invokeDotNetFromJS?(assemblyName: string | null, methodIdentifier: string, dotNetObjectId: number | null, argsJson: string): string | null;
        beginInvokeDotNetFromJS(callId: number, assemblyName: string | null, methodIdentifier: string, dotNetObjectId: number | null, argsJson: string): void;
    }
    const jsCallDispatcher: {
        findJSFunction: typeof findJSFunction;
        invokeJSFromDotNet: (identifier: string, argsJson: string) => string | null;
        beginInvokeJSFromDotNet: (asyncHandle: number, identifier: string, argsJson: string) => void;
        endInvokeDotNetFromJS: (asyncCallId: string, success: boolean, resultOrExceptionMessage: any) => void;
    };
    function findJSFunction(identifier: string): Function;
}

This is not actually the final version of the API shape, because @javiercn is tweaking this as part of improving the logging for JS interop.

However, (1) I'm quite happy with the existing shape of it - we've spent time polishing it already, and (2) the changes that @javiercn are making are very well reasoned and don't give me reason to fear that we'll end up with bad APIs.

blazor.server.js

It isn't meaningful to talk about a .d.ts file for this, because we're not publishing the package to NPM (at least, not currently). Instead the only public API surface is whatever we attach to window at runtime, which is:

window.Blazor = {
    "navigateTo": (uri: string, forceLoad: boolean) => void,
    "_internal": {
        various things, but they are pubternal
    },
    "start": (userOptions?: Partial<BlazorOptions>) => Promise<void>,
    "circuitHandlers": CircuitHandler[],
    "reconnect": (existingConnection?: signalR.HubConnection): Promise<boolean>
}

Opinion: This looks like a total mess, but there isn't much here so we're mostly OK. The only real public API surface is Blazor.navigateTo (already good), Blazor.start(options?) (already good), Blazor.circuitHandlers (pretty weird - who's going to use this extensibility anyway?), and Blazor.reconnect (fine except that we should remove the option to pass existingConnection as part of removing stateful prerendering).

Also, circuitHandlers by default contains an instance of AutoReconnectCircuitHandler, which exposes:

"logger": {
    "minimumLogLevel": LogLevel
},
"reconnectDisplay": {
    "document": Document,
    "addedToDom": boolean,
    "modal": HTMLDivElement,
    "message": HTMLHeadingElement,
    "button": HTMLButtonElement
}

Opinion: I don't really know why any of this is public. What would you do with any of it?

Also AutoReconnectCircuitHandler.constructor exposes static properties:

public static readonly MaxRetries = 5;
public static readonly RetryInterval = 3000;
public static readonly DialogId = 'components-reconnect-modal';

Opinion: Rather than random static properties, these should be options on BlazorOptions.

also with:

interface BlazorOptions {
  configureSignalR: SignalRBuilder;
  logLevel: LogLevel;
}
type SignalRBuilder = (builder: signalR.HubConnectionBuilder) => void;
export enum LogLevel {
  Trace = 0,
  Debug = 1,
  Information = 2,
  Warning = 3,
  Error = 4,
  Critical = 5,
  None = 6,
}
export interface CircuitHandler {
  onConnectionUp?(): void;
  onConnectionDown?(error?: Error): void;
}

Opinion: Mostly fine, except I'm unconvinced by the CircuitHandler extensibility point. It's unclear to me why we have a mutable array of them. It feels like we're trying to apply server-side ASP.NET Core patterns here when something simpler and more low-level would do just fine. For example, we could change BlazorOptions to be something like this:

interface BlazorOptions {
  configureSignalR: SignalRBuilder;
  logLevel: LogLevel;
  reconnectionOptions: {
    maxRetries: 5,
    retryIntervalMilliseconds: 3000,
    dialogId: 'components-reconnect-modal'
  },
  reconnectionHandler: {
    onConnectionUp?(options: BlazorOptions): void;
    onConnectionDown?(options: BlazorOptions, error?: Error): void;
  }
}

The behavior would be: if you haven't specified reconnectionHandler, then we use the built-in default one, which shows the dialog etc. and respects any values you configured for maxRetries etc. And if you have specified reconnectionHandler then we just call that, passing the reconnectionOptions to it, and it can do whatever it more general thing wants.

If someone wants to create a "composite reconnection handler", they can build one that passes through calls to multiple other handlers. We can also expose Blazor.defaultReconnectionHandler in case you want to keep using the built-in one (e.g., if you're just adding logging).

Metadata

Metadata

Labels

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

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions