shinyoshiaki / werift-webrtc

WebRTC Implementation for TypeScript (Node.js), includes ICE/DTLS/SCTP/RTP/SRTP/WEBM/MP4
MIT License
469 stars 30 forks source link

fix initializer vs constructor order #309

Closed koush closed 1 year ago

koush commented 1 year ago

I'm embedding the typescript files directly, and not using the werift build. There's a bug where typescript is initializing members before constructor parameters. This fixes those bugs.

koush commented 1 year ago

I think the correct solution is to address the ambiguity on initialization order altogether so it works regardless of how the any tsconfig.json is configured, whether it is werift or external.

koush commented 1 year ago

Researching this further and this is going to be an issue in es2022 and beyond as typescript modeled after esnext member initialization behavior and then esnext changed their planned behavior. Typescript is incorrect now and is slowly trying to migrate towards ecmascript behavior using useDefineForClassFields for legacy behavior, but that will also too be removed:

class Dashboard {
  name = 'stats' + this.amount;

  constructor(amount) {
    this.amount = amount;
  }

}

console.log(new Dashboard(3))

outputs:

Dashboard { name: 'statsundefined', amount: 3 }

Initializing members from constructor members will be broken in werift depending on tsconfig.json target at some point.

koush commented 1 year ago

References: https://github.com/babel/babel/issues/13779

https://github.com/microsoft/TypeScript/issues/45995

target ESNext will begin failing to compile with:

TS2729: Property 'gather' is used before its initialization.

koush commented 1 year ago

Using the above trick, I detected all the issues automatically at compile time and updated the change. Every typescript target works with this fix which makes the initialization order explicit and follows the ecmascript convention (which will become the default typescript behavior at some point).

kolserdav commented 1 year ago

I tried changing the target from ES2020 to esnext and got 26 related errors in 13 files. Then I propose to solve this problem systematically, namely, change the target to esnext in packages/webrtc/tsconfig.json and bring the types in line with the configuration.

