Update ESLint config and apply changes #5599

flozia · 2025-02-07T18:12:49Z

References:

Description

This PR does a couple of things:

Unblocks updating the eslint group (PR chore(deps-dev): bump the eslint group across 1 directory with 7 updates #5590)
Migrates and update our ESLint config from .eslintrc.json to eslint.config.js
Applies the changes to our codebase
Adds the commit 9d28f6b to .git-blame-ignore-revs which includes most of the automatically applied changes by Prettier.

How to test

npm install
npm run lint

Bumps the eslint group with 7 updates in the / directory: | Package | From | To | | --- | --- | --- | | [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin) | `7.5.0` | `8.23.0` | | [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser) | `7.5.0` | `8.23.0` | | [eslint](https://github.com/eslint/eslint) | `8.57.0` | `9.19.0` | | [eslint-import-resolver-typescript](https://github.com/import-js/eslint-import-resolver-typescript) | `3.6.1` | `3.7.0` | | [eslint-plugin-check-file](https://github.com/dukeluo/eslint-plugin-check-file) | `2.7.1` | `3.0.0` | | [eslint-plugin-jest](https://github.com/jest-community/eslint-plugin-jest) | `27.9.0` | `28.11.0` | | [eslint-plugin-jsdoc](https://github.com/gajus/eslint-plugin-jsdoc) | `48.2.2` | `50.6.3` | Updates `@typescript-eslint/eslint-plugin` from 7.5.0 to 8.23.0 - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v8.23.0/packages/eslint-plugin) Updates `@typescript-eslint/parser` from 7.5.0 to 8.23.0 - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/parser/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v8.23.0/packages/parser) Updates `eslint` from 8.57.0 to 9.19.0 - [Release notes](https://github.com/eslint/eslint/releases) - [Changelog](https://github.com/eslint/eslint/blob/main/CHANGELOG.md) - [Commits](eslint/eslint@v8.57.0...v9.19.0) Updates `eslint-import-resolver-typescript` from 3.6.1 to 3.7.0 - [Release notes](https://github.com/import-js/eslint-import-resolver-typescript/releases) - [Changelog](https://github.com/import-js/eslint-import-resolver-typescript/blob/master/CHANGELOG.md) - [Commits](import-js/eslint-import-resolver-typescript@v3.6.1...v3.7.0) Updates `eslint-plugin-check-file` from 2.7.1 to 3.0.0 - [Release notes](https://github.com/dukeluo/eslint-plugin-check-file/releases) - [Changelog](https://github.com/dukeluo/eslint-plugin-check-file/blob/main/CHANGELOG.md) - [Commits](dukeluo/eslint-plugin-check-file@v2.7.1...v3.0.0) Updates `eslint-plugin-jest` from 27.9.0 to 28.11.0 - [Release notes](https://github.com/jest-community/eslint-plugin-jest/releases) - [Changelog](https://github.com/jest-community/eslint-plugin-jest/blob/main/CHANGELOG.md) - [Commits](jest-community/eslint-plugin-jest@v27.9.0...v28.11.0) Updates `eslint-plugin-jsdoc` from 48.2.2 to 50.6.3 - [Release notes](https://github.com/gajus/eslint-plugin-jsdoc/releases) - [Changelog](https://github.com/gajus/eslint-plugin-jsdoc/blob/main/.releaserc) - [Commits](gajus/eslint-plugin-jsdoc@v48.2.2...v50.6.3) --- updated-dependencies: - dependency-name: "@typescript-eslint/eslint-plugin" dependency-type: direct:development update-type: version-update:semver-major dependency-group: eslint - dependency-name: "@typescript-eslint/parser" dependency-type: direct:development update-type: version-update:semver-major dependency-group: eslint - dependency-name: eslint dependency-type: direct:development update-type: version-update:semver-major dependency-group: eslint - dependency-name: eslint-import-resolver-typescript dependency-type: direct:development update-type: version-update:semver-minor dependency-group: eslint - dependency-name: eslint-plugin-check-file dependency-type: direct:development update-type: version-update:semver-major dependency-group: eslint - dependency-name: eslint-plugin-jest dependency-type: direct:development update-type: version-update:semver-major dependency-group: eslint - dependency-name: eslint-plugin-jsdoc dependency-type: direct:development update-type: version-update:semver-major dependency-group: eslint ... Signed-off-by: dependabot[bot] <[email protected]>

github-actions · 2025-02-07T18:29:44Z

Preview URL 🚀 : https://blurts-server-pr-5599-mgjlpikfea-uk.a.run.app

github-actions · 2025-02-07T18:29:44Z

Preview URL 🚀 : https://blurts-server-pr-5599-mgjlpikfea-uk.a.run.app

rhelmer

A few comments but lgtm, thanks for unblocking this eslint upgrade!

rhelmer · 2025-02-07T19:22:14Z

eslint.config.js

+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+


According to eslint/eslint#17726 (reply in thread) (I haven't verified further), Typescript is an available option for ESLint configuration now.

Only from ESLint 9.9.0 onwards, unfortunately. The dependency update PR #5590 is bumping ESLint from 8.57.0 to 9.19.0.

rhelmer · 2025-02-07T19:27:33Z

src/utils/email.test.ts

@@ -67,7 +67,7 @@ test("EmailUtils.init with SMTP URL invokes nodemailer.createTransport", async (

  const result = await initEmail(testSmtpUrl);
  expect(mockedNodemailer.createTransport).toHaveBeenCalledWith(testSmtpUrl);
-  // eslint-disable-next-line @typescript-eslint/unbound-method


So nice being able to remove these!

rhelmer · 2025-02-07T20:10:43Z

src/app/api/v1/admin/feature-flags/route.ts

@@ -24,7 +24,7 @@ export async function GET() {
    try {
      const flags = await getAllFeatureFlags();
      return NextResponse.json(flags);
-    } catch (e) {


I know you didn't introduce this and I'm fine with the change as-is, but I think there's value in logging these messages so we can tell exactly what went wrong when diagnosing. I don't think that it should be returned to the caller or anything.

I'm not sure how to tackle this, we might want a tech debt ticket to audit our existing exception handling. wdyt?

I think that’s a good idea — created MNTOR-4047.

Vinnl

Huh, that's fewer changes than I expected. (Probably still lot of effort though :) ) Nice work!

eslint.config.js

Vinnl · 2025-02-10T12:52:29Z

eslint.config.js

+        project: "tsconfig.json",
+      },
+    },
+    ignores: ["coverage", "dist", "scripts", "sentry.*.config.ts"],


Out of scope for this PR, but in case it's super easy: scripts is probably too broad, since it also includes our cron jobs. Could possibly be limited to src/scripts/loadtest/?

Oh, I see now it did run against some of the files in there? 🤔

I was confused with this at first: We had ./scriptswhich has been removed completely so I also removed it from the list of ignored files here. This is the updated ignore list src/scripts and I agree that it is a bit too general. I wanted to limit the changes introduced by this PR and will address this in a follow-up.

Vinnl · 2025-02-10T12:55:03Z

jest.setup.ts

@@ -49,7 +49,6 @@ global.TextEncoder = TextEncoder;

 // Jest doesn't like the top-level await in envVars.ts, so we mock it.
 jest.mock("./src/envVars", () => {
-  // eslint-disable-next-line @typescript-eslint/no-var-requires


Hmm, from what I can see this rule has been replaced by this rule in the recommended set, which I'd expect to still trigger here. Could that indicate that we're not running ESLint on all TS files anymore?

Good catch! Actually, the linter did complain about this line: Fixed in a43708c.

Oh? Hmm, then my followup question is: why don't I see any failed jobs here? Shouldn't the linter have complained about it in CI as well?

It seems like it never failed on jest.setup.ts. I’ve gone back to before the commits in this PR and checked (introduced intentional issues an ran the linter). I’m working on a follow-up to narrow down the ignored files even more and also to make sure that the linting issues in the config and setup files are blocking.

Follow-up: I’ve created PR #5619 to address next lint not running on the root files.

src/app/(proper_react)/(redesign)/(authenticated)/admin/emails/actions.tsx

…er into update-eslint-config

github-actions · 2025-02-10T15:36:49Z

Cleanup completed - database 'blurts-server-pr-5599' destroyed, cloud run service 'blurts-server-pr-5599' destroyed

dependabot bot and others added 8 commits February 5, 2025 14:04

Merge branch 'main' into dependabot/npm_and_yarn/eslint-4602aa654d

06be2f4

chore: Migrate ESLint config from .eslintrc.json to eslint.config.js

0201395

fix: Optionally include sharp

8b1894c

fix: Default export

f0f16da

chore: Update eslint config

fe45528

fix: Linter errors and warnings

49a5614

chore: Format some of the previously ignored JS files

9d28f6b

flozia changed the title ~~WIPUpdate ESLint config and apply changes~~ WIP: Update ESLint config and apply changes Feb 7, 2025

flozia marked this pull request as draft February 7, 2025 18:14

flozia changed the base branch from main to dependabot/npm_and_yarn/eslint-4602aa654d February 7, 2025 18:15

flozia changed the base branch from dependabot/npm_and_yarn/eslint-4602aa654d to main February 7, 2025 18:15

chore: Add commit 9d28f6b to .git-blame-ignore-revs

d0d6d5f

flozia changed the title ~~WIP: Update ESLint config and apply changes~~ Update ESLint config and apply changes Feb 7, 2025

flozia marked this pull request as ready for review February 7, 2025 18:26

flozia marked this pull request as draft February 7, 2025 18:27

chore: Remove some pre-migration files from .prettierignore

31f5a7f

flozia force-pushed the update-eslint-config branch from 9a4d79d to 31f5a7f Compare February 7, 2025 18:32

flozia marked this pull request as ready for review February 7, 2025 18:33

flozia requested review from Vinnl, rhelmer and mansaj February 7, 2025 18:33

rhelmer approved these changes Feb 7, 2025

View reviewed changes

flozia self-assigned this Feb 8, 2025

Vinnl approved these changes Feb 10, 2025

View reviewed changes

flozia added 3 commits February 10, 2025 16:14

fix: Linter for jest.seup.ts

4bb245f

fix: Linter for jest.setup.ts

a43708c

Merge branch 'update-eslint-config' of github.com:mozilla/blurts-serv…

1860977

…er into update-eslint-config

merge: main -> update-eslint-config

29b8b23

flozia merged commit a6a5495 into main Feb 10, 2025
16 checks passed

flozia deleted the update-eslint-config branch February 10, 2025 15:34

flozia mentioned this pull request Feb 12, 2025

Run ESLint on root files #5619

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update ESLint config and apply changes #5599

Update ESLint config and apply changes #5599

flozia commented Feb 7, 2025 •

edited

Loading

github-actions bot commented Feb 7, 2025

github-actions bot commented Feb 7, 2025

rhelmer left a comment

rhelmer Feb 7, 2025

flozia Feb 10, 2025

rhelmer Feb 7, 2025

rhelmer Feb 7, 2025

rhelmer Feb 7, 2025

flozia Feb 10, 2025

Vinnl left a comment

Vinnl Feb 10, 2025

Vinnl Feb 10, 2025

flozia Feb 10, 2025

Vinnl Feb 10, 2025

flozia Feb 10, 2025

Vinnl Feb 10, 2025

flozia Feb 10, 2025 •

edited

Loading

flozia Feb 12, 2025

github-actions bot commented Feb 10, 2025

Update ESLint config and apply changes #5599

Update ESLint config and apply changes #5599

Conversation

flozia commented Feb 7, 2025 • edited Loading

References:

Description

How to test

github-actions bot commented Feb 7, 2025

github-actions bot commented Feb 7, 2025

rhelmer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vinnl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flozia Feb 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 10, 2025

flozia commented Feb 7, 2025 •

edited

Loading

flozia Feb 10, 2025 •

edited

Loading