Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add TypeScript interfaces for all exported classes #1463

Merged
merged 14 commits into from
Nov 11, 2024
Merged

Add TypeScript interfaces for all exported classes #1463

merged 14 commits into from
Nov 11, 2024

Conversation

ibc
Copy link
Member

@ibc ibc commented Nov 8, 2024

Details

  • mediasoup.types namespace now exports Worker, Router, Producer, etc types instead of classes, and those types are TypeScript interfaces.
  • New XxxxxTypes.ts files export the Xxxxx interface and related types. For example, ProducerTypes.ts exports the Producer interface and Producer related types.
  • Xxxxx.ts files export the implementation classes with name XxxxxImpl. For example, Producer.ts exports the ProducerImpl class that implements the Producer interface declared in ProducerTypes.ts.
  • Use the new interfaces everywhere.
  • Split RtpParameters.ts into rtpParametersTypes.ts and rtpParametersFbsUtils.ts and same for SCTP and SRTP files.
  • Split RtpStream.ts into rtpStreamStatsTypes.ts and rtpStreamStatsFbsUtils.
  • Rename scalabilityModes.ts tp scalabilityModesTypes.ts
  • Use import type when possible instead of import.
  • The idea is that all files ending in Types.ts only import types and do not depend on FBS generated TS code/types.
  • Add new transport.type getter than returns 'webrtc' | 'plain' | 'pipe' | 'direct'.
  • Add new rtpObserver.type getter than returns 'activespeaker' | 'audiolevel'.
  • Remove Logger from EnhancedEventEmitter.
  • This makes it easier for applications to create mediasoup mocks.
  • Also fix real random bugs such as some dump() methods missing in same classes and wrong tests.

## Details

- `mediasoup.types` namespace no longer exports `Worker`, `Router`, `Producer`, etc classes in `types` but instead it exports `WorkerInterface`, `RouterInterface`, `ProducerInterface`, etc.
  - NOTE: This is a breaking change somehow, but just at TS level.
- Use the new interfaces everywhere.
- This makes it easier for applications to create mediasoup mocks.

## TODO

- How to export as interface/types the functions and consts exposed by `index.ts` such as `createWorker()`, `setLogEventListeners()`, `getSupportedRtpCapabilities()`, `observer`, etc?
- In the future we may want to move those interfaces/types to another separate package so a Node application that only depends on mediasoup types doesn't need to install mediaosup. Even if we don't do that, we should change things so `types.ts` doesn't export **anything** that depends on compiled FBS types.
@ibc ibc requested review from jmillan and nazar-pc November 8, 2024 14:13
Copy link
Member

@jmillan jmillan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it 👍

@ibc ibc merged commit 877c489 into v3 Nov 11, 2024
25 checks passed
@ibc ibc deleted the add-ts-interfaces branch November 11, 2024 11:56
@piranna
Copy link
Contributor

piranna commented Nov 11, 2024

  • This makes it easier for applications to create mediasoup mocks.

Definitelly yes, that will be useful, just only now I have a bit of work to update Mafalda... Have you though / do you mind if we move the interfaces to a types-only package that can be used by both projects as dependency?

And related to this, do you know of any other Mediasoup mock/API project?

@ibc
Copy link
Member Author

ibc commented Nov 11, 2024

Have you though / do you mind if we move the interfaces to a types-only package that can be used by both projects as dependency?

This will probably happen but definitely not yet. Will be done once we use and evaluate how easy and useful those interfaces are, and probably more changes/additions are needed (spoiler: they are).

@piranna
Copy link
Contributor

piranna commented Nov 11, 2024

Yeah, I know they are useful, I think I asked for that interfaces several years ego for selfish reasons 😈 Just curious if it was already in the roadmap and how much soonish, or if I can help on that. I need first to fix the compatibility with 3.14.15 (I have been taking self-care and focused on my day job during last months and got totally disconnected of Mediasoup since at least May...), but after that I would need to have the types package, just only to know if do it as a nightly package that extract and publish them the same I'm doing with the tests to check for compatibility and other components, or do it by hand and make Mediasoup as dependent of it.

TL;DR: if you need help on moving out the interfaces, you can count on me.

@ibc
Copy link
Member Author

ibc commented Nov 11, 2024

We will reach that moment in which it will make sense to talk about a separate types package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants