MetaMask
Only enable Node lint rules for Node files
Explanation
In a previous commit, we mistakenly introduced a Node-specific function for testing deep equality, and this ended up crashing the extension. This would usually have been caught by our ESLint rules, as they prohibit use of Node libraries by default. However, the ESLint configuration for this repo imports rules from our @metamask/eslint-config-nodejs package and applies them to all files, marking everything as Node-compatible, thereby allowing such usage. This is incorrect: these rules should only apply to test files and scripts.
This commit fix the ESLint configuration to match. Doing so revealed a couple of categories of violations, which this commit also fixes:
- Some packages import and make use of EventEmitter from Node's
eventsmodule. We add a notice to the README for these packages which advises consumers that these packages are designed to be used in a Node-compatible environment. - Some packages import and make use of Node's
assertmodule to check values at runtime. It isn't strictly necessary to use this module, and so we replace its usage with simpler code. - Some packages import and make use of Node's
inspectutility function. We don't strictly need this either, and we replace its usage with simpler code as well.
References
This issue was brought up in our company chat.
Changelog
(No functional changes to note here)
Checklist
- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate
However, the ESLint configuration for this repo imports rules from our @metamask/eslint-config-nodejs package and applies them to all files, marking everything as Node-compatible, thereby allowing such usage. This is incorrect: these rules should only apply to test files and scripts.
Does this mean that other files can be incompatible with Node.js? We use some packages in Node.js for Snaps.
Does this mean that other files can be incompatible with Node.js? We use some packages in Node.js for Snaps.
@Mrtenz Oh, sorry for the confusion, I just meant that these rules should only apply to test files and scripts in the context of this monorepo, not as a general statement for all monorepos. For Snaps, it sounds like we'd need some specific configuration to account for Node.js-compatible packages. That would make sense to me.
Does this mean that other files can be incompatible with Node.js? We use some packages in Node.js for Snaps.
The base configuration should ensure things work across Node.js and browsers. We're still applying the base config everywhere, so that should ensure compatibility with Node.js.
The Node.js rules are intended for scripts solely run in a Node.js environment, like scripts and tests.
