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

Consider removing Node built-in modules polyfills #450

Open
ShGKme opened this issue Apr 17, 2023 · 4 comments
Open

Consider removing Node built-in modules polyfills #450

ShGKme opened this issue Apr 17, 2023 · 4 comments

Comments

@ShGKme
Copy link
Contributor

ShGKme commented Apr 17, 2023

Preface

In the past Webpack polyfilled built-in Node.js modules. It was changed in Webpack 5. But some modules require built-in Node module polyfills. To fix that currently the node-polyfill-webpack-plugin is used, which returns Webpack 4 behavior.

The problem

Many Node polyfills are useless. (For example, console and Buffer are available in Web Browsers). But they still increase the bundle size and bundling time.

For example, for the Talk frontend, it adds about 189kb raw in the production build. It may sound small but:

  • JS size is more than just bytes, more than the image size. It has to be parsed, compiled, and evaluated.
  • This is just one app. If a server instance has 10 different apps installed, then it could be 10 times more.

⚠️ BREAKING CHANGE

This would be a breaking change. Any app building may break if it requires any polyfill.

To decrease the number of breaks we may provide fallback with polyfills, required by Nextcloud libs, by default: stream and path. For most app it might be enough.

resolve: {
  fallback: {
    stream: require.resolve('stream-browserify'),
    path: require.resolve('path-browserify'),
  },
}

Why do not change anything here

Nextcloud Server + apps frontend is very large, and this small change may seem like a drop in the sea. But on the other hand, it is small and simple change (with a small breaking change).

@ShGKme
Copy link
Contributor Author

ShGKme commented Apr 17, 2023

See also: #449

@juliushaertl
Copy link
Contributor

@ShGKme If Buffer really available? I noticed that with the latest @nextcloud/files i needed to add buffer and path to the resolve.fallback config to make this work in text: nextcloud/text@6935fa6

@skjnldsv
Copy link
Contributor

@ShGKme If Buffer really available? I noticed that with the latest @nextcloud/files i needed to add buffer and path to the resolve.fallback config to make this work in text: nextcloud/text@6935fa6

I had to do this to Text yesterday yes

@susnux
Copy link
Contributor

susnux commented Aug 11, 2023

Buffer is not available. Well it is but its API is different from the Node.js Buffer API.

Both share some API, but some parts do not behave the same, so basically you can not be sure 3rd party libraries only use API which works on browser and Node. Thats why you probably need to polyfill every node module (except console).

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

No branches or pull requests

4 participants