chore: consolidate eslint config #627

boneskull · 2023-07-21T21:42:21Z

The resulting config is my best guess at the intent of the current configuration. In addition to linting the repo (the "lint everything" commit), 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)
Upgrades @metamask/eslint-config-nodejs, 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, which the new version of @metamask/eslint-config-nodejs needs
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 npm run lint (which will also run depcheck) from the workspace root. Likewise, lint:fix runs a fix (across the whole repo), and lint:depcheck will use lerna to run lint:depcheck wherever it is defined.

boneskull · 2023-07-21T21:43:52Z

I note that depcheck is not monorepo-aware, and depending on what we want from it, could be replaced with eslint-plugin-import.

boneskull · 2023-07-21T22:30:26Z

Worth mentioning there were multiple references to eslint rules from endo's custom config in core/lib/strict-scope-terminator.js. This config doesn't appear anywhere else--I assume it was from a copy/paste--and it wasn't noticed because the aforementioned misconfiguration.

boneskull · 2023-07-25T20:36:32Z

Current dependencies on/for this PR:

main
- PR chore: replace yarn with npm #629
  - PR fix(lavamoat): allow prefixed builtins #653
  - PR chore: normalize test config #643
    - PR feat(core): add a schema for policy files #620
  - PR chore: add commit hooks #632
  - PR chore: reconfigure lerna for releases #631
  - PR chore(ci): add a release-please configuration #630
  - PR chore: consolidate eslint config #627 👈
    - PR chore(deps): remove @metamask/eslint-config-nodejs #639
    - PR feat(allow-scripts): generate & ship typescript declarations #617

This comment was auto-generated by Graphite.

naugtur · 2023-07-26T13:38:18Z

packages/lavapack/.depcheckrc

@@ -0,0 +1,3 @@
+ignores:
+  # monorepo deps
+  - "ava"


Why is this needed?

because depcheck is not monorepo-aware, ava will be referenced in e.g., lavapack/test/index.js, but it will not appear in the devDependencies of lavapack/package.json.

