ci: setup eslint workflow #231

mrstork · 2025-05-27T20:56:37Z

Use eslint setup from netlify/cli project as a base
Ignore any rules that are throwing errors, and look to resolve them once eslint is running as part of CI
Add lint workflow to CI

Part of FRB-1853

mrstork · 2025-05-27T21:02:44Z

eslint_temporary_suppressions.js

+        },
+      ],
+
+      '@typescript-eslint/array-type': 'off',


Technically a lot of these are auto-fixable, but I wanted to keep this PR as tightly-scoped to eslint changes as I could. Cleanup can be done in follow-ups.

serhalp

Thank you! Feel free to choose how you want to divide up follow-ups in separate PRs

serhalp · 2025-05-28T20:07:29Z

eslint.config.js

+// @ts-check
+import * as path from 'node:path'
+import { fileURLToPath } from 'node:url'
+import { includeIgnoreFile } from '@eslint/compat'


Suggested change

import { includeIgnoreFile } from '@eslint/compat'

This is a brand new repo, so I think it would be better to avoid using legacy compat stuff if we can help it. I'll leave another suggestion change below with how to do that.

serhalp · 2025-05-28T20:12:54Z

eslint.config.js

+  ...temporarySuppressions,
+
+  // Must be last
+  prettier,


https://eslint.org/docs/latest/use/configure/migration-guide#ignoring-files

It should just be taking the contents of .gitignore:

node_modules .env .DS_Store .netlify coverage .eslintcache .cache .vscode .fleet

and massaging it into an array of glob patterns (slightly different than ignore file patterns):

Suggested change

prettier,

prettier,

{

ignores: [

'.DS_Store',

'.cache/',

'.eslintcache',

'.netlify/',

'.vscode/',

'coverage/',

],

}

(I also removed some irrelevant/unused ones)

Will aim to do this in a follow-up, ideally this is a programatically generated list

🤔 how could it be generated? in any case, I think the code suggestion I made should work as is

serhalp · 2025-05-28T20:13:12Z

eslint.config.js

+
+export default tseslint.config(
+  // Global rules and configuration
+  includeIgnoreFile(path.resolve(__dirname, '.gitignore')),


Suggested change

includeIgnoreFile(path.resolve(__dirname, '.gitignore')),

serhalp · 2025-05-28T20:14:52Z

eslint.config.js

+  {
+    languageOptions: {
+      parserOptions: {
+        project: ['./tsconfig.base.json', './packages/*/tsconfig.json'],


It looks like this is actually dead code. The intent with a .base file is usually to be referenced in extends in each of the tsconfig.json files, but none of them do 😢.

Suggested change

project: ['./tsconfig.base.json', './packages/*/tsconfig.json'],

project: ['./packages/*/tsconfig.json'],

I'll fix that in my PR where I fix the issue with not type-checking test/ dirs.

serhalp · 2025-05-28T20:18:21Z

eslint.config.js

+  {
+    rules: {
+      'n/no-extraneous-import': 'off',
+      'n/no-extraneous-require': 'off',
+      'n/no-missing-import': 'off',
+      'n/no-missing-require': 'off',
+      'n/no-unpublished-import': 'off',
+      'n/no-unpublished-require': 'off',
+    },
+  },


Suggested change

{

rules: {

'n/no-extraneous-import': 'off',

'n/no-extraneous-require': 'off',

'n/no-missing-import': 'off',

'n/no-missing-require': 'off',

'n/no-unpublished-import': 'off',

'n/no-unpublished-require': 'off',

},

},

AFAICT this was some sort of tech debt shortcut specifically to the repo you copied this from that doesn't apply here. In fact I think these are the very rules that would have prevented undeclared dependencies.

Feel free to tackle this in a separate PR though!

... although ooh actually, let's keep the two no-unpublished rules off and leave a comment that that's covered by publint

Marking this as a follow-up item

serhalp · 2025-05-28T20:20:26Z

eslint.config.js

+      // `interface` and `type` have different use cases, allow both
+      '@typescript-eslint/consistent-type-definitions': 'off',


😶 For the record I disagree, but I know this is someone else's copy-pasted opinion and I won't bother elaborating—this isn't worth bikeshedding on, so leaving the rule off to enforce nothing seems reasonable.

Feel free to remove in a follow-up!

eslint_temporary_suppressions.js

serhalp · 2025-05-28T20:28:07Z

packages/headers/tsconfig.json

    "forceConsistentCasingInFileNames": true,
    "strict": true,
-    "skipLibCheck": true,
-    "typeRoots": ["node_modules/@types", "types"]


if you're deleting this, please also delete packages/headers/types/.gitkeep!

.github/workflows/lint.yaml

mrstork commented May 27, 2025

View reviewed changes

mrstork force-pushed the configure-eslint branch from 25fafc1 to 6ef8c5d Compare May 27, 2025 21:13

mrstork changed the title ~~Refresh project eslint setup~~ ci: setup eslint workflow May 28, 2025

mrstork force-pushed the configure-eslint branch 3 times, most recently from 0152f11 to 415e6e0 Compare May 28, 2025 12:14

mrstork marked this pull request as ready for review May 28, 2025 12:15

mrstork requested a review from a team as a code owner May 28, 2025 12:15

mrstork force-pushed the configure-eslint branch 6 times, most recently from 81287e3 to 61f8015 Compare May 28, 2025 15:18

mrstork added 7 commits May 28, 2025 11:19

ci: preliminary eslint configuration

1dfde50

chore: clean up unused ignore directives

775b947

ci: add lint workflow to ci

912aaac

ci: fix monorepo tslint configuration

c0b70c7

chore: resolve lint warning for no-unnecessary-type-assertion

6c43952

chore: resolve prefer-const lint error

3b8d8cf

chore: run prettier lint

3188198

mrstork force-pushed the configure-eslint branch from 61f8015 to a76e103 Compare May 28, 2025 15:19

chore: remove typeroots to resolve node:path warnings

4533459

mrstork force-pushed the configure-eslint branch from a76e103 to 4533459 Compare May 28, 2025 15:21

serhalp previously approved these changes May 28, 2025

View reviewed changes

Merge branch 'main' into configure-eslint

eb2a8a0

mrstork dismissed serhalp’s stale review via eb2a8a0 May 28, 2025 20:46

mrstork added 2 commits May 28, 2025 16:52

chore: rebuild per-file suppressions

68a3a00

chore: remove excessive suppressions

3aea0f3

mrstork enabled auto-merge (squash) May 28, 2025 21:02

serhalp approved these changes May 28, 2025

View reviewed changes

mrstork merged commit b4cbcea into main May 28, 2025
10 checks passed

mrstork deleted the configure-eslint branch May 28, 2025 21:04

-  prettier,
+  prettier,
+  {
+    ignores: [
+      '.DS_Store',
+      '.cache/',
+      '.eslintcache',
+      '.netlify/',
+      '.vscode/',
+      'coverage/',
+    ],
+  }

	project: ['./tsconfig.base.json', './packages/*/tsconfig.json'],
	project: ['./packages/*/tsconfig.json'],

		// `interface` and `type` have different use cases, allow both
		'@typescript-eslint/consistent-type-definitions': 'off',

ci: setup eslint workflow #231

ci: setup eslint workflow #231

Uh oh!

Conversation

mrstork commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

serhalp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mrstork commented May 27, 2025 •

edited

Loading