Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linting refactor and enable GitHub actions #3179

Merged
merged 23 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
d5c0338
lint: Upgrade to eslint 9 and refactor to enable simpler linting config
nellh Oct 22, 2024
79389b9
fix(server): Use Object.hasOwn over hasOwnProperty
nellh Oct 22, 2024
e6b3060
fix(app): Disable linting for react.d.ts type helper in tests
nellh Oct 22, 2024
58eb99b
fix(server): Prefix unreferenced validateUrl vars with _
nellh Oct 22, 2024
7d996c7
chore(server): Use mongoose types as a type import
nellh Oct 22, 2024
0928d94
chore(lint): Fix object prototype hasOwnProperty usage
nellh Oct 22, 2024
a0e36c9
chore(lint): Use object over any for model definitions
nellh Oct 22, 2024
00b718e
feat(graphql-client): Only warn once on mismatched versions
nellh Oct 22, 2024
7fbda3e
chore(lint): Don't suppress console from the CLI upload steps
nellh Oct 22, 2024
7e1eda7
chore(lint): Remove cjs import for node-mailjet
nellh Oct 22, 2024
feb1eaa
chore(lint): Indexer should print console messages
nellh Oct 22, 2024
27af188
chore(lint): Document issues with Readable interface types
nellh Oct 22, 2024
97d130e
chore(lint): Enable checking for schema.ts
nellh Oct 22, 2024
81240a6
chore(lint): Fix linter configuration for console and unused vars
nellh Oct 22, 2024
beada40
chore(lint): Fix types imported as values
nellh Oct 22, 2024
ccf90e1
fix(app): Error handling for FlaggedFileContainer query errors
nellh Oct 22, 2024
f0ab199
chore(lint): Cleanup unused variables
nellh Oct 22, 2024
9097564
refactor(app): Drop deprecated updateRef
nellh Oct 22, 2024
34a07ca
chore(lint): React type import fixes
nellh Oct 22, 2024
508a1dc
chore(lint): Linting and error handling fixes
nellh Oct 22, 2024
cf5956d
fix(app): Remove no-op value mutation TwoHandleRange change handlers
nellh Oct 23, 2024
eabf4dc
chore(lint): Remaining eslint refactor fixes
nellh Oct 23, 2024
dc70912
ci: Add ESLint step
nellh Oct 23, 2024
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
10 changes: 0 additions & 10 deletions .eslintignore

This file was deleted.

77 changes: 0 additions & 77 deletions .eslintrc.json

This file was deleted.

19 changes: 19 additions & 0 deletions .github/workflows/eslint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
name: ESLint

on:
push:
branches: [ main, master ]
pull_request:
branches: [ main, master ]

jobs:
eslint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Use Node.js
uses: actions/setup-node@v4
with:
node-version: '20.x'
- run: yarn install
- run: yarn run lint
1,799 changes: 1,067 additions & 732 deletions .pnp.cjs

Large diffs are not rendered by default.

53 changes: 53 additions & 0 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// @ts-check
import eslint from "@eslint/js"
import tseslint from "typescript-eslint"
import reactCompiler from "eslint-plugin-react-compiler"

