Unify transpilation of nextcloud JS libraries

susnux commented 1 year ago

Goal

Unify the way Javascript and Typescript libraries are transpiled and provided.
Prevent compatibility issues of (3rd party) dependencies and out supported browsers.

Background

Currently there are a bunch of libraries provided as is without being transpiled by babel, while other apps are transpiled using the nextcloud babel configuration. (This might origin from when we had supported ES5 browsers, but all supported browsers now support ES6+) Libraries are then bundled by apps which transpile their sources and might or might not transpile their dependencies.

So currently most apps exclude babel transpilation on node_modules, as done by the default @nextcloud/webpack-vue-config: https://github.com/nextcloud/webpack-vue-config/blob/53df7bbe61e4a6d960c1d13e63597ce361f3795d/rules.js#L39

But this might lead to issues when bundling for browsers, as now all dependencies, event 3rd party, are required to be already transpiled to work with currently supported browsers. Meaning app developers will have to check each 3rd party dependency if it is transpiled to some supported version.

> libs do not pass anything through babel (despite having an unnecessary babel.config.js present) > > * https://github.com/nextcloud/nextcloud-cypress/blob/master/rollup.config.mjs > * https://github.com/nextcloud/nextcloud-files/blob/master/rollup.config. > * https://github.com/nextcloud/nextcloud-upload/blob/master/rollup.config.js > * https://github.com/nextcloud/nextcloud-auth/blob/master/rollup.config.js > * https://github.com/nextcloud/nextcloud-event-bus/blob/master/rollup.config.js > * https://github.com/nextcloud/nextcloud-axios/blob/master/rollup.config.js > * https://github.com/nextcloud/nextcloud-initial-state/blob/master/rollup.config.js > > Transpiled by babel: > > * https://github.com/nextcloud/nextcloud-dialogs/blob/master/rollup.config.mjs > * https://github.com/nextcloud/nextcloud-logger/blob/master/package.json > * https://github.com/nextcloud/nextcloud-l10n/blob/master/package.json > * https://github.com/nextcloud/nextcloud-browser-storage/blob/master/package.json > * https://github.com/nextcloud/nextcloud-sharing/blob/main/package.json > * https://github.com/nextcloud/nextcloud-router/blob/master/package.json > * https://github.com/nextcloud/nextcloud-capabilities/blob/master/package.json > * https://github.com/nextcloud/nextcloud-paths/blob/master/package.json

Options

Doing nothing is not an option, as we provide the webpack configuration doing no transformation on app dependencies, while at the same time providing libraries which might be not compatible with our target browsers is very inconsistent.

Transpile our libraries using Babel

No huge build time for apps, but only increase library build time (a bit)
Users can use the libraries without worrying about supported browser
- But still need to check other 3rd party dependencies (e.g. exclude only /node_modules/@nextcloud)

Do not transpile[1] libraries

A lot faster build times
Small bundle size (no polyfills within every library but only in the final app)
Probably better treeshaking (
No need to rebuild after browserslist is updated
We need to enforce apps to transpile all dependencies

footnotes

[1]: Typescript libraries would still need to be transpiled so a target version of Javascript has to be defined, this should be the supported version of the current node version, for Node 16 this is es2022.

Origin: This issue came up in the discussion of https://github.com/nextcloud/nextcloud-l10n/pull/535 ( @skjnldsv )

susnux commented 1 year ago

Thoughts

From my point of view libraries should not be transpiled, only if written in Typescript (then transpiled to ES version supported by our Node version (currently ES2022)), as I would always recommend transpiling all dependencies to make sure there is no compatibility issue.

This way we could provide smaller bundles as we only need to include polyfills once in the apps and do not have them duplicated within all dependencies / libraries. Moreover I would recommend simply using ESBuildMinifyPlugin on Apps, which is a lot faster then running babel-loader on every dependency, of cause one would need to include polyfills as well, but this can be handled by webpack-plugin-corejs. See build time and size comparison data below.

But on a long term I would love to see nextcloud support ES6 modules as scripts, all of out target browser support ES6 natively, the only problem is nextcloud-server which does not support adding ES6 scripts (missing type="module" on script tag). See also comparison data below why this would yield much better performance.

Build time and size comparison

(based on the `forms` app as this is a mid-size nextcloud application) | Tools | Build time | Output filesize (sum) | |---------|-----------------|--------------------------------| | **current**
webpack + babel (**only own source**) + terser | 21065 ms | 26580 KiB | | webpack + babel (everything) + terser | 25375 ms | 22356 KiB | | webpack + esbuild + terser | 20581 ms | 26688 KiB | | webpack + esbuild | 12811 ms | 28260 KiB | | webpack + swc | 13753 ms | 27500 KiB | | **vite** (ES6) | **7564 ms** | **11476 KiB** |

skjnldsv commented 1 year ago

From my point of view libraries should not be transpiled, only if written in Typescript (then transpiled to ES version supported by our Node version (currently ES2022)), as I would always recommend transpiling all dependencies to make sure there is no compatibility issue.

I second that. It will also cleanup our code and libraries boilerplate

But on a long term I would love to see nextcloud support ES6 modules as scripts, all of out target browser support ES6 natively, the only problem is nextcloud-server which does not support adding ES6 scripts (missing type="module" on script tag).

A different discussion, but seeing the table you posted, oh man, sign me up for webpack+swc already! :scream: The difference with what we do is really manageable I'd say

nextcloud / standards