Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 0 additions & 145 deletions .eslintrc.js

This file was deleted.

159 changes: 159 additions & 0 deletions eslint.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
import { FlatCompat } from "@eslint/eslintrc"
import path from "path"
import { fileURLToPath } from "url"
import styledComponentsA11y from "eslint-plugin-styled-components-a11y"
import importPlugin from "eslint-plugin-import"
import * as mdxPlugin from "eslint-plugin-mdx"
import testingLibraryPlugin from "eslint-plugin-testing-library"
import typescriptEslint from "@typescript-eslint/eslint-plugin"
import jsxA11yPlugin from "eslint-plugin-jsx-a11y"

const __filename = fileURLToPath(import.meta.url)
const __dirname = path.dirname(__filename)

const compat = new FlatCompat({
baseDirectory: __dirname,
recommendedConfig: {},
allConfig: {},
})

const restrictedImports = ({ paths = [], patterns = [] } = {}) => ({
"@typescript-eslint/no-restricted-imports": [
"error",
{
paths: [
/**
* No direct imports from large "barrel files". They make Jest slow.
*
* For more, see:
* - https://github.com/jestjs/jest/issues/11234
* - https://github.com/faker-js/faker/issues/1114#issuecomment-1169532948
*/
{
name: "@faker-js/faker",
message: "Please use @faker-js/faker/locale/en instead.",
allowTypeImports: true,
},
{
name: "@mui/material",
message: "Please use @mui/material/<COMPONENT_NAME> instead.",
allowTypeImports: true,
},
...paths,
],
patterns: [...patterns],
},
],
})

