Skip to content

Commit

Permalink
chore: consolidate eslint config (#627)
Browse files Browse the repository at this point in the history
This change:

- Removes all `.eslintignore` , `.eslintrc` files or `package.json`-embedded config and replaces it with a single root `.eslintrc.js`
- Removes all lint-related scripts from all workspaces (including `depcheck`)
- Removes all dev deps on `eslint` and its ilk (and `ava`, since it will come from the workspace root)
- Upgrade `eslint`, `eslint-plugin-ava`, as well as other plugins used by specific packages; moves them all to workspace root
- Removes unused/deprecated `eslint-plugin-node` in lieu of `eslint-plugin-n`
- Adds a prettier config (in case someone wants to use it; I'd like to add it to our workflow later)

I note that ESLint seemed to be misconfigured before this change; none of the rules specified in the root config were being checked.  This means that there are a whole lot of lint fixes that need to be made.

To lint, run `yarn lint` as before (which will also run `depcheck`) from the workspace root.  Likewise, `lint:fix` runs a fix, and `lint:depcheck` will use lerna to run `lint:depcheck` wherever it is defined.

chore: remove @metamask/eslint-config-nodejs (#639)

Closes #637

This removes the dep on `@metamask/eslint-config-nodejs` which has a peer dep on `@metamask/eslint-config`.  `npm` will automatically install that module, and that module _itself_ has peer deps which may also conflict with our deps currently or in the future.

Given `@metamask/eslint-config-nodejs` is _intended_ to be used with `@metamask/eslint-config`, we then must drop it.

The ESLint config has been modified to extend both `eslint/recommended` and `eslint-plugin-n/recommended` configs.  Extra rules were copied from `@metamask/eslint-config-nodejs`, but many (such as restrictions on browser-only globals) were not, since these can be trivially addressed by ESLint's `env` configuration (unsure what they are doing where this is a problem).  Since this surfaced a whole lot of new issues--especially in test code and templates--additional overrides were necessary.  Any rules which were removed from the main `rules` prop were likely already enabled in `eslint/recommended`.

chore: lint everything
  • Loading branch information
boneskull committed Aug 22, 2023
1 parent 986fc1f commit 1b61289
Show file tree
Hide file tree
Showing 127 changed files with 1,584 additions and 1,818 deletions.
177 changes: 177 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
module.exports = {
root: true,
// this is the same as an .eslintignore file
ignorePatterns: [
'.yarn/**/*',
'**/*.umd.js',
'**/node_modules/**/*',
'docs/**/*',
'packages/*/examples/**/*',
'packages/*/test/**/fixtures/**/*',
'packages/*/test/projects/**/*',
'packages/lavapack/bundle.js',
'packages/lavapack/src/runtime-cjs.js',
'packages/lavapack/src/runtime.js',
'packages/perf/trials/**/*',
'packages/survey/mitm/**/*',
'packages/viz/dist/**/*',
'packages/viz/src/example-policies/**/*',
],
parserOptions: {
// this should be whatever the latest env version provides. some plugin is
// messing with this, so we need to set it manually.
// https://eslint.org/docs/latest/use/configure/language-options#specifying-environments
ecmaVersion: 12,
},
env: { es2020: true, node: true }, // this should be synced with the version of V8 used by the min supported node version
extends: ['eslint:recommended', 'plugin:n/recommended'],
rules: {
// base rules; none of these are in eslint/recommended
'arrow-spacing': ['error', { before: true, after: true }],
'brace-style': ['error', '1tbs'],
'comma-dangle': ['error', 'always-multiline'],
curly: ['error', 'all'],
'eol-last': ['error', 'always'],
indent: ['error', 2, { SwitchCase: 1, ObjectExpression: 'first' }],
'no-empty': ['error', { allowEmptyCatch: true }],
quotes: ['error', 'single'],
semi: ['error', 'never'],
'space-before-blocks': ['error', 'always'],
'space-before-function-paren': [
'error',
{
anonymous: 'ignore',
named: 'ignore',
asyncArrow: 'always',
},
],

// additional errors not in n/recommended
'n/callback-return': 'error',
'n/handle-callback-err': 'error',
'n/no-callback-literal': 'error',
'n/no-mixed-requires': 'error',
'n/no-new-require': 'error',
'n/no-restricted-import': 'error',
'n/no-restricted-require': 'error',
'n/prefer-global/buffer': 'error',
'n/prefer-global/console': 'error',
'n/prefer-global/process': 'error',
'n/prefer-global/text-decoder': 'error',
'n/prefer-global/text-encoder': 'error',
'n/prefer-global/url-search-params': 'error',
'n/prefer-global/url': 'error',
'n/prefer-promises/dns': 'error',
'n/prefer-promises/fs': 'error',

// these rules seem broken in a monorepo
'n/no-extraneous-require': 'off',
'n/no-unpublished-require': 'off',

// we should probably actually fix these three and turn these back on
'n/no-sync': 'off', // very few reasons to actually use this; "CLI tool" is not one of them
'n/no-process-exit': 'off', // should not be used with async code
},
// these are plugin-specific settings.
settings: {
// note that this name is "node" while the plugin is named "n".
// probably for historical reasons, but this may eventually break.
node: {
version: '14.17.0', // should be set to minimum node version supported
allowModules: ['deep-equal'], // something weird about this dependency
},
react: {
version: 'detect',
},
},
overrides: [
{
files: ['packages/*/test/**/*.js', 'packages/*/src/**/*.test.js'],
extends: ['plugin:ava/recommended'],
rules: {
'ava/no-import-test-files': 'off',
},
},
{
files: ['packages/core/src/**/*.js'],
globals: {
Compartment: 'readonly',
templateRequire: 'readonly',
lockdown: 'readonly',
},
},
{
files: ['packages/*/test/**/*.js'],
env: {
browser: true,
},
rules: {
'no-unused-vars': 'off',
'no-undef': 'off',
'n/no-path-concat': 'off', // this should be removed and the issues fixed
'n/no-missing-require': 'off',
},
},
{
files: ['packages/core/lib/**/*.js'],
rules: {
'no-unused-vars': 'off',
},
},
{
files: ['packages/lavapack/src/*-template.js'],
globals: {
templateRequire: 'readonly',
self: 'readonly',
__reportStatsHook__: 'readonly',
__createKernel__: 'readonly',
},
},
{
files: ['packages/core/src/*Template.js'],
globals: {
__lavamoatDebugOptions__: 'readonly',
__lavamoatSecurityOptions__: 'readonly',
self: 'readonly',
__createKernelCore__: 'readonly',
},
},
{
files: ['packages/perf/**/*.js'],
globals: {
lockdown: 'readonly',
Compartment: 'readonly',
},
},
{
// for viz package. most of this copied out of its own eslint config
files: ['packages/viz/**/*.js'],
env: {
browser: true,
es2020: true,
},
parserOptions: {
ecmaFeatures: {
jsx: true,
},
sourceType: 'module',
},
extends: ['plugin:react/recommended'],
plugins: ['react', 'import'],
rules: {
'no-negated-condition': 'off',
'import/extensions': ['error', 'always', { ignorePackages: true }],
'import/no-unassigned-import': 'off',
'import/unambiguous': 'off',
'react/prop-types': 'off',
'n/no-missing-import': 'off',
},
},
{
files: ['packages/viz/src/App.test.js'],
env: {
mocha: true,
},
},
],
}
54 changes: 0 additions & 54 deletions .eslintrc.json

This file was deleted.

2 changes: 1 addition & 1 deletion commitlint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ module.exports = {
'subject-case': [0],
'subject-full-stop': [0],
},
};
}
Loading

0 comments on commit 1b61289

Please sign in to comment.