M2-4835: Linting rules for mindlogger-admin (#1472)

This PR creates a GitHub Action to run linting tasks on pull requests submitted to the repository. As part of the ticket, I took the opportunity to update the ESlint rules on the project. These updates have raised a number of linting issues, but **won't block** the merging of pull requests. While I've addressed some of the smaller items (and any error ones), I expect that these will be addressed in the normal course of feature development so that we can eventually require linting checks to pass in order to merge PRs.

Here's a summary of what changed:

## Conflicting ESLint configurations

There are currently two ESLint configurations in the project:
- An `eslintConfig` property in the `package.json` file
- A `.eslintrc` file

[ESLint documentation](https://eslint.org/docs/latest/use/configure/configuration-files) says that only one of these is read, and the `.eslintrc` takes precedence. Thus, I've removed the ignored configuration since it wasn't being used.

## Prettier Code Formatting

Prettier has been configured in the project as the code formatter of choice, but it is currently conflicting with a number of code formatting ESLint rules. The prettier ESLint plugin disables these rules if it is placed as an inherited configuration at the end of the `extends` array, so I have made this change. The prettier ESLint plugin also enables Prettier as an ESLint rule, so that code formatting issues become errors. This should be okay, since Prettier fixes them for you.

In addition, I've disabled/removed some other code formatting related rules that aren't explicitly handled by the Prettier plugin. These include:
- `padding-line-between-statements`
- `no-extra-semi`
- `jsx-quotes` - I've moved this one to the Prettier config with `jsxSingleQuote`

## Opinionated changes

There are a number of other changes I've made that I think will allow for improved code quality. These are:
- Turning on the `react-hooks/exhaustive-deps` rule
- Adding the `unused-imports` ESLint plugin
- Turning on the `@typescript-eslint/no-non-null-assertion` rule

.eslintrc

@@ -1,51 +1,49 @@
 {
  "root": true,
  "parser": "@typescript-eslint/parser",
- "plugins": ["import", "prettier", "react-hooks", "@typescript-eslint"],
+ "plugins": ["import", "react-hooks", "@typescript-eslint", "unused-imports"],
  "env": {
  "node": true
  },
  "extends": [
- "prettier",
  "eslint:recommended",
  "plugin:@typescript-eslint/eslint-recommended",
- "plugin:@typescript-eslint/recommended"
+ "plugin:@typescript-eslint/recommended",
+ "prettier"
  ],
  "rules": {
  "arrow-body-style": ["error", "as-needed"],
  "react/react-in-jsx-scope": "off",
- "no-trailing-spaces": "error",
  "react/no-children-prop": "off",
  "no-console": ["warn", { "allow": ["warn", "error"] }],
- "quotes": ["error", "single"],
  "import/order": [
  "error",
-  {
-  "groups": [
-  ["external", "builtin"],
-  "internal",
-  ["sibling", "parent"],
-  "index"
-  ],
-  "pathGroups": [
-  {
-  "pattern":
-  "@(api|assets|modules|redux|routes|shared|resources)/**",
-  "group": "internal"
-  },
-  {
-  "pattern":
-  "@(redux|routes|resources|api|i18n)",
-  "group": "internal"
-  }
-  ],
-  "pathGroupsExcludedImportTypes": ["internal"],
-  "newlines-between": "always"
-  }
+ {
+ "groups": [
+ ["external", "builtin"],
+ "internal",
+ ["sibling", "parent"],
+ "index"
+ ],
+ "pathGroups": [
+ {
+ "pattern":
+ "@(api|assets|modules|redux|routes|shared|resources)/**",
+ "group": "internal"
+ },
+ {
+ "pattern":
+ "@(redux|routes|resources|api|i18n)",
+ "group": "internal"
+ }
+ ],
+ "pathGroupsExcludedImportTypes": ["internal"],
+ "newlines-between": "always"
+ }
  ],
  "import/no-cycle": "error",
  "react-hooks/rules-of-hooks": "error",
- "react-hooks/exhaustive-deps": "off",
+ "react-hooks/exhaustive-deps": "warn",
  "constructor-super": "error",
  "no-this-before-super": "error",
  "no-useless-computed-key": "error",
@@ -72,23 +70,20 @@
  "no-nested-ternary": "error",
  "no-unneeded-ternary": "error",
  "camelcase": "off",
- "arrow-parens": ["off", "as-needed"],
  "no-debugger": "error",
  "no-empty": "error",
- "no-underscore-dangle": "error",
  "no-unused-labels": "error",
  "prefer-const": "error",
- "no-unused-vars": "off",
- "jsx-quotes": ["error", "prefer-double"],
- "@typescript-eslint/no-unused-vars": [
+ "@typescript-eslint/no-unused-vars": "off",
+ "unused-imports/no-unused-imports": "error",
+ "unused-imports/no-unused-vars": [
  "warn",
- {
- "ignoreRestSiblings": true
- }
+ { "vars": "all", "varsIgnorePattern": "^_", "args": "after-used", "argsIgnorePattern": "^_" }
  ],
- "@typescript-eslint/no-non-null-assertion": "off",
+ "@typescript-eslint/no-non-null-assertion": "warn",
  "@typescript-eslint/no-explicit-any": "warn",
  "@typescript-eslint/no-duplicate-enum-values": "warn",
+ "no-extra-semi": "off",
  "padding-line-between-statements": [
  "error",
  { "blankLine": "always", "prev": "*", "next": "return" }

.github/workflows/ci.yml

@@ -0,0 +1,54 @@
+name: CI
+on: [pull_request]
+
+env:
+ NODE_VERSION: 18.19.0
+
+jobs:
+ lint:
+ name: Lint
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout@v4
+
+ - uses: actions/setup-node@v4
+ with:
+ node-version: ${{ env.NODE_VERSION }}
+
+ - uses: actions/cache@v4
+ with:
+ path: |
+ ~/node_modules
+ key: ${{ runner.os }}-node-modules-${{ hashFiles('**/package-lock.json') }}
+
+ - run: npm ci
+
+ - uses: wearerequired/lint-action@v2
+ with:
+ eslint: true
+ eslint_extensions: js,jsx,ts,tsx
+ prettier: true
+ prettier_extensions: js,jsx,ts,tsx
+ continue_on_error: false
+ tests:
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout@v4
+
+ - uses: actions/setup-node@v4
+ with:
+ node-version: ${{ env.NODE_VERSION }}
+
+ - name: Install deps
+ run: npm ci
+
+ - uses: actions/cache@v4
+ with:
+ path: |
+ ~/node_modules
+ key: ${{ runner.os }}-node-modules-${{ hashFiles('**/package-lock.json') }}
+
+ - name: Running tests
+ env:
+ REACT_APP_API_DOMAIN: http://localhost:8080
+ run: npm run test:nowatch

.node-version

@@ -0,0 +1 @@
+18.19.0

.prettierrc

@@ -1,6 +1,13 @@
 {
- "semi": true,
- "trailingComma": "all",
+ "arrowParens": "always",
+ "bracketSameLine": false,
+ "bracketSpacing": true,
  "singleQuote": true,
- "printWidth": 100
+ "jsxSingleQuote": false,
+ "trailingComma": "all",
+ "semi": true,
+ "printWidth": 100,
+ "tabWidth": 2,
+ "useTabs": false,
+ "endOfLine": "lf"
 }

package.json

@@ -2,6 +2,9 @@
  "name": "mindlogger-admin",
  "version": "1.0.0",
  "private": true,
+ "engines": {
+ "node": "18.x"
+ },
  "dependencies": {
  "@emotion/react": "^11.11.1",
  "@emotion/styled": "^11.11.0",
@@ -82,6 +85,8 @@
  "scripts": {
  "start": "react-app-rewired start",
  "build": "react-app-rewired build --project tsconfig.build.json",
+ "lint:check": "npx eslint src",
+ "lint:fix": "npx eslint src --fix",
  "test": "react-app-rewired test",
  "test:coverage": "npm run test -- --coverage --watchAll=false --colors",
  "test:junit": "npm run test -- --watchAll=false --reporters=default --reporters=jest-junit",
@@ -90,12 +95,6 @@
  "eject": "react-app-rewired eject",
  "prepare": "husky install"
  },
- "eslintConfig": {
- "extends": [
- "react-app",
- "react-app/jest"
- ]
- },
  "jest": {
  "globalSetup": "./global-setup.js",
  "reporters": [
@@ -139,8 +138,11 @@
  "cross-env": "^7.0.3",
  "eslint": "^8.50.0",
  "eslint-config-prettier": "^9.0.0",
+ "eslint-plugin-import": "^2.26.0",
  "eslint-plugin-prettier": "^5.0.0",
+ "eslint-plugin-react": "^7.31.10",
  "eslint-plugin-react-hooks": "^4.6.0",
+ "eslint-plugin-unused-imports": "^3.0.0",
  "husky": "^8.0.3",
  "jest-junit": "^16.0.0",
  "jest-mock-axios": "^4.7.3",
@@ -149,4 +151,4 @@
  "prettier": "^3.0.3",
  "react-app-rewired": "^2.2.1"
  }
-}
+}

src/modules/Builder/components/Popups/SaveAndPublishProcessPopup/SaveAndPublishProcessPopup.test.tsx

@@ -73,7 +73,7 @@ describe('SaveAndPublishProcessPopup', () => {
  payload: { saveChanges: true },
  });
 
- fireEvent.click(screen.getByText('Don\'t Save'));
+ fireEvent.click(screen.getByText("Don't Save"));
  expect(mockDispatch).toHaveBeenCalledWith({
  type: 'reportConfig/setReportConfigChanges',
  payload: { doNotSaveChanges: true },

src/modules/Builder/features/ActivityAbout/ActivityAbout.test.tsx

@@ -120,16 +120,16 @@ describe('ActivityAbout', () => {
  const isSkippable = screen.getByLabelText('Allow to skip all items');
  expect(isSkippable).toBeChecked();
  const isEditable = screen.getByLabelText(
- 'Disable the respondent\'s ability to change the response',
+ "Disable the respondent's ability to change the response",
  );
  expect(isEditable).toBeChecked();
  const isReviewable = screen.getByLabelText(
- 'This Activity is only displayed in the Admin panel for the reviewer to provide responses about respondent\'s data. Only single selection, multiple selection, and slider items are supported.',
+ "This Activity is only displayed in the Admin panel for the reviewer to provide responses about respondent's data. Only single selection, multiple selection, and slider items are supported.",
  );
  expect(isReviewable).not.toBeDisabled();
  });
 
- test('shouldn\'t turn activity to reviewer one', () => {
+ test("shouldn't turn activity to reviewer one", () => {
  renderWithAppletFormData({
  children: <ActivityAbout />,
  appletFormData: mockedAppletFormData,

src/modules/Builder/features/ActivityItems/ItemConfiguration/InputTypeItems/SelectionOption/SelectionOption.utils.tsx

@@ -35,8 +35,9 @@ export const getDependentConditions = (
  optionValue: string,
  conditionalLogic?: ConditionalLogic[],
 ) =>
- conditionalLogic?.filter(({ conditions }) =>
- conditions?.some(
- ({ payload }) => (payload as OptionCondition['payload'])?.optionValue === optionValue,
- ),
+ conditionalLogic?.filter(
+ ({ conditions }) =>
+ conditions?.some(
+ ({ payload }) => (payload as OptionCondition['payload'])?.optionValue === optionValue,
+ ),
  );

src/modules/Builder/features/ActivityItems/ItemConfiguration/Settings/ItemSettingsController/ItemSettingsController.test.tsx

@@ -359,15 +359,15 @@ describe('ItemSettingsController', () => {
  jest.clearAllMocks();
  });
 
- test('doesn\'t render if inputType is not provided', () => {
+ test("doesn't render if inputType is not provided", () => {
  const container = renderWithAppletFormData({
  children: <ItemSettingsController itemName="" inputType="" name="" />,
  });
 
  expect(container.container).toBeEmptyDOMElement();
  });
 
- test('doesn\'t render if inputType is not in enum', () => {
+ test("doesn't render if inputType is not in enum", () => {
  const container = renderWithAppletFormData({
  children: <ItemSettingsController itemName="" inputType={ItemResponseType.Flanker} name="" />,
  });

src/modules/Builder/features/ActivitySettings/ScoresAndReports/ScoresAndReports.test.tsx

@@ -110,7 +110,7 @@ describe('ScoreAndReports', () => {
  expect(screen.getByTestId(`${dataTestid}-section-1`)).toBeInTheDocument();
  });
 
- test('should show server status \'connected\'', () => {
+ test("should show server status 'connected'", () => {
  renderWithAppletFormData({
  children: <ScoresAndReports />,
  appletFormData: mockedAppletFormData,