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

fix initializer vs constructor order #309

Merged
merged 2 commits into from
Jan 15, 2023

Conversation

koush
Copy link
Collaborator

@koush koush commented Jan 1, 2023

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.

Copy link
Contributor

@kolserdav kolserdav left a comment

Choose a reason for hiding this comment

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

This type conflict is due to different settings in tsconfig.json. I also had experience connecting from source, but had to abandon it. It seems to me that in this question it will be correct to adjust your tsconfig.json to tsconfig.json of werift or discuss changes in tsconfig.json of werift and bring the code of werift to new required settings, but not adjust the code to your external conditions.

@koush
Copy link
Collaborator Author

koush commented Jan 1, 2023

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
Copy link
Collaborator Author

koush commented Jan 1, 2023

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
Copy link
Collaborator Author

koush commented Jan 1, 2023

References:
babel/babel#13779

microsoft/TypeScript#45995

target ESNext will begin failing to compile with:

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

@koush
Copy link
Collaborator Author

koush commented Jan 1, 2023

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
Copy link
Contributor

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
Copy link
Collaborator Author

koush commented Jan 2, 2023

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
Copy link
Collaborator Author

koush commented Jan 2, 2023

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
Copy link
Contributor

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

@kolserdav
Copy link
Contributor

Gone somewhere Checks is very alarming.

@koush
Copy link
Collaborator Author

koush commented Jan 12, 2023

@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.

@shinyoshiaki shinyoshiaki merged commit 1aaa806 into shinyoshiaki:develop Jan 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants