Feature request: call a procedure by passing its contract for better TS performance

[jedwards1211]

Describe the feature

I’ve been pretty happy with my new oRPC app and really appreciating your hard work on this framework! But TS performance with intellisense, autoimports and rechecking is pretty bad in my project. I think a lot of this is due to the app router being one giant type that requires TS to do a bunch of inference across many files to construct. Having my output types coming from drizzle probably makes this a lot worse, I do need to see if declaring explicit output schemas fixes this.

I looked at contract routing but I was disappointed that the client API still seems to rely on one big app router type in that case.

If we could specify the full url path on each contract, then we could make a request on the client by passing in just a contract and input data, instead of relying on a recursive proxy to build the url path from the root of a big app router type. Then I think performance would be way better, because each client module would only need to import the contracts it uses and TS would only have to extract the input, output and error types from those contracts.

Some people might not prefer the developer experience of needing to figure out which contract to import instead of having all the methods right there on one type. But I think the performance benefits would be worth it. Plus, if we can specify the URL path on each contract, it would make it possible to support URL path parameters for a conventional REST API.

Additional information

• Would you be willing to help implement this feature?

[unnoq]

Good idea, but there's currently a limitation in oRPC: a contract procedure doesn’t always have a .route.path. If the user doesn't define .route.path, a standalone contract procedure can't be used for routing.

What about if throw on call a procedure contract missing .route.path?

[jedwards1211]

Oh it can have a path, nice. Yeah throwing if it doesn't sounds fine to me

[jedwards1211]

Should I start working on a PR or are you not sure about it yet?

Another advantage I see to this approach is my that server context type (which includes the big drizzle schema) wouldn't affect client typechecking at all.

[jedwards1211]

Hmmm, looking at the code it looks like it's already possible to ContractRouterClient on a single ContractProcedure even though I haven't seen any examples like that in the docs. I guess creating a separate client for each procedure might work for HTTP methods, but for subscriptions I would want to be able to share a single client, so that it only makes one websocket connection. (Unless it's possible to attach multiple clients to the same websocket link? But that already feels messy...) This is why I'm imagining having a single client instance and passing the desired contract when calling a procedure.

[unnoq]

Should I start working on a PR or are you not sure about it yet?

Feel free, but I will not merge if it not good enough

but for subscriptions I would want to be able to share a single client, so that it only makes one websocket connection

I think it is not possible without create a different link that support dynamic contract (and I don't think you should because I don't want mess current src just for this simple usecase or we can think about a different architecture that we can change it in v2)

[unnoq]

I think it most optimized if we can make current link accept dynamic contract and resue it for each calls, something like:

const link = ...

const call = createContractCall(link)

const output = call(contract, 'input', { context: 'CLIENT_CONTEXT', ... })

[jedwards1211]

Yes that's exactly what I'm thinking :)

[unnoq]

I think we can change this property to
Value<AnyContractRouter, [options: ClientOptions<T>, path: readonly string[], input: unknown]>, similar to how url works.

With that change, the final API could look like this:

const link = new OpenAPILink(inferContractFromClientContext, { url: ... })

const call = createContractCall(link)

const output = call(contract, 'input', { context: 'CLIENT_CONTEXT', ... })

When createContractCall is called, it would automatically inject something like [inferContractFromClientContext_SYMBOL]: contract into client context

[jedwards1211]

I'd like a more typesafe and convient way to get the ContractProcedure being called within userland baseUrl and headers functions, StandardOpenapiLinkCodec, DynamicLink, interceptors, etc. than a symbol on the client context.

I think it would work to put the ContractProcedure in ClientOptions instead and add a generic type parameter for ClientOptions.

For example

export interface ClientLink<
  TClientContext extends ClientContext,
  TClientOptions extends ClientOptions<TClientContext> = ClientOptions<TClientContext>
> {
  call: (path: readonly string[], input: unknown, options: TClientOptions) => Promise<unknown>
}

export interface StandardOpenapiLinkCodecOptions<
  TClientContext extends ClientContext,
  TClientOptions extends ClientOptions<TClientContext> = ClientOptions<TClientContext>
> {
  /**
   * Base url for all requests.
   */
  url: Value<Promisable<string | URL>, [
        options: TClientOptions,
        path: readonly string[],
        input: unknown,
  ]>

  ...
}

export class StandardOpenapiLinkCodec<
  TClientContext extends ClientContext,
  TClientOptions extends ClientOptions<TClientContext> = ClientOptions<TClientContext>
> implements StandardLinkCodec<TClientContext, TClientOptions> {
  private readonly baseUrl: Exclude<StandardOpenapiLinkCodecOptions<TClientContext, TClientOptions>['url'], undefined>
  ...

  constructor(
    private readonly contract: Value<AnyContractRouter, [options: TClientOptions, path: readonly string[], input: unknown]>,
    ...
  ) {
    ...
  }
}

Then createContractCall would be something like

export type ContractCallClientOptions<TContext> = ClientOptions<TContext> & {
  contractProcedure: AnyContractProcedure,
}

export function createContractCall<
  TClientContext extends ClientContext,
  TClientOptions extends ContractCallClientOptions<TClientContext> = ContractCallClientOptions<TClientContext>
>(
  link: ClientLink<TClientContext, TClientOptions>
) {
  return async function callContract<TContract extends AnyContractProcedure>(
    contractProcedure: TContract,
    input: InferContractRouterInputs<TContract>,
    options = {} as FriendlyClientOptions<TClientContext>
  ): ClientPromiseResult<
    InferContractRouterOutputs<TContract>,
    ErrorFromErrorMap<InferContractRouterErrorMap<TContract>>
  > {
    return await link.call(
      path,
      input,
      resolveFriendlyClientOptions({ ...options, contractProcedure })
    )
  }
}

We could make a StandardOpenAPIContractCallLink that provides the contract value function implementation:

export class StandardOpenAPIContractCallLink<
  TClientContext extends ClientContext,
  TClientOptions extends ContractCallClientOptions<TClientContext> = ContractCallClientOptions<TClientContext>
> extends StandardLink<TClientContext, TClientOptions> {
  constructor(
    linkClient: StandardLinkClient<TClientContext, TClientOptions>,
    options: StandardOpenAPILinkOptions<TClientContext, TClientOptions>
  ) {
    const jsonSerializer = new StandardOpenAPIJsonSerializer(options)
    // Server response is trusted, so we can use the maximum possible array index.
    const bracketNotationSerializer = new StandardBracketNotationSerializer({ maxBracketNotationArrayIndex: 4_294_967_294 })
    const serializer = new StandardOpenAPISerializer(jsonSerializer, bracketNotationSerializer)
    const linkCodec = new StandardOpenapiLinkCodec(
      (options) => options.contractProcedure,
      serializer,
      options
    )

    super(linkCodec, linkClient, options)
  }
}

With this setup, if the user injects a headers function into StandardOpenAPIContractCallLink, for example, options.contractProcedure will be defined in their function.

What do you think about this?

[unnoq]

I don't think we should introduce something like TClientOptions. I'd prefer to avoid adding that level of complexity just to support an edge case.

[jedwards1211]

It's a shame you think of this as an edge case

[jedwards1211]

To me this is the next evolution in making API systems like this highly scalable