Migrate to ESBuild in Webpack #10055

ShGKme · 2023-07-26T13:29:35Z

☑️ Resolves

Part 1: fixing issues with `webrtc-adapter` and `attachmediastream`.

webrtc-adapter library is supposed to be imported as ESModule and doesn't work with require.

Because of this, we had a special processing for webrtc-adapter with babel-plugin-add-module-export:

spreed/webpack.common.config.js

Lines 46 to 65 in 36f53c1

    
           				/** 
        
           				 * webrtc-adapter main module does no longer provide 
        
           				 * "module.exports", which is expected by some elements using it 
        
           				 * (like "attachmediastream"), so it needs to be added back with 
        
           				 * a plugin. 
        
           				 */ 
        
           				test: /node_modules[\\/]webrtc-adapter[\\/].*\.js$/, 
        
           				loader: 'babel-loader', 
        
           				options: { 
        
           					plugins: ['add-module-exports'], 
        
           					presets: [ 
        
           						/** 
        
           						 * From "add-module-exports" documentation: 
        
           						 * "webpack doesn't perform commonjs transformation for 
        
           						 * codesplitting. Need to set commonjs conversion." 
        
           						 */ 
        
           						['@babel/env', { modules: 'commonjs' }], 
        
           					], 
        
           				}, 
        
           			},

But this problem can be avoided by:

Importing webrtc-adapter with ESM import instead of require

