[RFC] Definition API -> Builder pattern?

[Vorlias]

I feel like the definitions API could be cleaner, perhaps instead of doing something like:

    [InventoryClientFunctionId.MoveEquippedItemToInventorySlot]: ServerAsyncFunction<
        (
            request: MoveEquippedItemToInventorySlotRequest,
        ) => Serializer.NetworkResult<MoveEquippedItemToInventorySlotResponse, MoveEquippedItemToInventorySlotError>
    >([
        Net.Middleware.TypeChecking(
            t.interface({
                InventoryId: t.numberMin(0),
                SlotId: t.numberMin(0),
                EquipSlot: t.string,
            }),
        ),
    ]),

We could do

    [InventoryClientFunctionId.MoveEquippedItemToInventorySlot]: ServerAsyncFunction<
        (
            request: MoveEquippedItemToInventorySlotRequest,
        ) => Serializer.NetworkResult<MoveEquippedItemToInventorySlotResponse, MoveEquippedItemToInventorySlotError>
    >().WithMiddleware([
        Net.Middleware.TypeChecking(
            t.interface({
                InventoryId: t.numberMin(0),
                SlotId: t.numberMin(0),
                EquipSlot: t.string,
            }),
        ),
    ]),

Could simplify this further for built-in middleware, such as .WithTypeChecking(...types) and .WithRateLimit(options)

    [InventoryClientFunctionId.MoveEquippedItemToInventorySlot]: ServerAsyncFunction<
        (
            request: MoveEquippedItemToInventorySlotRequest,
        ) => Serializer.NetworkResult<MoveEquippedItemToInventorySlotResponse, MoveEquippedItemToInventorySlotError>
    >().WithTypeChecking(
        t.interface({
            InventoryId: t.numberMin(0),
            SlotId: t.numberMin(0),
            EquipSlot: t.string,
        }),
    ),

This could play well with https://github.com/roblox-aurora/rbx-net/issues/62.

[OverHash]

This looks a lot easier to read, doesn't require referring to documentation/IntelliSense to understand the order of parameters.

Would the .With* calls be chainable? (i.e., could you do .WithMiddleware([...]).WithTypeChecking([...])? How is the order of that resolved at call-time, from start to finish in the chain (i.e., the WithMiddleware is called first, and then the WithTypeChecking calls)?

[Vorlias]

This looks a lot easier to read, doesn't require referring to documentation/IntelliSense to understand the order of parameters.

Would the .With* calls be chainable? (i.e., could you do .WithMiddleware([...]).WithTypeChecking([...])? How is the order of that resolved at call-time, from start to finish in the chain (i.e., the WithMiddleware is called first, and then the WithTypeChecking calls)?

Yeah they'd be chainable. Order wouldn't necessarily matter as it'd just add to the middleware array - the calls would just modify the resulting object it uses anyway for the definition (like your typical builder)

[OverHash]

Sounds good to me!

[Vorlias]

Alrighty have a loose idea I'm playing around with, but being backwards-compat:

        TestV4ServerFunction: Net.Definitions.AsyncFunction()
            .EnsureArguments<[value: string]>(t.string)
            .EnsureReturns(t.string)
            .WithRateLimit({
                MaxRequestsPerMinute: 10,
            })
            .WithCallbackMiddleware((procNext, instance) => {
                return (player, name) => {
                    if (someCondition) {
                        return procNext(player, name);
                    } else {
                        return NetMiddleware.Skip;
                    }
                };
            })
            .ForServer(),

Essentially will have ForServer and ForClient being the context-specific "build" functions. Note also NetMiddleware.Skip for skipping middleware processing further.

I ideally do want to make type checks mandatory but it's less backwards compatible and awkward with the async function.

[Vorlias]

Net.Definitions.Create() without the arguments will use a builder:

const BuilderRemotes = Net.Definitions.Create()
    // Create a namespacae using an object model
    .AddNamespace("ExampleNamespaceObject", {
        ExampleEvent: Net.Definitions.Event().EnsureArguments(t.string).OnServer(), // Event for server taht takes
    })
    // Create a namespace using another builder + object model
    .AddNamespace(
        "ExampleNamespaceUsingInnerBuilder",
        Net.Definitions.Create().Add({
            // Example object-like
            ExampleAsyncFunction: Net.Definitions.AsyncFunction().OnClient(),
        }),
    )
    // Create a namespace with a builder functionally
    .AddNamespace(
        "ExampleNamespaceUsingFunctionalBuilder",
        builder => {
            // You can add remotes one by one functionally like this
            return builder
                .AddClientRemote("ClientRemoteName", Net.Definitions.AsyncFunction())
                .AddServerRemote("ServerRemoteName", Net.Definitions.Event());
        }, // add a client remote using a function
    )
    // Of course like our inner builder, we can use 'Add' here.
    .Add({
        ExampleBaseRemote: Net.Definitions.AsyncFunction().OnServer(),
        // Can add a created definition as a namespace to an object model
        ToNamespaceBehaviour: Net.Definitions.Create().ToNamespace(),
    })
    .AddServerRemote("NamedBaseServerRemoteFunction", Net.Definitions.AsyncFunction())
    .AddClientRemote("NamedBaseClientRemoteEvent", Net.Definitions.Event())
    .Build();

I'm also considering AddServerOwned/AddClientOwned instead of or in addition to Add - where it will handle the internal OnServer/OnClient for you.

[Vorlias]

:eyes:

It's a bit more explicit than the following:-

However this should ensure types are checked etc.

[Vorlias]

Simplified a bit :-)

[Vorlias]

Lol 😆