Refactor ServiceContext as generic

[daweifeng-replit]

Why

When we are creating a server using river, we have to declare the ServiceContext interface globally so TypeScript can infer the type for ctx within a procedure.

 declare module '@replit/river' {
   interface ServiceContext {
       db: Database;
   }
 }

However, it will be an issue in a mono repo setup where we want to create multiple servers with different ServiceContext.

What changed

Use builder pattern for ServiceSchema so we can create a ServiceSchema with Context like

const ServiceSchema = createServiceSchema<Context>();

Example:

const extendedContext = {
  testctx: Math.random().toString(),
};

const ServiceSchema = createServiceSchema<typeof extendedContext>();

const services = {
  testservice: ServiceSchema.define({
    testrpc: Procedure.rpc({
      requestInit: Type.Object({}),
      responseData: Type.String(),
      handler: async ({ ctx }) => {
        return Ok(ctx.testctx);
      },
    }),
  }),
};

When we create a server, it will perform type checking to make sure that all the services are expecting the same ServiceContext as the type of extendedContext in the config.

createServer(serverTransport, services, {
  extendedContext,
});

There is no type-checking for Context in createClient(). We are using any to skip it behind the scene because it is a implementation detail on the server side.

const client = createClient<typeof services>(
  clientTransport,
  serverTransport.clientId,
); 

Versioning

• Breaking protocol change
• Breaking ts/js API change

[jackyzha0]

hm I feel pretty strongly we shouldn't change the default ServiceSchema.define into ServiceSchema.defineWithContext() for this to work, we might be able to get away with a slightly less breaking change

my other main nit is that in theory, we shouldn't need to change the generics on createClient either as ServiceContext is purely a server-side concept so it makes no sense for a client to reason about ServiceContext

let's talk about this more tomorrow in the office but perhaps we can subclass ServiceSchema to be ServiceSchemaWithContext (metaclasses/class factories :))

[Monkatraz]

Another idea is to do something like:

import { Procedure as BaseProcedure } from '@replit/river'

export const Procedure = BaseProcedure.extend<ExtendedContext>()

[Monkatraz]

Ah yeah Jacky's idea is better, we basically want a server builder scaffold

[Monkatraz]

Do we have to pass the extended context as a parameter? That goes against how e.g. Chort defines its services and will require a significant refactor

I think it goes against the spirit of River's service schema if you have to instantiate a bunch of potential side effects just to construct the schema

[daweifeng-replit]

Do we have to pass the extended context as a parameter? That goes against how e.g. Chort defines its services and will require a significant refactor

I think it goes against the spirit of River's service schema if you have to instantiate a bunch of potential side effects just to construct the schema

There is another option which is

const extendedContext = {
    testctx: Math.random().toString(),
};
const ServiceSchema = createServiceSchema<typeof extendedContext>();

Would this work for Chort?

[jackyzha0]

yeah now that i think of it just having the type and not constructing the whole object is probably better, lets go with the second approach

[daweifeng-replit]

yeah now that i think of it just having the type and not constructing the whole object is probably better, lets go with the second approach

Yup, it is updated to const ServiceSchema = createServiceSchema<typeof extendedContext>();

[jackyzha0]

id like to see if we can spend a bit more time avoiding the any before we merge, it scares me and tells us the types are flowing wrong somewhere

[daweifeng-replit]

id like to see if we can spend a bit more time avoiding the any before we merge, it scares me and tells us the types are flowing wrong somewhere

Looking into the code. We might not be able to do it without any. Here is an example

type Handler<T> = (ctx: T) => void;

const specificHandler: Handler<{ db: string }> = (ctx) => {
  console.log(ctx.db); // Expects 'db' to exist
};

type AnyHandlerMap<Context extends object = object> = Record<string, Handler<Context>>

const services = {
  specificHandler
}

const generalServices: AnyHandlerMap<object> = services

TypeScript would complain about

Type '{ specificHandler: Handler<{ db: string; }>; }' is not assignable to type 'AnyHandlerMap<object>'.
  Property 'specificHandler' is incompatible with index signature.
    Type 'Handler<{ db: string; }>' is not assignable to type 'Handler<object>'.
      Property 'db' is missing in type '{}' but required in type '{ db: string; }'. (typescript)

Because AnyHandlerMap is not actual accepting "Any" procedures unless we set it to AnyHandlerMap<any>

[daweifeng-replit]

As we discussed offline, we are going to keep any in the Context param of createClient() and Client generic. One potential idea to fix that is to create a new ClientService type that does not require Context so we can map ServiceSchema to ClientService. We can try it in the future.

[daweifeng-replit]

• Refactor ParsedMetadata as generic #324
• Refactor ServiceContext as generic #322 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[daweifeng-replit]

Merge activity

• Jun 16, 6:51 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Jun 16, 6:51 PM UTC: @daweifeng-replit merged this pull request with Graphite.