versatica / mediasoup

Cutting Edge WebRTC Video Conferencing
https://mediasoup.org
ISC License
6.18k stars 1.12k forks source link

fix: Windows compatibility #1402

Closed brhenrique closed 4 months ago

jmillan commented 4 months ago

@nazar-pc, this is Bruno helping me with Windows. We've done this PR p2p. I'll comment on this on Monday.

There was a problem because workerChannel had it's own tsconfig.json file and a source file was accessing the root typescript project. So now workerChannel is inside node/src/ which also makes sense.

I'll re-review it on Monday.

ibc commented 4 months ago

@nazar-pc, notice that this PR doesn't target v3 but @jmillan's ongoing branch.

Hi @brhenrique! Thanks for this. Some questions:

ibc commented 4 months ago
  • Why are you moving node/workerChannel to node/src/workerChannel? This is literally producing a lib/ folder with JS transpired files in node/src folder, which IMHO it's controversial.

I think I'm wrong. It's generating it in node/lib/workerChannel in theory. But, why do we need to include ""workerChannel/src" in "include" in node/tsconfig.json? https://github.com/versatica/mediasoup/pull/1402/files#diff-ebc4763df409943ccc3b18574546aba49d388b676eece10cd115b00239974761R13

That's wrong IMHO, such a path doesn't even exist.

jmillan commented 4 months ago

I think I'm wrong. It's generating it in node/lib/workerChannel in theory. But, why do we need to include ""workerChannel/src" in "include" in node/tsconfig.json? https://github.com/versatica/mediasoup/pull/1402/files#diff-ebc4763df409943ccc3b18574546aba49d388b676eece10cd115b00239974761R13

This PR moves workerChannel inside node/src rather than having it at the same level of node. workerChannel folder has the sources in src/ folder so workerChannel/src is where the .ts files are. That is correct.

Anyway, let's try to keep workerChannel at node level as before. The problem was introduced when I used ../../enhancedEventEmitter inside workerChannel as per your suggestion of using EnhancedEventEmitter @ibc. Windows is seeing that we are referencing a file from a different TS project and it's screwing the out dir for the .js files as seeing here.

We can simply copy enhancedEventEmitter.ts file to workerChannel/src/ so both TS projects remain independent.

I'm closing this PR for the given reasons.