Migrated `modules/RTC/RTC.ts` #2879

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.

Sign up for GitHub

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

Jump to bottom

Open

naman9271 wants to merge 1 commit into jitsi:master from naman9271:RTC

+263 −219

Contributor

naman9271 commented Aug 13, 2025

No description provided.

jitsi-jenkins commented Aug 13, 2025

Hi, thanks for your contribution!
If you haven't already done so, could you please make sure you sign our CLA (https://jitsi.org/icla for individuals and https://jitsi.org/ccla for corporations)? We would unfortunately be unable to merge your patch unless we have that piece :(.

naman9271 force-pushed the RTC branch 2 times, most recently from 43550ea to 5ce95dd Compare

August 16, 2025 11:48

jallamsetty1 requested a review from Copilot

August 28, 2025 18:55

Copilot AI reviewed

View reviewed changes

Copilot AI left a comment

Pull Request Overview

This PR migrates the modules/RTC/RTC.ts file to TypeScript by adding comprehensive type definitions and interfaces. The migration transforms JavaScript functions and class methods to TypeScript with proper type annotations while maintaining the existing functionality.

Key changes:

Added TypeScript interfaces for RTC options, media stream metadata, and peer connection configurations
Converted class methods and static functions to use explicit TypeScript typing
Reorganized method declarations with proper access modifiers and type safety
Added type annotations for parameters, return values, and class properties

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.

File	Description
modules/RTC/RTC.ts	Complete TypeScript migration with comprehensive interfaces and type definitions
modules/RTC/JitsiLocalTrack.ts	Changed method visibility from private to public with @internal annotation
modules/RTC/BridgeChannel.ts	Updated import to use interface instead of concrete class
JitsiMediaDevices.ts	Added @ts-ignore comment for type compatibility

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

