Add rules for hybrid Node.js and browser environments

[Mrtenz]

This adds a new package for hybrid Node.js and browser envrionments. It prevents the use of any Node.js or browser-specific globals.

Because we make use of Node.js and browser APIs in many modules right now, adding this to an existing package would cause a lot of breaking changes. That's why I created a new package instead. It includes a script to update the rules automatically.

Using the following file as example:

import { createHash } from 'crypto';
import { join } from 'path';

export const doThing = () => {
  const value = Buffer.from('foo bar');
  createHash('sha256').update(value).digest('hex');
  join('foo');
};

Results in these errors:

  1:1   error  'crypto' import is restricted from being used. Use 'ethereum-cryptography' instead        no-restricted-imports
  2:1   error  'path' import is restricted from being used. This module is not available in the browser  no-restricted-imports
  5:17  error  Unexpected use of 'Buffer'. Use 'Uint8Array' instead                                      no-restricted-globals

To do

• Disable rules in test files?
• Test in production to ensure it works as expected.

[Mrtenz]

I'm not sure what the best way would be to handle test files. For testing in production, I simply added this package to the *.ts override, which would also apply it to test files. We could set it up in the module template to turn off those rules for *.test.ts maybe.

[Gudahtt]

Could we add this to the base package instead? That package is already meant to prevent the use of anything Node.js or browser-specific.

I'm still confused as to why Node.js specific globals aren't already disallowed though 🤔 The base package uses no-undef, and doesn't use an environment that includes globals like Buffer.

[Mrtenz]

Could we add this to the base package instead? That package is already meant to prevent the use of anything Node.js or browser-specific.

I'm afraid that it will cause many breaking changes in projects, preventing them from upgrading to the latest config. Fixing these errors is not as simple as changing one line of code, some possibly requiring a larger refactor (like swapping out Buffer for Uint8Array and crypto with a browser-friendly version).

It might be better to have it as a new package, wait until most of our modules have implemented it, and then merge it with the base rules.

I'm still confused as to why Node.js specific globals aren't already disallowed though 🤔 The base package uses no-undef, and doesn't use an environment that includes globals like Buffer.

It seems to be overridden somewhere. Explicitly adding /* eslint no-undef: "error" */ to a file does cause it to error when using Buffer. no-undef doesn't solve the problem for Node.js built-in modules though.

[Gudahtt]

I think this is the culprit: https://github.com/typescript-eslint/typescript-eslint/blob/efc147b04dce52ab17415b6a4ae4076b944b9036/packages/eslint-plugin/src/configs/eslint-recommended.ts

It's disabled because TypeScript is already checking for undefined types. But TypeScript isn't seeing it as undefined because we have @types/node installed (presumably so that we can use Node.js APIs in tests?). Not sure what the best way to solve this would be - either explicitly re-add no-undef in our TypeScript ESLint config, or ask TypeScript to not include @types/node when checking the production files by default.

[Gudahtt]

I'm afraid that it will cause many breaking changes in projects, preventing them from upgrading to the latest config. Fixing these errors is not as simple as changing one line of code, some possibly requiring a larger refactor (like swapping out Buffer for Uint8Array and crypto with a browser-friendly version).

In the past, we've handled this by disabling disruptive rules in the project ESLint config to make the update easier. Then we can re-enable any rules one-by-one to fix lint errors. We're made many disruptive lint changes recently, and many more are planned (e.g. enabling the ESLint TypeScript rules that require type information); making a new package each time would be confusing.

[Gudahtt]

Not sure what the best way to solve this would be - either explicitly re-add no-undef in our TypeScript ESLint config, or ask TypeScript to not include @types/node when checking the production files by default.

I looked into this, and couldn't find a reasonable way of excluding @types/node from tsconfig.build.json specifically, while including it in tsconfig.json.

So I guess we should explicitly add no-undef to our TypeScript ESLint config.