export default tseslint.config({
ignores: [
"**/build/**",
"**/dist/**",
"**/node_modules/**",
".yarn/*",
".pnp.cjs",
".pnp.loader.mjs",
"**/coverage/**",
"packages/openneuro-app/pluralize-esm.js",
"packages/openneuro-app/src/scripts/utils/schema-validator.js",
Comment on lines +15 to +16
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be helpful to future readers to know why we're ignoring these. I think this is the reason?

Suggested change
"packages/openneuro-app/pluralize-esm.js",
"packages/openneuro-app/src/scripts/utils/schema-validator.js",
// Generated from external sources
"packages/openneuro-app/pluralize-esm.js",
"packages/openneuro-app/src/scripts/utils/schema-validator.js",

],
}, {
files: [
"packages/**/*.ts",
"packages/**/*.tsx",
"packages/**/*.jsx",
"packages/**/*.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it reasonable for this thing to lint itself?

Suggested change
"packages/**/*.js",
"packages/**/*.js",
"eslint.config.mjs",

],
extends: [
eslint.configs.recommended,
...tseslint.configs.recommended,
],
rules: {
"@typescript-eslint/array-type": "error",
"@typescript-eslint/consistent-type-imports": "error",
"no-console": "error",
"@typescript-eslint/no-unused-vars": [
"error",
{
"argsIgnorePattern": "^_",
"varsIgnorePattern": "^_",
"caughtErrorsIgnorePattern": "^_",
},
],
},
}, {
files: ["packages/**/*.tsx", "packages/**/*.jsx"],
plugins: {
"react-compiler": reactCompiler,
},
rules: {
"react-compiler/react-compiler": "error",
},
}, {
files: ["packages/**/*.js"],
...tseslint.configs.disableTypeChecked,
})
15 changes: 6 additions & 9 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"test": "vitest",
"coverage": "vitest --coverage",
"ci-coverage": "vitest --coverage && codecov",
"lint": "eslint --ext .jsx --ext .js --ext .ts --ext .tsx packages/*/src",
"lint": "eslint",
"openneuro": "node packages/openneuro-cli/src",
"_postinstall": "husky install",
"prepublish": "rm -fr packages/*/dist .build-cache/ && yarn build",
Expand All @@ -17,22 +17,18 @@
},
"devDependencies": {
"@elastic/apm-rum-react": "2.0.2",
"@eslint/js": "^9.13.0",
"@sentry/cli": "1.37.4",
"@testing-library/jest-dom": "6.1.3",
"@types/ioredis-mock": "^8.2.2",
"@types/jsdom": "^16",
"@types/testing-library__jest-dom": "5.14.5",
"@typescript-eslint/eslint-plugin": "^4.19.0",
"@typescript-eslint/parser": "^4.19.0",
"@vitest/coverage-v8": "^1.5.0",
"@yarnpkg/pnpify": "^3.1.1-rc.8",
"babel-plugin-react-compiler": "19.0.0-beta-8a03594-20241020",
"codecov": "3.8.3",
"eslint": "7.23.0",
"eslint-config-prettier": "^8.1.0",
"eslint-plugin-import": "^2.22.1",
"eslint-plugin-prettier": "^3.3.1",
"eslint-plugin-react": "^7.23.1",
"eslint-plugin-react-hooks": "^4.2.0",
"eslint": "^9.13.0",
"eslint-plugin-react-compiler": "19.0.0-beta-8a03594-20241020",
"graphql": "16.8.1",
"history": "5.2.0",
"husky": "5.1.1",
Expand All @@ -48,6 +44,7 @@
"sass": "^1.56.1",
"tsc-watch": "6.0.4",
"typescript": "5.1.6",
"typescript-eslint": "^8.11.0",
"vite": "5.4.8",
"vitest": "1.5.0",
"vitest-fetch-mock": "0.2.2"
Expand Down
1 change: 1 addition & 0 deletions packages/openneuro-app/src/@types/react.d.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable */
import React from "react"

/**
Expand Down
6 changes: 4 additions & 2 deletions packages/openneuro-app/src/scripts/app.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import React, { FC, ReactNode } from "react"
import React from "react"
import type { FC, ReactNode } from "react"

Check warning on line 2 in packages/openneuro-app/src/scripts/app.tsx

View check run for this annotation

Codecov / codecov/patch

packages/openneuro-app/src/scripts/app.tsx#L2

Added line #L2 was not covered by tests
import Helmet from "react-helmet"
import { frontPage } from "./pages/front-page/front-page-content"
import { Cookies, CookiesProvider } from "react-cookie"
import type { Cookies } from "react-cookie"
import { CookiesProvider } from "react-cookie"

Check warning on line 6 in packages/openneuro-app/src/scripts/app.tsx

View check run for this annotation

Codecov / codecov/patch

packages/openneuro-app/src/scripts/app.tsx#L5-L6

Added lines #L5 - L6 were not covered by tests
import { ToastContainer } from "react-toastify"
import "react-toastify/dist/ReactToastify.css"
import { MediaContextProvider } from "./styles/media"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import jwtDecode from "jwt-decode"

interface OpenNeuroTokenProfile {
export interface OpenNeuroTokenProfile {
sub: string
email: string
provider: string
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-disable */
import React from "react"
import { useCookies } from "react-cookie"
import { getProfile, guardExpired } from "./profile"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ vi.mock("../../../uploader/uploader-view.jsx", () => ({
default: () => "mocked UploaderView",
}))
vi.mock("react-router-dom", async () => ({
// @ts-ignore-check
...(await vi.importActual("react-router-dom")),
useNavigate: () => navigate,
}))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, { FC } from "react"
import React from "react"
import type { FC } from "react"

Check warning on line 2 in packages/openneuro-app/src/scripts/common/containers/footer.tsx

View check run for this annotation

Codecov / codecov/patch

packages/openneuro-app/src/scripts/common/containers/footer.tsx#L2

Added line #L2 was not covered by tests
import { Footer } from "@openneuro/components/footer"
import { version as openneuroVersion } from "../../../lerna.json"

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, { FC, useContext } from "react"
import React, { useContext } from "react"
import type { FC } from "react"
import useState from "react-usestateref"
import UploaderContext from "../../uploader/uploader-context.js"
import UploadProgress from "../../uploader/upload-progress.jsx"
Expand Down Expand Up @@ -26,6 +27,7 @@ export const HeaderContainer: FC = () => {
const [cookies] = useCookies()
const profile = getUnexpiredProfile(cookies)

/* eslint-disable-next-line @typescript-eslint/no-unused-vars */
const { userModalOpen, setUserModalOpen } = useContext(UserModalOpenCtx)

const [newKeyword, setNewKeyword, newKeywordRef] = useState("")
Expand Down
4 changes: 2 additions & 2 deletions packages/openneuro-app/src/scripts/common/content/terms.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { ReactElement } from "react"
import React from "react"

/** Terms and conditions content. */
export function Terms(): ReactElement {
export function Terms(): React.ReactElement {
return (
<>
<p>
Expand Down
7 changes: 5 additions & 2 deletions packages/openneuro-app/src/scripts/components/data-table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ import {
flexRender,
getCoreRowModel,
getSortedRowModel,
SortingState,
useReactTable,
} from "@tanstack/react-table"
import type { SortingState } from "@tanstack/react-table"
import styled from "@emotion/styled"
import { format, isValid, parse, parseISO } from "date-fns"
import { format, isValid, parse } from "date-fns"

interface DataTableProps {
/* eslint-disable-next-line @typescript-eslint/no-explicit-any */
data: any[]
downloadFilename?: string
hideColumns?: string[]
Expand Down Expand Up @@ -55,6 +56,7 @@ export function extractDateString(dateString) {
return false
}

/* eslint-disable-next-line @typescript-eslint/no-explicit-any */
function CellFormat(props): any {
const value = props.getValue()
let extractedDate
Expand Down Expand Up @@ -86,6 +88,7 @@ export function DataTable<T>({
Object.keys(data[0])
.filter((name) => !hideColumns.includes(name))
.map((name) =>
// eslint-disable-next-line @typescript-eslint/no-explicit-any
columnHelper.accessor(name as any, {
header: name,
cell: CellFormat,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
interface SnapshotDatasetProps {
datasetId: string
tag: string
changes: Array<string>
changes: string[]

Check warning on line 36 in packages/openneuro-app/src/scripts/datalad/mutations/snapshot.tsx

View check run for this annotation

Codecov / codecov/patch

packages/openneuro-app/src/scripts/datalad/mutations/snapshot.tsx#L36

Added line #L36 was not covered by tests
}

const SnapshotDataset = ({ datasetId, tag, changes }: SnapshotDatasetProps) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
variables: { datasetId, userEmail, level: metadata },
})
done()
} catch (err) {
} catch (_err) {

Check warning on line 61 in packages/openneuro-app/src/scripts/datalad/mutations/update-permissions.jsx

View check run for this annotation

Codecov / codecov/patch

packages/openneuro-app/src/scripts/datalad/mutations/update-permissions.jsx#L61

Added line #L61 was not covered by tests
toast.error(
<ToastContent body="A user with that email address does not exist" />,
)
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
const { datasetId } = useParams() as { datasetId: string }
const { pathname } = useLocation()

if (redirectLib.hasOwnProperty(datasetId)) {
if (Object.hasOwn(redirectLib, datasetId)) {

Check warning on line 65 in packages/openneuro-app/src/scripts/datalad/routes/dataset-redirect.tsx

View check run for this annotation

Codecov / codecov/patch

packages/openneuro-app/src/scripts/datalad/routes/dataset-redirect.tsx#L65

Added line #L65 was not covered by tests
const newPath = replaceDatasetId(pathname, redirectLib[datasetId])
return <Navigate to={newPath} replace />
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
)
const doAfterSubmit = () => {
setEditorState(EditorState.createEmpty())
done && done()
if (done) done()

Check warning on line 23 in packages/openneuro-app/src/scripts/dataset/comments/comment-editor.jsx

View check run for this annotation

Codecov / codecov/patch

packages/openneuro-app/src/scripts/dataset/comments/comment-editor.jsx#L23

Added line #L23 was not covered by tests
}
const disabled = editorState.getUndoStack().size === 0
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
{comments.map((comment) => {
if (!comment) return null
// Join any replies
const nextLevel = comment.hasOwnProperty("replies")
const nextLevel = Object.hasOwn(comment, "replies")

Check warning on line 17 in packages/openneuro-app/src/scripts/dataset/comments/comments.jsx

View check run for this annotation

Codecov / codecov/patch

packages/openneuro-app/src/scripts/dataset/comments/comments.jsx#L17

Added line #L17 was not covered by tests
? comment.replies.map((reply) => commentMap[reply.id])
: []
return (
Expand Down
Loading
Loading