ESM/commonjs: parallel attempt

[ibc]

Fork of https://github.com/versatica/mediasoup-client/pull/267

• Here I fix the typescript:watch task by using tsc --watch --noEmit false instead of relying on tsc-watch dep.

[ibc]

Concern here is the size of the lib folder, which is the one in the mediasoup-client NPM package, and "thanks" to this PR its size moves from 1.0 MB to 2.4 MB :(

Generated lib folder by calling npm run typescript:

Using mediasoup-client v3 branch

-rw-r--r--   1 ibc  wheel   2.8K Jun 23 14:27 Consumer.d.ts
-rw-r--r--   1 ibc  wheel   2.3K Jun 23 14:27 Consumer.d.ts.map
-rw-r--r--   1 ibc  wheel   4.6K Jun 23 14:27 Consumer.js
-rw-r--r--   1 ibc  wheel   2.4K Jun 23 14:27 DataConsumer.d.ts
-rw-r--r--   1 ibc  wheel   1.9K Jun 23 14:27 DataConsumer.d.ts.map
-rw-r--r--   1 ibc  wheel   4.3K Jun 23 14:27 DataConsumer.js
-rw-r--r--   1 ibc  wheel   2.5K Jun 23 14:27 DataProducer.d.ts
-rw-r--r--   1 ibc  wheel   1.9K Jun 23 14:27 DataProducer.d.ts.map
-rw-r--r--   1 ibc  wheel   4.9K Jun 23 14:27 DataProducer.js
-rw-r--r--   1 ibc  wheel   3.4K Jun 23 14:27 Device.d.ts
-rw-r--r--   1 ibc  wheel   2.0K Jun 23 14:27 Device.d.ts.map
-rw-r--r--   1 ibc  wheel    18K Jun 23 14:27 Device.js
-rw-r--r--   1 ibc  wheel   1.4K Jun 23 14:27 EnhancedEventEmitter.d.ts
-rw-r--r--   1 ibc  wheel   1.8K Jun 23 14:27 EnhancedEventEmitter.d.ts.map
-rw-r--r--   1 ibc  wheel   2.0K Jun 23 14:27 EnhancedEventEmitter.js
-rw-r--r--   1 ibc  wheel   313B Jun 23 14:27 Logger.d.ts
-rw-r--r--   1 ibc  wheel   420B Jun 23 14:27 Logger.d.ts.map
-rw-r--r--   1 ibc  wheel   1.2K Jun 23 14:27 Logger.js
-rw-r--r--   1 ibc  wheel   4.3K Jun 23 14:27 Producer.d.ts
-rw-r--r--   1 ibc  wheel   3.6K Jun 23 14:27 Producer.d.ts.map
-rw-r--r--   1 ibc  wheel   8.1K Jun 23 14:27 Producer.js
-rw-r--r--   1 ibc  wheel   9.7K Jun 23 14:27 RtpParameters.d.ts
-rw-r--r--   1 ibc  wheel   2.6K Jun 23 14:27 RtpParameters.d.ts.map
-rw-r--r--   1 ibc  wheel   177B Jun 23 14:27 RtpParameters.js
-rw-r--r--   1 ibc  wheel   1.7K Jun 23 14:27 SctpParameters.d.ts
-rw-r--r--   1 ibc  wheel   783B Jun 23 14:27 SctpParameters.d.ts.map
-rw-r--r--   1 ibc  wheel    77B Jun 23 14:27 SctpParameters.js
-rw-r--r--   1 ibc  wheel   7.5K Jun 23 14:27 Transport.d.ts
-rw-r--r--   1 ibc  wheel   5.7K Jun 23 14:27 Transport.d.ts.map
-rw-r--r--   1 ibc  wheel    33K Jun 23 14:27 Transport.js
-rw-r--r--   1 ibc  wheel   341B Jun 23 14:27 errors.d.ts
-rw-r--r--   1 ibc  wheel   241B Jun 23 14:27 errors.d.ts.map
-rw-r--r--   1 ibc  wheel   1.1K Jun 23 14:27 errors.js
drwxr-xr-x  43 ibc  wheel   1.3K Jun 23 14:27 handlers/
-rw-r--r--   1 ibc  wheel   541B Jun 23 14:27 index.d.ts
-rw-r--r--   1 ibc  wheel   466B Jun 23 14:27 index.d.ts.map
-rw-r--r--   1 ibc  wheel   2.1K Jun 23 14:27 index.js
-rw-r--r--   1 ibc  wheel   5.0K Jun 23 14:27 ortc.d.ts
-rw-r--r--   1 ibc  wheel   1.9K Jun 23 14:27 ortc.d.ts.map
-rw-r--r--   1 ibc  wheel    34K Jun 23 14:27 ortc.js
-rw-r--r--   1 ibc  wheel   210B Jun 23 14:27 scalabilityModes.d.ts
-rw-r--r--   1 ibc  wheel   272B Jun 23 14:27 scalabilityModes.d.ts.map
-rw-r--r--   1 ibc  wheel   555B Jun 23 14:27 scalabilityModes.js
drwxr-xr-x  11 ibc  wheel   352B Jun 23 14:27 tests/
-rw-r--r--   1 ibc  wheel   462B Jun 23 14:27 types.d.ts
-rw-r--r--   1 ibc  wheel   443B Jun 23 14:27 types.d.ts.map
-rw-r--r--   1 ibc  wheel   1.2K Jun 23 14:27 types.js
-rw-r--r--   1 ibc  wheel   239B Jun 23 14:27 utils.d.ts
-rw-r--r--   1 ibc  wheel   233B Jun 23 14:27 utils.d.ts.map
-rw-r--r--   1 ibc  wheel   533B Jun 23 14:27 utils.js

Total size: 1.0 MB

Using esm/commonjs branch

-rw-r--r--  1 ibc  wheel    45K Jun 23 14:32 index.d.ts
-rw-r--r--  1 ibc  wheel   375K Jun 23 14:32 index.js
-rw-r--r--  1 ibc  wheel   840K Jun 23 14:32 index.js.map
-rw-r--r--  1 ibc  wheel   374K Jun 23 14:32 index.mjs
-rw-r--r--  1 ibc  wheel   840K Jun 23 14:32 index.mjs.map

Total size: 2.4 MB

[ibc]

@satoren can you review please?

[satoren]

LGTM

Concern here is the size of the lib folder, which is the one in the mediasoup-client NPM package, and "thanks" to this PR its size moves from 1.0 MB to 2.4 MB :(

I don't know the exact reason why, but it seems that map files tend to be larger. I assume that the code changes by bundler are probably so large that a lot of data is needed for the difference.

Using esm/commonjs branch

 % du -h -s lib/**/*.map   
840K    lib/index.js.map
840K    lib/index.mjs.map

% du -h -c lib/**/*.js     
376K    lib/index.js
376K    total

mediasoup-client v3 branch

 % du -h -c lib/**/*.map
164K    total

% du -h -c lib/**/*.js 
660K    total

[ibc]

Ok so are we good anyway? AFAIU bigger size is a downside of this approach and pros are... smaller bundled app when importing mediasoup-client in other apps/projects? To simplify: which are the pros of this approach? I would like to know them in order to properly document the benefits of this change.

[ibc]

Another question: why are we setting "module: esnext" in tsconfig.json? Couldn't we set a fixed mode instead of "esnext" which always means "last and/or experimental ES version"?

[satoren]

Ok so are we good anyway? AFAIU bigger size is a downside of this approach and pros are... smaller bundled app when importing mediasoup-client in other apps/projects? To simplify: which are the pros of this approach? I would like to know them in order to properly document the benefits of this change.

The disadvantage is the larger size of the npm package. Advantage is smaller js size. (The map file is not downloaded by the end user.)

Another question: why are we setting "module: esnext" in tsconfig.json? Couldn't we set a fixed mode instead of "esnext" which always means "last and/or experimental ES version"?

https://github.com/rollup/plugins/pull/788 ES2015 may also work.

[ibc]

ES2015 may also work

Perhaps ES2020 would be better? (assuming that all browsers and JS engines such as react-native support it).

[ibc]

I'm merging this. Thanks a lot @satoren

[ibc]

@satoren I've reverted this PR.

By design it generates a bundle in lib folder which means that lib folder only contains these files:

index.d.ts    
index.js      
index.js.map  
index.mjs     
index.mjs.map

This prevents documented and desired usage such as import * as mediasoupClientTypes from 'mediasoup-client/lib/types';. Definitely I want to make it possible for users to directly import a specific file from the mediasoup-client/lib folder rather than having to import all types from the index file.

Not sure if there is a way to address this problem by exposing both ESM and CommonJS modules.

[satoren]

I see, that usage is not compatible with bundler. If it is only some of the files, it can be handled by adding entry points like this, but probably not enough, right? It may be better to leave the tsc output in the lib directory as it is and output esm to another directory. Like this

Also, it should be noted that import * as mediasoupClientTypes from 'mediasoup-client/lib/types'; imports the actual class, since types.ts exports the actual class, not just the type, for almost all classes. Therefore, there is currently little advantage to importing mediasoup-client/lib/types directly. I think types.ts should be changed to export only types.

[ibc]

I see, that usage is not compatible with bundler. If it is only some of the files, it can be handled by adding entry points like this, but probably not enough, right?

Nope. Definitely I want that the user can import any specific file in the path. I know that there are lot of different approaches here, all them valid. One of them is encouraging users to import specific files in the path. We chose that and cannot break it now.

It may be better to leave the tsc output in the lib directory as it is and output esm to another directory. Like this

So how would that work in real life? If an app imports main file it will import the ESM module. However, if within the same app it also imports a specific file in the path, then it will import a commonjs file? Honestly I think that could be even problematic.

Also, it should be noted that import * as mediasoupClientTypes from 'mediasoup-client/lib/types'; imports the actual class, since types.ts exports the actual class, not just the type, for almost all classes. Therefore, there is currently little advantage to importing mediasoup-client/lib/types directly. I think types.ts should be changed to export only types.

Makes sense. Will do in a separate PR.

[ibc]

Here: https://github.com/versatica/mediasoup-client/issues/273

[satoren]

@ibc I haven't tried it because I'm busy, but it may be possible to build while maintaining individual files using the options below. https://rollupjs.org/configuration-options/#output-preservemodules https://rollupjs.org/configuration-options/#output-preservemodulesroot

[ibc]

Thanks, will check in v4.