[0] src/werift-webrtc/packages/dtls/examples/transport/ice.ts:11:24 - error TS2729: Property 'ice' is used before its initialization.
[0] 
[0] 11   readonly send = this.ice.send;
[0]                           ~~~
[0] 
[0]   src/werift-webrtc/packages/dtls/examples/transport/ice.ts:4:15
[0]     4   constructor(private ice: Connection) {
[0]                     ~~~~~~~~~~~~~~~~~~~~~~~
[0]     'ice' is declared here.
[0] 
[0] src/werift-webrtc/packages/dtls/src/context/cipher.ts:114:7 - error TS2322: Type 'CryptoKeyPair | CryptoKey' is not assignable to type 'CryptoKeyPair'.
[0]   Type 'CryptoKey' is missing the following properties from type 'CryptoKeyPair': privateKey, publicKey
[0] 
[0] 114       keys,
[0]           ~~~~
[0] 
[0]   src/werift-webrtc/node_modules/@peculiar/x509/build/index.d.ts:1364:5
[0]     1364     keys: CryptoKeyPair;
[0]              ~~~~
[0]     The expected type comes from property 'keys' which is declared here on type 'X509CertificateCreateSelfSignedParams'
[0] 
[0] src/werift-webrtc/packages/dtls/src/context/cipher.ts:119:51 - error TS2339: Property 'privateKey' does not exist on type 'CryptoKeyPair | CryptoKey'.
[0]   Property 'privateKey' does not exist on type 'CryptoKey'.
[0] 
[0] 119       await crypto.subtle.exportKey("pkcs8", keys.privateKey as any),
[0]                                                       ~~~~~~~~~~
[0] 
[0] src/werift-webrtc/packages/dtls/src/context/transport.ts:6:24 - error TS2729: Property 'socket' is used before its initialization.
[0] 
[0] 6   readonly send = this.socket.send;
[0]                          ~~~~~~
[0] 
[0]   src/werift-webrtc/packages/dtls/src/context/transport.ts:4:15
[0]     4   constructor(public socket: Transport) {}
[0]                     ~~~~~~~~~~~~~~~~~~~~~~~~
[0]     'socket' is declared here.
[0] 
[0] src/werift-webrtc/packages/dtls/src/socket.ts:39:10 - error TS2729: Property 'options' is used before its initialization.
[0] 
[0] 39     this.options.transport
[0]             ~~~~~~~
[0] 
[0]   src/werift-webrtc/packages/dtls/src/socket.ts:56:15
[0]     56   constructor(public options: Options, public sessionType: SessionTypes) {
[0]                      ~~~~~~~~~~~~~~~~~~~~~~~
[0]     'options' is declared here.
[0] 
[0] src/werift-webrtc/packages/dtls/src/socket.ts:42:10 - error TS2729: Property 'sessionType' is used before its initialization.
[0] 
[0] 42     this.sessionType,
[0]             ~~~~~~~~~~~
[0] 
[0]   src/werift-webrtc/packages/dtls/src/socket.ts:56:40
[0]     56   constructor(public options: Options, public sessionType: SessionTypes) {
[0]                                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[0]     'sessionType' is declared here.
[0] 
[0] src/werift-webrtc/packages/dtls/src/socket.ts:43:10 - error TS2729: Property 'options' is used before its initialization.
[0] 
[0] 43     this.options.cert,
[0]             ~~~~~~~
[0] 
[0]   src/werift-webrtc/packages/dtls/src/socket.ts:56:15
[0]     56   constructor(public options: Options, public sessionType: SessionTypes) {
[0]                      ~~~~~~~~~~~~~~~~~~~~~~~
[0]     'options' is declared here.
[0] 
[0] src/werift-webrtc/packages/dtls/src/socket.ts:44:10 - error TS2729: Property 'options' is used before its initialization.
[0] 
[0] 44     this.options.key,
[0]             ~~~~~~~
[0] 
[0]   src/werift-webrtc/packages/dtls/src/socket.ts:56:15
[0]     56   constructor(public options: Options, public sessionType: SessionTypes) {
[0]                      ~~~~~~~~~~~~~~~~~~~~~~~
[0]     'options' is declared here.
[0] 
[0] src/werift-webrtc/packages/dtls/src/socket.ts:45:10 - error TS2729: Property 'options' is used before its initialization.
[0] 
[0] 45     this.options.signatureHash
[0]             ~~~~~~~
[0] 
[0]   src/werift-webrtc/packages/dtls/src/socket.ts:56:15
[0]     56   constructor(public options: Options, public sessionType: SessionTypes) {
[0]                      ~~~~~~~~~~~~~~~~~~~~~~~
[0]     'options' is declared here.
[0] 
[0] src/werift-webrtc/packages/dtls/src/socket.ts:47:44 - error TS2729: Property 'options' is used before its initialization.
[0] 
[0] 47   dtls: DtlsContext = new DtlsContext(this.options, this.sessionType);
[0]                                               ~~~~~~~
[0] 
[0]   src/werift-webrtc/packages/dtls/src/socket.ts:56:15
[0]     56   constructor(public options: Options, public sessionType: SessionTypes) {
[0]                      ~~~~~~~~~~~~~~~~~~~~~~~
[0]     'options' is declared here.
[0] 
[0] src/werift-webrtc/packages/dtls/src/socket.ts:47:58 - error TS2729: Property 'sessionType' is used before its initialization.
[0] 
[0] 47   dtls: DtlsContext = new DtlsContext(this.options, this.sessionType);
[0]                                                             ~~~~~~~~~~~
[0] 
[0]   src/werift-webrtc/packages/dtls/src/socket.ts:56:40
[0]     56   constructor(public options: Options, public sessionType: SessionTypes) {
[0]                                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[0]     'sessionType' is declared here.
[0] 
[0] src/werift-webrtc/packages/ice/src/stun/transaction.ts:16:15 - error TS2729: Property 'retransmissions' is used before its initialization.
[0] 
[0] 16     1 + (this.retransmissions ? this.retransmissions : RETRY_MAX);
[0]                  ~~~~~~~~~~~~~~~
[0] 
[0]   src/werift-webrtc/packages/ice/src/stun/transaction.ts:23:5
[0]     23     private retransmissions?: number
[0]            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[0]     'retransmissions' is declared here.
[0] 
[0] src/werift-webrtc/packages/ice/src/stun/transaction.ts:16:38 - error TS2729: Property 'retransmissions' is used before its initialization.
[0] 
[0] 16     1 + (this.retransmissions ? this.retransmissions : RETRY_MAX);
[0]                                         ~~~~~~~~~~~~~~~
[0] 
[0]   src/werift-webrtc/packages/ice/src/stun/transaction.ts:23:5
[0]     23     private retransmissions?: number
[0]            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[0]     'retransmissions' is declared here.
[0] 
[0] src/werift-webrtc/packages/ice/src/transport.ts:15:38 - error TS2729: Property 'type' is used before its initialization.
[0] 
[0] 15   private socket = createSocket(this.type);
[0]                                         ~~~~
[0] 
[0]   src/werift-webrtc/packages/ice/src/transport.ts:19:5
[0]     19     private type: SocketType,
[0]            ~~~~~~~~~~~~~~~~~~~~~~~~
[0]     'type' is declared here.
[0] 
[0] src/werift-webrtc/packages/rtp/src/processor/avBuffer.ts:15:23 - error TS2729: Property 'options' is used before its initialization.
[0] 
[0] 15   bufferLength = this.options.bufferLength ?? 50;
[0]                          ~~~~~~~
[0] 
[0]   src/werift-webrtc/packages/rtp/src/processor/avBuffer.ts:31:5
[0]     31     private options: Partial<AvBufferOptions> = {}
[0]            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[0]     'options' is declared here.
[0] 
[0] src/werift-webrtc/packages/rtp/src/processor/avBuffer.ts:25:27 - error TS2729: Property 'options' is used before its initialization.
[0] 
[0] 25   private interval = this.options.interval ?? 500;
[0]                              ~~~~~~~
[0] 
[0]   src/werift-webrtc/packages/rtp/src/processor/avBuffer.ts:31:5
[0]     31     private options: Partial<AvBufferOptions> = {}
[0]            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[0]     'options' is declared here.
[0] 
[0] src/werift-webrtc/packages/sctp/src/sctp.ts:102:28 - error TS2729: Property 'port' is used before its initialization.
[0] 
[0] 102   private localPort = this.port;
[0]                                ~~~~
[0] 
[0]   src/werift-webrtc/packages/sctp/src/sctp.ts:168:44
[0]     168   constructor(public transport: Transport, public port = 5000) {
[0]                                                    ~~~~~~~~~~~~~~~~~~
[0]     'port' is declared here.
[0] 
[0] src/werift-webrtc/packages/webrtc/src/dataChannel.ts:23:21 - error TS2729: Property 'parameters' is used before its initialization.
[0] 
[0] 23   id: number = this.parameters.id;
[0]                        ~~~~~~~~~~
[0] 
[0]   src/werift-webrtc/packages/webrtc/src/dataChannel.ts:31:5
[0]     31     private readonly parameters: RTCDataChannelParameters,
[0]            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[0]     'parameters' is declared here.
[0] 
[0] src/werift-webrtc/packages/webrtc/src/media/rtpSender.ts:76:17 - error TS2729: Property 'trackOrKind' is used before its initialization.
[0] 
[0] 76     typeof this.trackOrKind === "string"
[0]                    ~~~~~~~~~~~
[0] 
[0]   src/werift-webrtc/packages/webrtc/src/media/rtpSender.ts:126:15
[0]     126   constructor(public trackOrKind: Kind | MediaStreamTrack) {
[0]                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[0]     'trackOrKind' is declared here.
[0] 
[0] src/werift-webrtc/packages/webrtc/src/media/rtpSender.ts:77:14 - error TS2729: Property 'trackOrKind' is used before its initialization.
[0] 
[0] 77       ? this.trackOrKind
[0]                 ~~~~~~~~~~~
[0] 
[0]   src/werift-webrtc/packages/webrtc/src/media/rtpSender.ts:126:15
[0]     126   constructor(public trackOrKind: Kind | MediaStreamTrack) {
[0]                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[0]     'trackOrKind' is declared here.
[0] 
[0] src/werift-webrtc/packages/webrtc/src/media/rtpSender.ts:78:14 - error TS2729: Property 'trackOrKind' is used before its initialization.
[0] 
[0] 78       : this.trackOrKind.kind;
[0]                 ~~~~~~~~~~~
[0] 
[0]   src/werift-webrtc/packages/webrtc/src/media/rtpSender.ts:126:15
[0]     126   constructor(public trackOrKind: Kind | MediaStreamTrack) {
[0]                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[0]     'trackOrKind' is declared here.
[0] 
[0] src/werift-webrtc/packages/webrtc/src/transport/dtls.ts:52:44 - error TS2729: Property 'certificates' is used before its initialization.
[0] 
[0] 52   localCertificate?: RTCCertificate = this.certificates[0];
[0]                                               ~~~~~~~~~~~~
[0] 
[0]   src/werift-webrtc/packages/webrtc/src/transport/dtls.ts:59:5
[0]     59     readonly certificates: RTCCertificate[],
[0]            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[0]     'certificates' is declared here.
[0] 
[0] src/werift-webrtc/packages/webrtc/src/transport/dtls.ts:343:24 - error TS2729: Property 'ice' is used before its initialization.
[0] 
[0] 343   readonly send = this.ice.send;
[0]                            ~~~
[0] 
[0]   src/werift-webrtc/packages/webrtc/src/transport/dtls.ts:334:15
[0]     334   constructor(private ice: Connection) {
[0]                       ~~~~~~~~~~~~~~~~~~~~~~~
[0]     'ice' is declared here.
[0] 
[0] src/werift-webrtc/packages/webrtc/src/transport/ice.ts:12:21 - error TS2729: Property 'gather' is used before its initialization.
[0] 
[0] 12   connection = this.gather.connection;
[0]                        ~~~~~~
[0] 
[0]   src/werift-webrtc/packages/webrtc/src/transport/ice.ts:19:15
[0]     19   constructor(private gather: RTCIceGatherer) {
[0]                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[0]     'gather' is declared here.
[0] 
[0] src/werift-webrtc/packages/webrtc/src/transport/ice.ts:119:52 - error TS2729: Property 'options' is used before its initialization.
[0] 
[0] 119   readonly connection = new Connection(false, this.options);
[0]                                                        ~~~~~~~
[0] 
[0]   src/werift-webrtc/packages/webrtc/src/transport/ice.ts:121:15
[0]     121   constructor(private options: Partial<IceOptions> = {}) {}
[0]                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[0]     'options' is declared here.
[0] 
[0] src/werift-webrtc/packages/webrtc/src/transport/sctp.ts:384:24 - error TS2729: Property 'dtls' is used before its initialization.
[0] 
[0] 384   readonly send = this.dtls.sendData;
[0]                            ~~~~
[0] 
[0]   src/werift-webrtc/packages/webrtc/src/transport/sctp.ts:380:15
[0]     380   constructor(private dtls: RTCDtlsTransport) {}
[0]                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[0]     'dtls' is declared here.
[0] 
[0] 
[0] Found 26 errors in 13 files.
[0] 
[0] Errors  Files
[0]      1  src/werift-webrtc/packages/dtls/examples/transport/ice.ts:11
[0]      2  src/werift-webrtc/packages/dtls/src/context/cipher.ts:114
[0]      1  src/werift-webrtc/packages/dtls/src/context/transport.ts:6
[0]      7  src/werift-webrtc/packages/dtls/src/socket.ts:39
[0]      2  src/werift-webrtc/packages/ice/src/stun/transaction.ts:16
[0]      1  src/werift-webrtc/packages/ice/src/transport.ts:15
[0]      2  src/werift-webrtc/packages/rtp/src/processor/avBuffer.ts:15
[0]      1  src/werift-webrtc/packages/sctp/src/sctp.ts:102
[0]      1  src/werift-webrtc/packages/webrtc/src/dataChannel.ts:23
[0]      3  src/werift-webrtc/packages/webrtc/src/media/rtpSender.ts:76
[0]      2  src/werift-webrtc/packages/webrtc/src/transport/dtls.ts:52
[0]      2  src/werift-webrtc/packages/webrtc/src/transport/ice.ts:12
[0]      1  src/werift-webrtc/packages/webrtc/src/transport/sctp.ts:384
[0] npm run dev:server exited with code 2
koush commented 1 year ago

Yes, that's what I did. But I only changed the files necessary for including werift source files. The tsconfig I changed was my own project. The only difference between my change and your errors is likely the one examples file which is not embedded in my project.

koush commented 1 year ago

Werift should be not be changing the packages/webrtc/tsconfig.json to esnext, if that is what you are suggesting. That will break the build for people using werift running on targets like node 14/16, etc. My change is a minimally invasive change. I'm fine updating the examples file within my fix, but it's not necessary for the scope of my request.

kolserdav commented 1 year ago

for what reason will not work on 14 and 16 nodes?

kolserdav commented 1 year ago

Gone somewhere Checks is very alarming.

koush commented 1 year ago

@shinyoshiaki rebased on top of your changes. any thoughts? would like to get this merged as embedding werift source files lets me debug it much easier.