MetaMask / metamask-module-template

A simple template repository for starting new modules in the latest MetaMask fashion.
28 stars 23 forks source link

Consider preventing the use of Node.js built-in modules and globals by default #127

Open Mrtenz opened 1 year ago

Mrtenz commented 1 year ago

Most modules we develop need to be browser-compatible. The use of Node.js built-in modules means that some kind of polyfill is required for those modules. Usually this means using polyfills like crypto-browserify (hasn't been updated in years, and the bundle is quite large), buffer, etc., while there are often better alternatives available that have a much smaller bundle size (e.g., the noble and scure libraries), which offer compatibility with both Node.js and browsers by default.

Working with Uint8Array and bigint instead of Buffer and BN.js is quite easy now, since we have a lot of util functions in @metamask/utils. Most other Buffer-specific functions, like writeUint...() can be replaced with a DataView (available on browsers and Node.js). See MetaMask/key-tree#83 for example.

For that reason I propose we add some ESLint rules that prevent the use of built-in Node.js modules (mainly crypto), and Node.js-specific globals (mainly Buffer).

There are some valid use-cases for using Node.js built-in modules (like Node.js-only modules interacting with the file system), but projects needing it can simply remove or disable those rules.

Gudahtt commented 1 year ago

Our base ESLint config should already prevent using Node.js or browser-specific APIs (here).

They are allowed for JavaScript files because this template is meant for TypeScript libraries, and what few JavaScript modules we have tend to be build-related, rather than part of the library.

Maybe I'm mistaken though; where have you seen that Node.js APis are allowed where they shouldn't be?

Mrtenz commented 1 year ago

Those rules don't seem to be applied to the module template. As a test I added this file, which does not cause any errors upon running yarn lint.

mcmire commented 11 months ago

Revisiting this. Does this issue still exist?