any dev dep in a workspace which is required from code (unsure if this applies to dev deps only referenced in e.g., package.json's scripts prop) but is only in the workspace root's dev deps would need to be in this ignore property.

boneskull · 2023-07-27T00:51:08Z

just gonna retarget this one to #629

legobeat · 2023-08-22T09:53:21Z

.eslintrc.js

+    'n/no-process-env': 0,
+    // this rule seems broken
+    'n/no-unpublished-require': 0,
+    // we should probably actually fix these three and disable this override.


q for later: What's the upside of n/no-process-exit here? Agreed on the two other.

Not sure exactly what you're asking. process.exit() can be problematic if any of your code is async, because it exits before the loop is clear. It's safer to just let the event loop die. Anyway; it's in the codebase, and I didn't want to remove it, so I disabled the rule for now.

legobeat

Needs a rebase before merge but LGTM otherwise!

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) - Upgrades `@metamask/eslint-config-nodejs`, `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`, which the new version of `@metamask/eslint-config-nodejs` needs - 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.

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`. # Conflicts: # package-lock.json # package.json

legobeat · 2023-08-22T23:30:58Z

5e27065 & 85e402f should be squashed prior to merge

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 (LavaMoat#639) Closes LavaMoat#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

boneskull requested review from naugtur and kumavis July 21, 2023 21:42

boneskull self-assigned this Jul 21, 2023

boneskull force-pushed the boneskull/consolidate-eslint branch 2 times, most recently from 570d8b4 to 939792a Compare July 21, 2023 22:27

boneskull force-pushed the boneskull/consolidate-eslint branch from 939792a to ff6f77e Compare July 25, 2023 20:36

boneskull force-pushed the boneskull/consolidate-eslint branch 2 times, most recently from d4a1560 to 95f581c Compare July 25, 2023 23:01

boneskull added the chore overhead, tests, dev env, etc. label Jul 25, 2023

boneskull force-pushed the boneskull/consolidate-eslint branch 2 times, most recently from f389c76 to 75e5442 Compare July 25, 2023 23:17

naugtur reviewed Jul 26, 2023

View reviewed changes

boneskull force-pushed the boneskull/consolidate-eslint branch from 75e5442 to c283bb0 Compare July 26, 2023 23:01

boneskull changed the base branch from main to boneskull/npm July 27, 2023 01:00

boneskull force-pushed the boneskull/consolidate-eslint branch from c283bb0 to 0f27be3 Compare July 27, 2023 01:00

This was referenced Jul 27, 2023

chore: replace yarn with npm #629

Merged

chore: reconfigure lerna for releases #631

Merged

chore: add commit hooks #632

Merged

chore(ci): add a release-please configuration #630

Merged

boneskull marked this pull request as draft July 27, 2023 07:57

boneskull force-pushed the boneskull/consolidate-eslint branch 2 times, most recently from cf45819 to 57df31b Compare July 27, 2023 19:48

boneskull marked this pull request as ready for review July 27, 2023 20:16

boneskull requested review from leotm, weizman and legobeat July 27, 2023 20:17

boneskull mentioned this pull request Aug 8, 2023

chore: normalize test config #643

Merged

boneskull force-pushed the boneskull/npm branch from 9b9495d to 62065a6 Compare August 10, 2023 21:57

boneskull force-pushed the boneskull/consolidate-eslint branch 2 times, most recently from a753724 to 62bb6ea Compare August 10, 2023 22:03

boneskull mentioned this pull request Aug 10, 2023

feat(core): add a schema for policy files #620

Closed

boneskull force-pushed the boneskull/consolidate-eslint branch from 62bb6ea to 6ec7fce Compare August 11, 2023 04:55

This was referenced Aug 15, 2023

chore(node): run tests against multiple package managers #648

Merged

fix(lavamoat): allow prefixed builtins #653

Merged

boneskull force-pushed the boneskull/npm branch from 6d08610 to f69b08b Compare August 16, 2023 22:18

boneskull force-pushed the boneskull/consolidate-eslint branch from 6ec7fce to 6657c9b Compare August 16, 2023 22:18

boneskull changed the base branch from boneskull/npm to main August 16, 2023 22:33

boneskull force-pushed the boneskull/consolidate-eslint branch 6 times, most recently from 227df10 to 2b1eb8e Compare August 21, 2023 23:23

legobeat reviewed Aug 22, 2023

View reviewed changes

legobeat approved these changes Aug 22, 2023

View reviewed changes

boneskull force-pushed the boneskull/consolidate-eslint branch from 4b5f6fc to 9f11a04 Compare August 22, 2023 21:48

boneskull added 2 commits August 22, 2023 14:49

chore: lint all the things

5e27065

boneskull force-pushed the boneskull/consolidate-eslint branch from 9f11a04 to 5e27065 Compare August 22, 2023 21:50

boneskull added 2 commits August 22, 2023 16:01

chore: lint everything again

85e402f

boneskull merged commit 1b61289 into main Aug 22, 2023
9 checks passed

boneskull deleted the boneskull/consolidate-eslint branch August 22, 2023 23:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: consolidate eslint config #627

chore: consolidate eslint config #627

boneskull commented Jul 21, 2023 •

edited

boneskull commented Jul 21, 2023

boneskull commented Jul 21, 2023 •

edited

boneskull commented Jul 25, 2023 •

edited

naugtur Jul 26, 2023

boneskull Jul 26, 2023 •

edited

boneskull commented Jul 27, 2023

legobeat Aug 22, 2023

boneskull Aug 22, 2023

legobeat left a comment

legobeat commented Aug 22, 2023

chore: consolidate eslint config #627

chore: consolidate eslint config #627

Conversation

boneskull commented Jul 21, 2023 • edited

boneskull commented Jul 21, 2023

boneskull commented Jul 21, 2023 • edited

boneskull commented Jul 25, 2023 • edited

naugtur Jul 26, 2023

Choose a reason for hiding this comment

boneskull Jul 26, 2023 • edited

Choose a reason for hiding this comment

boneskull commented Jul 27, 2023

legobeat Aug 22, 2023

Choose a reason for hiding this comment

boneskull Aug 22, 2023

Choose a reason for hiding this comment

legobeat left a comment

Choose a reason for hiding this comment

legobeat commented Aug 22, 2023

boneskull commented Jul 21, 2023 •

edited

boneskull commented Jul 21, 2023 •

edited

boneskull commented Jul 25, 2023 •

edited

boneskull Jul 26, 2023 •

edited