Skip to content

Commit

Permalink
🌱 Setup eslint with prettier (konveyor#1291)
Browse files Browse the repository at this point in the history
Replace the old and broken eslint configuration in `client/package.json`
with a more robust configuration. `eslint`, `typescript-eslint` and
`prettier` are configured to work together to lint javascript and
typescript files.

Changes:
  - One eslint config for all packages: `.eslintrc.cjs`

  - Add explicit config files for prettier: `.prettierrc.mjs`,
    `.prettierignore`, and `.editorconfig`

  - Prettier v2 and v3 have changes to some styles, including the default
    for tailing commas. Our configurations are now set to v2 default "es5"
    trailing commas.

  - A total of 10 eslint rules have been configurated as warnings instead
    of errors. There are a lot of these warnings, and fixing them in this PR
    would create a very large PR. Setting as warnings will allow us to fix
    the problems and set them back to errors in future PRs. 1 PR for each
    warn to error conversion is what I anticipate.

  - A set of errors have been fixed in the PR. These errors only hit a few
    times each and were easy to fix.

  - Use `lint-staged` as pre-commit hook:
    - Previously `pretty-quick` was used to run `prettier@^2` in the client
      package when committing staged files. Since `pretty-quick` does not
      currently play well with `prettier@^3`, and since we'd like to also use
      the eslint setup, the pre-commit hook has be changed over to use
      `lint-staged`.

    - Each monorepo package has its own `lint-staged` configuration based on
      the files in each repo. Both `eslint` and `prettier` are used as
      appropriate. If any staged files cannot be auto-fixed by eslint, the
      commit should fail.

Signed-off-by: Scott J Dickerson <[email protected]>
  • Loading branch information
sjd78 authored Aug 30, 2023
1 parent 427ed02 commit 9063539
Show file tree
Hide file tree
Showing 30 changed files with 2,176 additions and 416 deletions.
13 changes: 13 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
root=true

[*]
# standard prettier behaviors
charset = utf-8
insert_final_newline = true
trim_trailing_whitespace = true

# Configurable prettier behaviors
end_of_line = lf
indent_style = space
indent_size = 2
max_line_length = 80
90 changes: 90 additions & 0 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/* eslint-env node */

module.exports = {
root: true,

env: {
browser: true,
es2020: true,
jest: true,
},

parser: "@typescript-eslint/parser",
parserOptions: {
ecmaFeatures: {
jsx: true,
},
ecmaVersion: 2020, // keep in sync with tsconfig.json
sourceType: "module",
},

// eslint-disable-next-line prettier/prettier
extends: [
"eslint:recommended",
"plugin:@typescript-eslint/recommended",
"plugin:react/recommended",
"prettier",
],

// eslint-disable-next-line prettier/prettier
plugins: [
"prettier",
"unused-imports", // or eslint-plugin-import?
"@typescript-eslint",
"react",
"react-hooks",
],

// NOTE: Tweak the rules as needed when bulk fixes get merged
rules: {
// TODO: set to "error" when prettier v2 to v3 style changes are fixed
"prettier/prettier": ["warn"],

// TODO: set to "error" when all resolved, but keep the `argsIgnorePattern`
"@typescript-eslint/no-unused-vars": ["warn", { argsIgnorePattern: "^_" }],

// TODO: each one of these can be removed or set to "error" when they're all resolved
"unused-imports/no-unused-imports": ["warn"],
"@typescript-eslint/ban-types": "warn",
"@typescript-eslint/no-explicit-any": "warn",
"react/jsx-key": "warn",
"react-hooks/rules-of-hooks": "warn",
"react-hooks/exhaustive-deps": "warn",
"no-extra-boolean-cast": "warn",
"prefer-const": "warn",

// Allow the "cy-data" property for tackle-ui-test (but should really be "data-cy" w/o this rule)
"react/no-unknown-property": ["error", { ignore: ["cy-data"] }],
},

settings: {
react: { version: "detect" },
},

ignorePatterns: [
// don't ignore dot files so config files get linted
"!.*.js",
"!.*.cjs",
"!.*.mjs",

// take the place of `.eslintignore`
"dist/",
"generated/",
"node_modules/",
],

// this is a hack to make sure eslint will look at all of the file extensions we
// care about without having to put it on the command line
overrides: [
{
files: [
"**/*.js",
"**/*.jsx",
"**/*.cjs",
"**/*.mjs",
"**/*.ts",
"**/*.tsx",
],
},
],
};
3 changes: 3 additions & 0 deletions .github/workflows/ci-actions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ jobs:
- name: Install
run: npm clean-install

- name: Lint sources
run: npm run lint

- name: Build
run: npm run build

Expand Down
2 changes: 1 addition & 1 deletion client/.husky/pre-commit → .husky/pre-commit
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env sh
. "$(dirname -- "$0")/_/husky.sh"

npx pretty-quick --staged
npx lint-staged
12 changes: 12 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Library, IDE and build locations
**/node_modules/
**/coverage/
**/dist/
.vscode/
.idea/
.eslintcache/

#
# NOTE: Could ignore anything that eslint will look at since eslint also applies
# prettier.
#
14 changes: 14 additions & 0 deletions .prettierrc.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/** @type {import("prettier").Config} */
const config = {
trailingComma: "es5", // es5 was the default in prettier v2
semi: true,
singleQuote: false,

// Values used from .editorconfig:
// - printWidth == max_line_length
// - tabWidth == indent_size
// - useTabs == indent_style
// - endOfLine == end_of_line
};

export default config;
4 changes: 3 additions & 1 deletion client/__mocks__/fileMock.js
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
module.exports = 'test-file-stub';
/* eslint-env node */

module.exports = "test-file-stub";
2 changes: 2 additions & 0 deletions client/__mocks__/styleMock.js
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
/* eslint-env node */

module.exports = {};
30 changes: 0 additions & 30 deletions client/config/stylePaths.js

This file was deleted.

30 changes: 30 additions & 0 deletions client/config/stylePaths.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/* eslint-env node */

import * as path from "path";

export const stylePaths = [
path.resolve(__dirname, "../src"),
path.resolve(__dirname, "../../node_modules/patternfly"),
path.resolve(__dirname, "../../node_modules/@patternfly/patternfly"),
path.resolve(__dirname, "../../node_modules/@patternfly/react-styles/css"),
path.resolve(
__dirname,
"../../node_modules/@patternfly/react-core/dist/styles/base.css"
),
path.resolve(
__dirname,
"../../node_modules/@patternfly/react-core/dist/esm/@patternfly/patternfly"
),
path.resolve(
__dirname,
"../../node_modules/@patternfly/react-core/node_modules/@patternfly/react-styles/css"
),
path.resolve(
__dirname,
"../../node_modules/@patternfly/react-table/node_modules/@patternfly/react-styles/css"
),
path.resolve(
__dirname,
"../../node_modules/@patternfly/react-inline-edit-extension/node_modules/@patternfly/react-styles/css"
),
];
2 changes: 2 additions & 0 deletions client/i18next-parser.config.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* eslint-env node */

module.exports = {
createOldCatalogs: true, // Save the \_old files

Expand Down
16 changes: 5 additions & 11 deletions client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
"license": "Apache-2.0",
"private": true,
"scripts": {
"prepare": "cd .. && husky install client/.husky",
"analyze": "source-map-explorer 'dist/static/js/*.js'",
"clean": "rimraf ./dist",
"extract": "i18next --config i18next-parser.config.js",
Expand All @@ -13,8 +12,13 @@
"build:dev": "webpack --config ./config/webpack.dev.ts",
"start:dev": "webpack serve --config ./config/webpack.dev.ts",
"test": "jest --rootDir=. --config=./config/jest.config.ts",
"lint": "eslint .",
"tsc": "tsc -p ./tsconfig.json"
},
"lint-staged": {
"*.{js,cjs,mjs,jsx,ts,cts,mts,tsx}": "eslint --fix",
"*.{css,json,md,yaml,yml}": "prettier --write"
},
"dependencies": {
"@dnd-kit/core": "^6.0.7",
"@dnd-kit/sortable": "^7.0.2",
Expand Down Expand Up @@ -78,12 +82,10 @@
"css-loader": "^5.2.7",
"css-minimizer-webpack-plugin": "^3.4.1",
"dotenv-webpack": "^7.0.3",
"eslint": "^8.7.0",
"exports-loader": "^3.1.0",
"file-loader": "^6.2.0",
"fork-ts-checker-webpack-plugin": "^8.0.0",
"html-webpack-plugin": "^5.5.0",
"husky": "^8.0.3",
"i18next-parser": "^0.13.0",
"jest": "^29.6.2",
"jest-environment-jsdom": "^29.6.2",
Expand All @@ -92,8 +94,6 @@
"mini-css-extract-plugin": "^2.5.2",
"monaco-editor-webpack-plugin": "^7.0.1",
"msw": "^1.2.3",
"prettier": "^2.2.1",
"pretty-quick": "^3.1.3",
"raw-loader": "^4.0.2",
"react-refresh": "^0.14.0",
"react-refresh-typescript": "^2.0.9",
Expand All @@ -111,12 +111,6 @@
"webpack-dev-server": "^4.15.1",
"webpack-merge": "^5.9.0"
},
"eslintConfig": {
"extends": [
"react-app",
"react-app/jest"
]
},
"browserslist": {
"production": [
">0.2%",
Expand Down
16 changes: 8 additions & 8 deletions client/src/app/Routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -196,22 +196,22 @@ export const AppRoutes = () => {
<Suspense fallback={<AppPlaceholder />}>
<ErrorBoundary FallbackComponent={ErrorFallback} key={location.pathname}>
<Switch>
{devRoutes.map(({ ...props }, index) => (
{devRoutes.map(({ comp, path, exact }, index) => (
<RouteWrapper
comp={props.comp}
comp={comp}
key={index}
roles={devRoles}
path={props.path}
exact={props.exact}
path={path}
exact={exact}
/>
))}
{adminRoutes.map(({ ...props }, index) => (
{adminRoutes.map(({ comp, path, exact }, index) => (
<RouteWrapper
comp={props.comp}
comp={comp}
key={index}
roles={adminRoles}
path={props.path}
exact={props.exact}
path={path}
exact={exact}
/>
))}
<Redirect from="*" to="/applications" />
Expand Down
4 changes: 2 additions & 2 deletions client/src/app/components/tests/ConditionalRender.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ describe("ConditionalRender", () => {
it("Renders WHEN=true", () => {
render(
<ConditionalRender when={true} then={"Hello world"}>
I'm the content
I&apos;m the content
</ConditionalRender>
);
screen.findByRole("heading", { name: /Hello world/i });
Expand All @@ -15,7 +15,7 @@ describe("ConditionalRender", () => {
it("Renders WHEN=false", () => {
render(
<ConditionalRender when={false} then={"Hello world"}>
I'm the content
I&apos;m the content
</ConditionalRender>
);
screen.findByRole("heading", { name: /I'm the content/i });
Expand Down
1 change: 1 addition & 0 deletions client/src/app/hooks/useRuleFiles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { XMLValidator } from "fast-xml-parser";
import XSDSchema from "./windup-jboss-ruleset.xsd";
import { checkRuleFileType } from "../utils/rules-utils";
import { DropEvent } from "@patternfly/react-core";
// eslint-disable-next-line @typescript-eslint/no-var-requires
const xmllint = require("xmllint");

export default function useRuleFiles(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,9 @@ const AssessmentSettings: React.FC = () => {
},
} = tableControls;

// TODO: Check RBAC access
const rbacWriteAccess = true; // checkAccess(userScopes, questionnaireWriteScopes);

return (
<>
<PageSection variant={PageSectionVariants.light}>
Expand Down Expand Up @@ -189,25 +192,21 @@ const AssessmentSettings: React.FC = () => {
</Button>
</ToolbarItem>
{/* </RBAC> */}
{
//RBAC
// xxxxWriteAccess = checkAccess(userScopes, questionnaireWriteScopes);
true ? ( //TODO: Check RBAC access
<ToolbarItem>
<Button
type="button"
id="download-yaml-template"
aria-label="Download questionnaire"
variant={ButtonVariant.link}
onClick={() => setIsDownloadTemplateModal(true)}
>
{t("dialog.title.download", {
what: t("terms.YAMLTemplate"),
})}
</Button>
</ToolbarItem>
) : null
}
{rbacWriteAccess ? (
<ToolbarItem>
<Button
type="button"
id="download-yaml-template"
aria-label="Download questionnaire"
variant={ButtonVariant.link}
onClick={() => setIsDownloadTemplateModal(true)}
>
{t("dialog.title.download", {
what: t("terms.YAMLTemplate"),
})}
</Button>
</ToolbarItem>
) : null}
</ToolbarGroup>
<ToolbarItem {...paginationToolbarItemProps}>
<SimplePagination
Expand Down Expand Up @@ -416,7 +415,7 @@ const AssessmentSettings: React.FC = () => {
isOpen={isDownloadTemplateModal}
onClose={() => setIsDownloadTemplateModal(false)}
>
<Text>TODO Downlaod YAML Template component</Text>
<Text>TODO Download YAML Template component</Text>
</Modal>
<ConfirmDialog
title={t("dialog.title.delete", {
Expand Down
Loading

0 comments on commit 9063539

Please sign in to comment.