Using attachmediastream with its build for bundlers (

import attachmediastream from 'attachmediastream/attachmediastream.bundle.js'`

This PR does as described above. So we don't need additional babel plugin and special processing for webrtc-adapter. This is also required for migration to ESBuild as well as Vite.

This change can be separated into individual PR.

cc @danxuliu

Part 2: Use ESBuild for transpiling JS instead of Terser and minimizing instead of Terser

Fix: Decrease the bundle size and the build time #9406

Production build with Terser minification in Webpack is very slow and requires much RAM.
The same (but not so huge) problem with Babel.

I close all applications on my 16GB RAM server to be able to make a production build 😣

ESBuild is slightly worse as a minifier but much more resource-effective. Results on my server:

Metric	Babel + Terser	ESBuild
`npm run build` assets size	51.33 MB	52.41 MB
`npm run build` time	83.9s	26.5s
`npm run build` RAM	8.8 GB	3.3 GB
`npm run dev` time	25.5s	13.8s
`npm run dev` RAM	1.3 GB	0.9 GB

👀 Alternative solutions

Fully migrate to Vite

🚧 Tasks

Add esbuild-loader
Use ESBuildPlugin as minimizer
Use esbuild-loader as JavaScript transpiler
Setup config merging rules
Check with Talk Desktop
- Only requires to add esbuild-loader as a dependency and change target to es2022
- But requires a bit more work to make a config compatible with both esbuild and babel
Update Babel config to be used only for Jest tests

🏁 Checklist

⛑️ Tests (unit and/or integration) are included or not required
📘 API documentation in docs/ has been updated or is not required
📗 User documentation in https://github.com/nextcloud/documentation/tree/master/user_manual/talk has been updated or is not required
🔖 Capability is added or not needed

ShGKme · 2023-07-26T14:02:15Z

We can use this as a simple solution to save some time and RAM requirements for production build.

Or we can try to make a full Vite migration and make a hackable shared Vite config with @susnux

ShGKme · 2023-07-26T14:44:57Z

UPD: this vite config https://github.com/nextcloud/nextcloud-vite-config

Antreesy · 2023-07-26T17:09:50Z

So far, here a mine results (16 Gb RAM):

Param	Babel + Terser	ESBuild
Notes	Had to leave only IDE working	IDE + Talk Desktop + Docker
assets by status	55 MiB	56 MiB
compiled in	193 700 ms	47 070 ms

ShGKme · 2023-07-27T15:36:56Z

Vite migration is not doing well for me for now.

I'd consider using this PR as a very simple and not so risky solution.

SystemKeeper · 2023-07-27T18:13:56Z

Works for me only if I downgrade esbuild-loader to version 2:

diff --git a/package.json b/package.json
index 8520ca615..79ce7f82d 100644
--- a/package.json
+++ b/package.json
@@ -78,7 +78,7 @@
     "@vue/vue2-jest": "^29.2.4",
     "babel-loader-exclude-node-modules-except": "^1.2.1",
     "babel-plugin-add-module-exports": "^1.0.4",
-    "esbuild-loader": "^3.0.1",
+    "esbuild-loader": "^2.21.0",
     "flush-promises": "^1.0.2",
     "jest": "^29.6.1",
     "jest-environment-jsdom": "^29.6.1",
diff --git a/webpack.config.js b/webpack.config.js
index 450b878a2..b87670a7c 100644
--- a/webpack.config.js
+++ b/webpack.config.js
@@ -1,6 +1,6 @@
 const path = require('node:path')
 
-const { EsbuildPlugin } = require('esbuild-loader')
+const { ESBuildMinifyPlugin } = require('esbuild-loader')
 const webpack = require('webpack')
 const { mergeWithRules } = require('webpack-merge')
 
@@ -62,7 +62,7 @@ module.exports = talkWebMerge(nextcloudWebpackConfig, commonWebpackConfig, {
 
        optimization: {
                minimizer: [
-                       new EsbuildPlugin({
+                       new ESBuildMinifyPlugin({
                                target: 'es2020',
                        }),
                ],

If I use version 3, I get

Error: File '@vue/tsconfig/tsconfig.json' not found.
    at Oe (/Users/marcel/Desktop/Nextcloud/workspace/server/apps-extra/spreed/node_modules/get-tsconfig/dist/index.cjs:3:9061)
    at G (/Users/marcel/Desktop/Nextcloud/workspace/server/apps-extra/spreed/node_modules/get-tsconfig/dist/index.cjs:3:9848)
    at Object.je [as getTsconfig] (/Users/marcel/Desktop/Nextcloud/workspace/server/apps-extra/spreed/node_modules/get-tsconfig/dist/index.cjs:3:10586)
    at Object.ESBuildLoader (/Users/marcel/Desktop/Nextcloud/workspace/server/apps-extra/spreed/node_modules/esbuild-loader/dist/index.cjs:49:23)
    at LOADER_EXECUTION (/Users/marcel/Desktop/Nextcloud/workspace/server/apps-extra/spreed/node_modules/loader-runner/lib/LoaderRunner.js:132:14)
    at runSyncOrAsync (/Users/marcel/Desktop/Nextcloud/workspace/server/apps-extra/spreed/node_modules/loader-runner/lib/LoaderRunner.js:133:4)
    at iterateNormalLoaders (/Users/marcel/Desktop/Nextcloud/workspace/server/apps-extra/spreed/node_modules/loader-runner/lib/LoaderRunner.js:250:2)
    at /Users/marcel/Desktop/Nextcloud/workspace/server/apps-extra/spreed/node_modules/loader-runner/lib/LoaderRunner.js:223:4
    at /Users/marcel/Desktop/Nextcloud/workspace/server/apps-extra/spreed/node_modules/webpack/lib/NormalModule.js:836:15
    at eval (eval at create (/Users/marcel/Desktop/Nextcloud/workspace/server/apps-extra/spreed/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:12:1)

It works if I add a empty tsconfig.json file...

SystemKeeper · 2023-07-27T21:19:04Z

Okay, so the issue is the following:

I have cloned the spreed repo in server/apps-extra/spreed/.
According to https://www.typescriptlang.org/docs/handbook/tsconfig-json.html the tsconfig.json file is searched in the current directory and if not found, searched in the parent directory
Since we don't have a tsconfig.json in talk, the tsconfig.json of server is used
As I usually don't develop frontend on server, I did not run npm ci in the server repository. As the tsconfig.json from server extends @vue/tsconfig/tsconfig.json, this file can't be found on my machine.
Running npm ci inside the server repo was enough to make talk in a subdirectory build (note: it worked fine as well when moved outside of the server repository, e.g. no tsconfig.json could be located in any parent directory).

Would it make sense to adjust the behaviour in talk about that? Either webpack config or include a tsconfig.json to prevent any sideeffects with parent-level tsconfig.json files?

SystemKeeper · 2023-07-27T21:21:06Z

For the performance:

# npm run build
webpack 5.76.2 compiled with 2 warnings in 15411 ms

# npm run dev
webpack 5.76.2 compiled successfully in 7790 ms

so that's quite nice ;)

Reference on master:

# npm run build
webpack 5.76.2 compiled with 2 warnings in 49627 ms

# npm run dev
webpack 5.76.2 compiled successfully in 12890 ms

ShGKme · 2023-07-28T06:46:52Z

Thank you @SystemKeeper for your investigation!

So it seems that eslint-loader is trying to find tsconfig by get-tsconfig and then the config is found in the server root. And there is no way to disable it ><

https://github.com/esbuild-kit/esbuild-loader/blob/60cb3b4746c675167ff3a3ab4f4bddefde9e44fe/src/loader.ts#L51-L65

I'll implicitly set some config to avoid such errors.

ShGKme · 2023-07-28T09:41:54Z

Reference on master:
# npm run build
49627 ms
# npm run dev
12890 ms

So fast 😯

ShGKme · 2023-07-28T10:31:15Z

@SystemKeeper Could you, please, check that now it works for you?

SystemKeeper · 2023-07-28T12:49:51Z

@SystemKeeper Could you, please, check that now it works for you?

Works fine now for me, thanks 👍

ShGKme · 2023-07-28T13:49:14Z

Ready for the the review, description updated

Antreesy

Worked for me as well, finally can build a Talk app 🥲

nickvergessen

Also compiles here, 11s dev build, 21s prod build

susnux · 2023-08-04T12:49:59Z

Use only one module system (ESM) everywhere and do not mix

Much cleaner solution 👍

ShGKme · 2023-08-04T19:38:15Z

Calls work now, including video, virtual backgrounds, and blur.

Antreesy

Also tested the main functionality in calls and chats, Firefox and Chrome (Linux). Consoles are clean 🏁

We could try to merge it and see if we'll catch some bugs (because of these changes) during the development or backport (though any issues with bundling will be seen on sermo next day, I suppose).

Antreesy · 2023-08-14T14:23:25Z

src/utils/webrtc/simplewebrtc/getscreenmedia.js

-module.exports = function(mode, constraints, cb) {
+/**
+ *
+ * @param mode


Will be super to add some docs

Code that might be related:
https://github.com/HenrikJoreteg/webrtc.js/blob/f7da40fcc599e5650c210f7864b4ebeeefbbc31b/webrtc.bundle.js

nickvergessen · 2023-08-15T05:51:55Z

or backport (though any issues with bundling will be seen on sermo next day, I suppose).

This is far from a change I would put into a stable branch …

Antreesy · 2023-08-15T06:12:10Z

This is far from a change I would put into a stable branch …

I meant, that it could bring changes we will notice when backporting another fixes. I also don't mean to backport exactly this PR, and let it simmer on the master branch

danxuliu

Tested and works 👍

As a rebase is anyway needed I would suggest to squash the last commit, build: always use ESM and don't mix with CJS, into the first one (and adjust the commit description if needed), build: fix bundling issues with webrtc-adapter and attachmediastream , as calls do not work after the first commit without the changes from the last commit.

Besides that, when import attachMediaStream from 'attachmediastream was used, a mock for attachmediastream was needed in the unit tests. Now that the bundle is used the mock is no longer needed (I have not actually checked why, though; maybe because the bundle includes its own copy of webrtc-adapter or something like that, but I do not really know), so it could be removed.

src/utils/webrtc/simplewebrtc/localmedia.js

ShGKme · 2023-08-17T14:50:46Z

As a rebase is anyway needed I would suggest to squash the last commit, build: always use ESM and don't mix with CJS, into the first one (and adjust the commit description if needed), build: fix bundling issues with webrtc-adapter and attachmediastream , as calls do not work after the first commit without the changes from the last commit.

Not exactly. They are different issues.

The first commit fixes the issue we had even in Webpack, having to add the Babel plugin as a fix.

While moving from mixed modules to ESM is required for ESBuild (and Vite in the future), because it has different module interop mechanism.

ShGKme · 2023-08-17T14:53:05Z

Rebased onto master, including conflict with Fix checking if WebAssembly is supported for virtual background #10230
Removed the mock for attachmediastream mentioned by @danxuliu
Squashed fixups

danxuliu

As a rebase is anyway needed I would suggest to squash the last commit, build: always use ESM and don't mix with CJS, into the first one (and adjust the commit description if needed), build: fix bundling issues with webrtc-adapter and attachmediastream , as calls do not work after the first commit without the changes from the last commit.

Not exactly. They are different issues.

The first commit fixes the issue we had even in Webpack, having to add the Babel plugin as a fix.

While moving from mixed modules to ESM is required for ESBuild (and Vite in the future), because it has different module interop mechanism.

I have to admit that I forgot that changing require('XXX') calls to require('XXX').default calls was another option to fix the first commit (and, in fact, changing only require('./peer.js') would be needed, not the others). That is why I thought that moving from mixed modules to ESM was also a requirement to fix the issues in Webpack 🤦

Had I remembered it before I would have suggested to change require('./peer.js') to require('./peer.js').default in the first commit instead ;-) (but it would still be a nice change for correctness and future reference :-) )

In any case, tested and works 👍

- Always use ES import to import `webrtc-adapter` - Use `attachmediastream` with its build for bundlers - Remove special transpiling babel config with `add-module-exports` for importing `webrtc-adapter` Signed-off-by: Grigorii K. Shartsev <[email protected]> Signed-off-by: Daniel Calviño Sánchez <[email protected]>

It is unused after fixing errors with webrtc-adapter (attachmediastream). Signed-off-by: Grigorii K. Shartsev <[email protected]>

Signed-off-by: Grigorii K. Shartsev <[email protected]>

Without local tsconfig.json esbuild-loader is searchting for the config in any parent directory. Then server's root tsconfig is used. It is not related to Talk and requires server's node_modules to be installed. Signed-off-by: Grigorii K. Shartsev <[email protected]>

Signed-off-by: Grigorii K. Shartsev <[email protected]>

danxuliu

As agreed with @ShGKme , rebased to fix the first and last commits.

Tested and works 👍

ShGKme requested review from nickvergessen, skjnldsv, DorraJaouad and Antreesy July 26, 2023 14:00

nickvergessen added this to the 💜 Next Major (28) milestone Jul 26, 2023

nickvergessen added 2. developing technical debt feature: frontend 🖌️ "Web UI" client labels Jul 26, 2023

ShGKme force-pushed the chore/esbuild branch from 4acb8b3 to 6c18996 Compare July 28, 2023 09:41

ShGKme requested a review from danxuliu July 28, 2023 10:30

ShGKme marked this pull request as ready for review July 28, 2023 10:30

ShGKme changed the title ~~chore: use ESBuild in Webpack~~ Migrate to ESBuild in Webpack Jul 28, 2023

ShGKme removed the request for review from skjnldsv July 28, 2023 10:32

Antreesy approved these changes Jul 28, 2023

View reviewed changes

nickvergessen approved these changes Jul 28, 2023

View reviewed changes

ShGKme force-pushed the chore/esbuild branch from 6c18996 to cf609ed Compare July 28, 2023 15:10

ShGKme mentioned this pull request Jul 28, 2023

Add optional support for ESBuild nextcloud/talk-desktop#275

Merged

ShGKme marked this pull request as draft July 28, 2023 16:22

ShGKme marked this pull request as ready for review August 4, 2023 19:36

Antreesy approved these changes Aug 14, 2023

View reviewed changes

ShGKme requested a review from danxuliu August 14, 2023 15:13

danxuliu reviewed Aug 17, 2023

View reviewed changes

src/utils/webrtc/simplewebrtc/localmedia.js Outdated Show resolved Hide resolved

ShGKme force-pushed the chore/esbuild branch 2 times, most recently from 9c5f4e4 to 2ba4088 Compare August 17, 2023 14:50

ShGKme requested a review from danxuliu August 17, 2023 14:53

ShGKme added 3. to review and removed 2. developing labels Aug 17, 2023

ShGKme self-assigned this Aug 17, 2023

danxuliu reviewed Aug 17, 2023

View reviewed changes

nickvergessen requested a review from danxuliu August 18, 2023 11:25

ShGKme added 6 commits August 18, 2023 15:16

chore(deps): remove babel-plugin-add-module-exports

935c882

It is unused after fixing errors with webrtc-adapter (attachmediastream). Signed-off-by: Grigorii K. Shartsev <[email protected]>

chore(deps): add esbuild-loader

dd82801

Signed-off-by: Grigorii K. Shartsev <[email protected]>

build: use ESBuild for transpiling and minimizing in Webpack

06cdf9a

Signed-off-by: Grigorii K. Shartsev <[email protected]>

build: always use ESM and don't mix with CJS

abec32c

Signed-off-by: Grigorii K. Shartsev <[email protected]>

danxuliu force-pushed the chore/esbuild branch from 2ba4088 to abec32c Compare August 18, 2023 13:35

danxuliu approved these changes Aug 18, 2023

View reviewed changes

danxuliu enabled auto-merge August 18, 2023 13:37

danxuliu merged commit bd997d3 into master Aug 18, 2023
18 checks passed

danxuliu deleted the chore/esbuild branch August 18, 2023 16:31

nickvergessen mentioned this pull request Nov 15, 2023

Can not hear nor see people in Safari #10910

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to ESBuild in Webpack #10055

Migrate to ESBuild in Webpack #10055

ShGKme commented Jul 26, 2023 •

edited

Loading

ShGKme commented Jul 26, 2023

ShGKme commented Jul 26, 2023

Antreesy commented Jul 26, 2023

ShGKme commented Jul 27, 2023

SystemKeeper commented Jul 27, 2023

SystemKeeper commented Jul 27, 2023

SystemKeeper commented Jul 27, 2023 •

edited

Loading

ShGKme commented Jul 28, 2023

ShGKme commented Jul 28, 2023 •

edited

Loading

ShGKme commented Jul 28, 2023

SystemKeeper commented Jul 28, 2023

ShGKme commented Jul 28, 2023

Antreesy left a comment

nickvergessen left a comment

susnux commented Aug 4, 2023

ShGKme commented Aug 4, 2023

Antreesy left a comment •

edited

Loading

Antreesy Aug 14, 2023

nickvergessen commented Aug 15, 2023

Antreesy commented Aug 15, 2023

danxuliu left a comment

ShGKme commented Aug 17, 2023

ShGKme commented Aug 17, 2023

danxuliu left a comment

danxuliu left a comment

	/**
	* webrtc-adapter main module does no longer provide
	* "module.exports", which is expected by some elements using it
	* (like "attachmediastream"), so it needs to be added back with
	* a plugin.
	*/
	test: /node_modules[\\/]webrtc-adapter[\\/].*\.js$/,
	loader: 'babel-loader',
	options: {
	plugins: ['add-module-exports'],
	presets: [
	/**
	* From "add-module-exports" documentation:
	* "webpack doesn't perform commonjs transformation for
	* codesplitting. Need to set commonjs conversion."
	*/
	['@babel/env', { modules: 'commonjs' }],
	],
	},
	},

Migrate to ESBuild in Webpack #10055

Migrate to ESBuild in Webpack #10055

Conversation

ShGKme commented Jul 26, 2023 • edited Loading

☑️ Resolves

Part 1: fixing issues with webrtc-adapter and attachmediastream.

Part 2: Use ESBuild for transpiling JS instead of Terser and minimizing instead of Terser

👀 Alternative solutions

🚧 Tasks

🏁 Checklist

ShGKme commented Jul 26, 2023

ShGKme commented Jul 26, 2023

Antreesy commented Jul 26, 2023

ShGKme commented Jul 27, 2023

SystemKeeper commented Jul 27, 2023

SystemKeeper commented Jul 27, 2023

SystemKeeper commented Jul 27, 2023 • edited Loading

ShGKme commented Jul 28, 2023

ShGKme commented Jul 28, 2023 • edited Loading

ShGKme commented Jul 28, 2023

SystemKeeper commented Jul 28, 2023

ShGKme commented Jul 28, 2023

Antreesy left a comment

Choose a reason for hiding this comment

nickvergessen left a comment

Choose a reason for hiding this comment

susnux commented Aug 4, 2023

ShGKme commented Aug 4, 2023

Antreesy left a comment • edited Loading

Choose a reason for hiding this comment

Antreesy Aug 14, 2023

Choose a reason for hiding this comment

nickvergessen commented Aug 15, 2023

Antreesy commented Aug 15, 2023

danxuliu left a comment

Choose a reason for hiding this comment

ShGKme commented Aug 17, 2023

ShGKme commented Aug 17, 2023

danxuliu left a comment

Choose a reason for hiding this comment

danxuliu left a comment

Choose a reason for hiding this comment

ShGKme commented Jul 26, 2023 •

edited

Loading

Part 1: fixing issues with `webrtc-adapter` and `attachmediastream`.

SystemKeeper commented Jul 27, 2023 •

edited

Loading

ShGKme commented Jul 28, 2023 •

edited

Loading

Antreesy left a comment •

edited

Loading