Add generic types for appData
Regarding #832
I've been pretty swamped lately but I finally got around to do a barebones prototype of the suggested changes. I'll have to preface that I am not very fluent in TypeScript. But from what VSCode tells me it's been successful. I'll attach some screenshots to show the impact of the changes. Because of the fallbacktype = AppData ({ [key: string]: unknown }) this is a non-breaking change.
To be clear: This only covers the Router class as an example. To bring these changes to all classes where appData is used, this would need to be implemented across many more files. I just want to bring this issue into an actionable state. If you like, I can also extend this to cover all of the core code but it will take some time. Update: I've added another commit that covers all of mediasoup core code.
The only real downside with this, as far as I can tell, is that this does not correctly alarm of a TypeError if there was no appData provided - but I think it's an edge case. If you don't use appData, you also shouldn't be accessing it.
Cheers
I've updated the PR already to cover all of the code - it was much more straightforward than I thought.
Few questions remain about naming the types individually (in some classes a general name would conflict with another type that was declared at the class level - so I went with naming them individually). This might be a bit hard to maintain.
Another thing is the location of my helper type AppData. Basically you could replace every occurence of AppData with Record<string, unknown> but it just seemed reasonable to have a bit of a shorter name for that.
Lastly about formatting. Because The added types would often exceed the linter limit of characters per line, I had to put in a few line-breaks.
Also there are no fallback types for any Options. It just takes a generic. This might lead to confusing result where you can type an option like
const routerOptions: RouterOptions<number> = { // OK
appData: 10,
}
const router = await worker.createRouter(routerOptions) // Typeerror
Can add the same structure for options as well so it will already throw a typeerror when declaring them, it just seemed a little bit of an overkill but looking at it now, it should not be like this.
Let me know how you want to proceed with this. Or not, up to you :D
We don't forget about this. It's just that we have been very busy.
@mango-martin I've replaced your PR with this one https://github.com/versatica/mediasoup/pull/1046 due to missing additions that shown up after we migrated tests to TypeScript. Credits to you. Thanks!