export default [
// Global ignores
{
ignores: ["**/build/**"],
},

// Convert legacy configs to flat config - apply to all files
...compat.extends("eslint-config-mitodl"),
...compat.extends("eslint-config-prettier"),

// Base configuration for all files
{
files: ["**/*.{js,jsx,ts,tsx}"],
plugins: {
"@typescript-eslint": typescriptEslint,
"styled-components-a11y": styledComponentsA11y,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mentioned that you thought the accessibility linting got weaker. I believe that's because we used to use the recommended configs for jsxA11y and styled-components-a11y, but we aren't anymore.

We can do:

diff --git a/eslint.config.ts b/eslint.config.ts
index 6250f8c..f47c04f 100644
--- a/eslint.config.ts
+++ b/eslint.config.ts
@@ -1,12 +1,13 @@
 import { FlatCompat } from "@eslint/eslintrc"
 import path from "path"
 import { fileURLToPath } from "url"
-import styledComponentsA11y from "eslint-plugin-styled-components-a11y"
+import styledA11y from "eslint-plugin-styled-components-a11y"
 import importPlugin from "eslint-plugin-import"
 import * as mdxPlugin from "eslint-plugin-mdx"
 import testingLibraryPlugin from "eslint-plugin-testing-library"
 import typescriptEslint from "@typescript-eslint/eslint-plugin"
-import jsxA11yPlugin from "eslint-plugin-jsx-a11y"
+import jsxA11y from "eslint-plugin-jsx-a11y"
+import { defineConfig } from "eslint/config"

 const __filename = fileURLToPath(import.meta.url)
 const __dirname = path.dirname(__filename)
@@ -55,20 +56,20 @@ export default [
   // Convert legacy configs to flat config - apply to all files
   ...compat.extends("eslint-config-mitodl"),
   ...compat.extends("eslint-config-prettier"),
-
   // Base configuration for all files
   {
     files: ["**/*.{js,jsx,ts,tsx}"],
     plugins: {
       "@typescript-eslint": typescriptEslint,
-      "styled-components-a11y": styledComponentsA11y,
+      "styled-components-a11y": styledA11y,
       import: importPlugin,
       mdx: mdxPlugin,
       "testing-library": testingLibraryPlugin,
-      "jsx-a11y": jsxA11yPlugin,
+      "jsx-a11y": jsxA11y,
     },
     settings: {
       "jsx-a11y": {
+        ...jsxA11y.flatConfigs.recommended.settings,
         components: {
           "ListCard.Image": "img",
           "Card.Image": "img",
@@ -78,8 +79,13 @@ export default [
           ActionButtonLink: "a",
         },
       },

(I changed the names to be shorter and match the READMEs for those pckages).

Maybe defineConfig? I was just reading https://eslint.org/blog/2025/03/flat-config-extends-define-config-global-ignores/ and all the eslint doc examples seem to use defineConfig now. It supports extends, which might make the above simpler. Plus is has typechecking.

import: importPlugin,
mdx: mdxPlugin,
"testing-library": testingLibraryPlugin,
"jsx-a11y": jsxA11yPlugin,
},
settings: {
"jsx-a11y": {
components: {
"ListCard.Image": "img",
"Card.Image": "img",
Button: "button",
ButtonLink: "a",
ActionButton: "button",
ActionButtonLink: "a",
},
},
},
rules: {
...restrictedImports(),
"react/display-name": [2, {}],
// This rule is disabled in the default a11y config, but unclear why.
// It does catch useful errors, e.g., buttons with no text or label.
// If it proves to be flaky, we can find other ways to check for this.
// We need both rules below. One for normal elements, one for styled
"jsx-a11y/control-has-associated-label": ["error"],
"styled-components-a11y/control-has-associated-label": ["error"],
"jsx-a11y/anchor-has-content": ["error"],
"@typescript-eslint/triple-slash-reference": [
"error",
{
path: "never",
types: "prefer-import",
lib: "never",
},
],
"import/no-extraneous-dependencies": [
"error",
{
devDependencies: [
"**/*.test.ts",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should add eslint.config.ts to this, I think.

"**/*.test.tsx",
"**/src/setupJest.ts",
"**/jest-setup.ts",
"**/jsdom-extended.ts",
"**/test-utils/**",
"**/test-utils/**",
"**/webpack.config.js",
"**/webpack.exports.js",
"**/postcss.config.js",
"**/*.stories.ts",
"**/*.stories.tsx",
"**/*.mdx",
"vite.config.mts",
".storybook/**",
],
},
],
"import/no-duplicates": "error",
quotes: ["error", "double", { avoidEscape: true }],
"no-restricted-syntax": [
"error",
/**
* See https://eslint.org/docs/latest/rules/no-restricted-syntax
*
* The selectors use "ES Query", a css-like syntax for AST querying. A
* useful tool is https://estools.github.io/esquery/
*/
{
selector:
"Property[key.name=fontWeight][value.raw=/\\d+/], TemplateElement[value.raw=/font-weight: \\d+/]",
message:
"Do not specify `fontWeight` manually. Prefer spreading `theme.typography.subtitle1` or similar. If you MUST use a fontWeight, refer to `fontWeights` theme object.",
},
{
selector:
"Property[key.name=fontFamily][value.raw=/Neue Haas/], TemplateElement[value.raw=/Neue Haas/]",
message:
"Do not specify `fontFamily` manually. Prefer spreading `theme.typography.subtitle1` or similar. If using neue-haas-grotesk-text, this is ThemeProvider's default fontFamily.",
},
],
},
},

// Test files configuration
{
files: ["./**/*.test.{ts,tsx}"],
...compat.extends("eslint-config-mitodl/jest")[0],
plugins: {
"testing-library": testingLibraryPlugin,
},
rules: {
"testing-library/no-node-access": "off",
},
},
]
6 changes: 5 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
"@chromatic-com/storybook": "^3.0.0",
"@emotion/react": "^11.11.1",
"@emotion/styled": "^11.11.0",
"@eslint/eslintrc": "^3.2.0",
"@faker-js/faker": "^9.0.0",
"@jest/environment": "^29.7.0",
"@mui/lab": "6.0.0-dev.240424162023-9968b4889d",
Expand Down Expand Up @@ -110,7 +111,7 @@
"@types/react-dom": "^19.0.0",
"@typescript-eslint/eslint-plugin": "^8.13.0",
"@typescript-eslint/typescript-estree": "^8.13.0",
"eslint": "8.57.1",
"eslint": "9.29.0",
"eslint-config-mitodl": "^2.1.0",
"eslint-config-prettier": "^10.0.0",
"eslint-import-resolver-typescript": "^4.0.0",
Expand Down Expand Up @@ -155,5 +156,8 @@
"workerDirectory": [
"storybook-public"
]
},
"resolutions": {
"jiti": "^2.0.0"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we added a resolution because:

I don't think this should require a resolution. Since ESLint has it as a peer dep, we should add it explicitly (as a dev dep). Storybook's version should get stuffed into node_modules/storybook/nextjs/node_modules, or something.

}
}
1 change: 0 additions & 1 deletion src/bundles/AiDrawer/AiDrawerManager.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-disable react-hooks/rules-of-hooks */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe missing recommended config for hooks?

import * as React from "react"
import type { Meta, StoryObj } from "@storybook/react"
import invariant from "tiny-invariant"
Expand Down
1 change: 0 additions & 1 deletion src/components/AiChat/AiChat.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// This was giving false positives
/* eslint-disable testing-library/await-async-utils */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe missing a recommended config from testing library, too.

import { render, screen, waitFor } from "@testing-library/react"
import user from "@testing-library/user-event"
import { AiChat, replaceMathjax } from "./AiChat"
Expand Down
9 changes: 8 additions & 1 deletion src/components/Checkbox/Checkbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,28 +106,35 @@ const Checkbox: React.FC<CheckboxProps> = ({
className,
disabled = false,
}) => {
const checkboxId = React.useId()
const labelId = React.useId()

return (
<Container className={className}>
{label ? (
<label>
<label htmlFor={checkboxId} id={labelId}>
<input
id={checkboxId}
type="checkbox"
name={name}
value={value}
checked={checked}
onChange={onChange}
disabled={disabled}
aria-labelledby={labelId}
/>
<span className="checkbox-label">{label}</span>
</label>
) : (
<input
id={checkboxId}
type="checkbox"
name={name}
value={value}
checked={checked}
onChange={onChange}
disabled={disabled}
aria-label={`Checkbox ${name || value || ""}`.trim() || "Checkbox"}
/>
)}
</Container>
Expand Down
1 change: 0 additions & 1 deletion src/components/Input/Input.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ export const States: Story = {
<Grid size={{ xs: 8 }}>
<Input
// This is a story just demonstrating the autofocus prop
// eslint-disable-next-line jsx-a11y/no-autofocus
autoFocus
{...args}
/>
Expand Down
Loading