modules/RTC/RTC.ts Outdated

    
                      this._receiverVideoConstraints = cloneDeep(constraints);

                      if (this._channel && this._channel.isOpen()) {

                      if (this?._channel.isOpen()) {

Copilot AI Aug 28, 2025

The optional chaining operator ?. is incorrect here. Since _channel is being checked for truthiness, it should be this._channel?.isOpen() or this._channel && this._channel.isOpen() to properly handle null/undefined cases.

Copilot uses AI. Check for mistakes.

modules/RTC/RTC.ts Outdated

    
                  sendSourceVideoType(sourceName, videoType) {

                      if (this._channel && this._channel.isOpen()) {

                  public sendSourceVideoType(sourceName: SourceName, videoType: BridgeVideoType): void {

                      if (this?._channel.isOpen()) {

Copilot AI Aug 28, 2025

The optional chaining operator ?. is incorrect here. Since _channel is being checked for truthiness, it should be this._channel?.isOpen() or this._channel && this._channel.isOpen() to properly handle null/undefined cases.

Copilot uses AI. Check for mistakes.

modules/RTC/RTC.ts Outdated

    
                  sendEndpointStatsMessage(payload) {

                      if (this._channel && this._channel.isOpen()) {

                  public sendEndpointStatsMessage(payload: any): void {

                      if (this?._channel.isOpen()) {

Copilot AI Aug 28, 2025

The optional chaining operator ?. is incorrect here. Since _channel is being checked for truthiness, it should be this._channel?.isOpen() or this._channel && this._channel.isOpen() to properly handle null/undefined cases.

Copilot uses AI. Check for mistakes.

modules/RTC/RTC.ts Outdated

    
                  sendReceiverAudioSubscriptionMessage(message) {

                      if (this._channel && this._channel.isOpen()) {

                  public sendReceiverAudioSubscriptionMessage(message: any): void {

                      if (this?._channel.isOpen()) {

Copilot AI Aug 28, 2025

The optional chaining operator ?. is incorrect here. Since _channel is being checked for truthiness, it should be this._channel?.isOpen() or this._channel && this._channel.isOpen() to properly handle null/undefined cases.

Copilot uses AI. Check for mistakes.

modules/RTC/RTC.ts Outdated

    
                      if (this._lastN !== value) {

                          this._lastN = value;

                          if (this._channel && this._channel.isOpen()) {

                          if (this?._channel.isOpen()) {

Copilot AI Aug 28, 2025

The optional chaining operator ?. is incorrect here. Since _channel is being checked for truthiness, it should be this._channel?.isOpen() or this._channel && this._channel.isOpen() to properly handle null/undefined cases.

Copilot uses AI. Check for mistakes.

modules/RTC/RTC.ts Outdated

    
                      enteringForwardedSources = forwardedSources.filter(

                          sourceName => oldForwardedSources.indexOf(sourceName) === -1);

                      logger.debug(`Fowarded sources changed leaving=${leavingForwardedSources}, entering=`

Copilot AI Aug 28, 2025

Typo in the debug message: 'Fowarded' should be 'Forwarded'.

Suggested change

      
                    logger.debug(`Fowarded sources changed leaving=${leavingForwardedSources}, entering=`
          
                    logger.debug(`Forwarded sources changed leaving=${leavingForwardedSources}, entering=`

Copilot uses AI. Check for mistakes.

modules/RTC/JitsiLocalTrack.ts Show resolved Hide resolved

JitsiMediaDevices.ts Outdated

    
                      // We would still want to update the permissions cache in case the permissions API is not supported.

                      RTC.addListener(

                          RTCEvents.PERMISSIONS_CHANGED,

                          // @ts-ignore

Copilot AI Aug 28, 2025

Using @ts-ignore suppresses type checking without addressing the underlying type issue. Consider fixing the type mismatch or using a more specific TypeScript directive like @ts-expect-error with an explanation.

Suggested change

      
                        // @ts-ignore
          
                        // @ts-expect-error: The type of 'permissions' from RTCEvents.PERMISSIONS_CHANGED may not match the expected type for _handlePermissionsChange, but this is intentional due to legacy event payloads.

Copilot uses AI. Check for mistakes.

naman9271 force-pushed the RTC branch from aa6b0f2 to f971ac9 Compare

August 29, 2025 07:26

Contributor Author

naman9271 commented Aug 29, 2025

changes dine and rebased please take a look

saghul reviewed

View reviewed changes

modules/RTC/RTC.ts

    
                   * @param options

                   */

                  constructor(conference, options = {}) {

                  constructor(conference: JitsiConference, options: IRTCOptions | IConferenceOptions = {}) {

Member

saghul Sep 2, 2025

Isn't it the union of both types?

Contributor Author

naman9271 Sep 2, 2025

yeah i put the union of it IRTCOptions | IConferenceOptions

modules/RTC/RTC.ts

    
                  public conference: JitsiConference;

                  public peerConnections: Map<number, TraceablePeerConnection>;

                  public localTracks: JitsiLocalTrack[];

                  public options: IRTCOptions | IConferenceOptions;

Member

saghul Sep 2, 2025

You have defined this above as static.

modules/RTC/RTC.ts Outdated

    
                          timestamp);

                  }

                  /* eslint-enable max-params */

Member

saghul Sep 2, 2025

why do we need this? if we do, please keep it as narrow as possible, just for the one function(s)

modules/RTC/RTC.ts Outdated

    
                   * @param eventType

                   * @param listener

                   */

                  public static addListener(eventType: string, listener: EventListener): void {

Member

saghul Sep 2, 2025

This class already extends from Listenable, what's going on here?

modules/xmpp/JingleSessionPC.ts Outdated

    
                          audioQuality: options.audioQuality,

                          capScreenshareBitrate: undefined,

                          codecSettings: options.codecSettings,

                          codecSettings: options.codecSettings ? { codecList: options.codecSettings } : undefined,

Member

saghul Sep 2, 2025

Is this right?

naman9271 force-pushed the RTC branch from 22e0f16 to 3d3a788 Compare

September 2, 2025 14:22

Contributor Author

naman9271 commented Sep 2, 2025

updated and rebased please take a look.

naman9271 force-pushed the RTC branch 3 times, most recently from ee0b1ba to 3dc7f21 Compare

September 5, 2025 09:29

Contributor Author

naman9271 commented Sep 5, 2025

ping @saghul , @jallamsetty1

naman9271 mentioned this pull request

Migrated modules/RTC/RTCUtils.ts #2881

Open

naman9271 force-pushed the RTC branch from 798354b to 60b7e92 Compare

September 5, 2025 20:25


          migrated(squashed)

048962f

naman9271 force-pushed the RTC branch from 60b7e92 to 048962f Compare

September 9, 2025 17:37

Member

saghul commented Sep 9, 2025

Jenkins please test this please.

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

Labels

None yet