Allow static typing of appData

[mango-martin]

Feature Request

Hey guys. This is my first created issue please be gentle :)

A while back I ran into trouble typing my appData object because of "any" type incompatibilities. That's why I ended up not typing it. Latest changes to replace any were an improvement for sure. And it is possible to extract appData and do a type conversion when you need it, like

const appData = router.appData as RouterAppData;

But it is not ideal in my opinion.

I played around with generic types in the definition files and it is now possible for me to declare an AppDataType that is directly accessible. As an example I'll take a Router instance.

declare interface MyRouterAppData {
  existing: string
}

async function exampleWithType() {
  const worker = await createWorker();

  const router = await worker.createRouter<MyRouterAppData>({
    appData: { existing: 'TEST' },
  });

  const existing = router.appData.existing; // No type error => OK
  const nonExisting = router.appData.nonExisting; // Type error => OK
}

Changes look like this: Worker.d.ts

createRouter<T = Record<string, unknown>>({ mediaCodecs, appData }?: RouterOptions): Promise<Router<T>>;

Router.d.ts

export declare class Router <T = Record<string, unknown>> extends EnhancedEventEmitter<RouterEvents> {
...
constructor(
...
        appData?: T;
...
get appData(): T;
set appData(appData: T);

It has the current type Record<string, unknown> as a fallback value so it is a non-breaking change (but I think with these changes the standard type can be removed and the fallback type can be set to any. If you want to use a single number as your appData then I don't see any reason why it shouldn't be possible). And T is just a placeholder here. I think something like AppDataType would be better to use.

These changes would need to be made in many places depending on the scope of the refactoring. It does look a bit unnecessary to have the AppDataType variable at the top level like this but I don't really see how else this would be possible. Maybe someone who is more proficient in Typescript has a better approach? With these changes you would also need to add some boilerplate to your code. Wherever you use the Router type (to stay with the example) you would then need to write Router<MyRouterAppData> to opt-in and use your own types.

But all in all I still think it's desirable to have some way of statically typing the appData. Any thoughts on this? Cheers

Edit: Regarding #821 I can see how it might be a wanted solution to keep application logic completely separate from Mediasoup entities. But is a huge convenience for me as a developer to keep related extra data directly in the instance.

For example I keep all producers in a router in the router's appData - among other things. So instead of having to create a Map, Array or Object to keep track of all producers and query it with the router's id, I can just do the following:

onPeerJoined(router: Router, recvTransport: WebRTCTransport) {
  router.appData.producers.forEach((producer: Producer) => this.createConsumer(producer, recvTransport))
}

Where the recvTransport has a socketId in their appData and the producer has a uid in their appData.

It eliminates the need for globals and keeps the logic tightly aligned with its immediate context. I don't want to have global entries for all Transports, Producers, Consumers, Users, etc. I think it is much more sensible to keep all those things close-by. This is a very specific design-question. I personally like the current approach - but I wish I had more freedom to apply my own types to appData.

For me it comes down to: appData already exists and is completely optional. No-one forces you to use appData. But for those of us who like it, I don't see why it should be removed.

Or as ibc put it appData has a purpose and I see no reason to remove it or restrict it. 🙏

Another edit: Speaking about last edit. It's clear now that I like to use appData a lot because it makes sense to me to directly attach the data to instances where they belong. Exactly for my case, the "appData power user", these suggested changes would be a welcome addition. I have different appData for all of mediasoup's entities and it gets a bit fuzzy to make out which value is available where. Just as an example, this is my pipeTransportAppData:

const pipe = await router.createPipeTransport({
  listenIp: SERVER_IP,
  appData: {
    roomId,
    remoteId,
    localId: router.id,
    consumers: new Map() as Map<string, Consumer>,
    producers: new Map() as Map<string, Producer>,
  },
});

And this is my routerAppData

const router = await worker.createRouter({
  mediaCodecs,
  appData: {
    ...appData,
    pipes: new Map(),
    transports: new Map(),
    producers: new Map(),
    consumers: new Map(),
    users: new Set(),
  }
})

[nazar-pc]

It is not helpful to type things that you can't guarantee to conform to that API. Use https://www.npmjs.com/package/zod and parse appData into specific typed version.

But it is unlikely that mediasoup would support generic for this as I don't think it will be able to guarantee that appData always contains what you think it should.

[ibc]

I think this makes sense as far as it's not mandatory to do createRouter<MyRouterAppData>(...). So passing such a MyRouterAppData should be optional. Is it? If so, could you make a PoC PR adding that just for Router to see how it looks?

[ibc]

@nazar-pc, appData is never edited by mediasoup internals so it would be good to have proper user given TS type for it. This would avoid having to cast it everywhere in the user app. I know it because it happens to me 😊