Upgrade ESLint (#1713)

* Upgrade eslint-config-shakacode

* Upgrade to ESLint 9

* Disable some unneeded rules

* Auto-fixes

* Enable ESLint on config files

* Update Knip

* Enable ESLint on generator templates

---------

Co-authored-by: Alexey Romanov <alexey.v.romanov@gmail.com>

Author: alexeyr-ci2

.eslintignore

@@ -81,7 +81,7 @@ jobs:
           yarn run knip
           yarn run knip --production
       - name: Lint JS
-        run: yarn start lint
+        run: yarn run eslint --report-unused-disable-directives
       - name: Check formatting
         run: yarn start format.listDifferent
       - name: Type-check TypeScript

.eslintrc

@@ -38,7 +38,6 @@ jobs:
       - name: Install Node modules with Yarn for renderer package
         run: |
           yarn install --no-progress --no-emoji
-          yarn run eslint -v
           sudo yarn global add yalc
       - name: Run JS unit tests for Renderer package
         run: yarn test

.github/workflows/lint-js-and-ruby.yml

@@ -17,7 +17,3 @@ spec/dummy/public
 .rubocop.yml
 # Intentionally invalid
 spec/react_on_rails/fixtures/i18n/locales_symbols/
-
-# Weirdly, fixing this file creates linting errors, even though it shouldn't make a difference.
-# Resolve later when upgrading ESLint.
-.eslintrc

.github/workflows/package-js-tests.yml

@@ -0,0 +1,163 @@
+import { globalIgnores } from 'eslint/config';
+import prettierRecommended from 'eslint-plugin-prettier/recommended';
+import globals from 'globals';
+import tsEslint from 'typescript-eslint';
+import js from '@eslint/js';
+import { FlatCompat } from '@eslint/eslintrc';
+
+const compat = new FlatCompat({
+  baseDirectory: __dirname,
+  recommendedConfig: js.configs.recommended,
+  allConfig: js.configs.all,
+});
+
+const config = tsEslint.config([
+  globalIgnores([
+    // compiled code
+    'node_package/lib/',
+    // used for tests only
+    'spec/react_on_rails/dummy-for-generators',
+    // temporary and generated files
+    'spec/dummy/.yalc',
+    'spec/dummy/public',
+    'spec/dummy/vendor',
+    'spec/dummy/tmp',
+    'spec/dummy/app/assets/config/manifest.js',
+    'spec/dummy/client/app/packs/server-bundle.js',
+    '**/*.res.js',
+    '**/coverage',
+    '**/assets/webpack/**/*',
+    '**/public/webpack/**/*',
+    '**/generated/**/*',
+    '**/app/assets/javascripts/application.js',
+    '**/cable.js',
+    '**/public/packs*/*',
+    '**/gen-examples/',
+    '**/bundle/',
+    // dependencies
+    '**/node_modules/**/*',
+  ]),
+  {
+    files: ['**/*.[jt]s', '**/*.[jt]sx', '**/*.[cm][jt]s'],
+  },
+  js.configs.recommended,
+  compat.extends('eslint-config-shakacode'),
+  {
+    languageOptions: {
+      globals: {
+        ...globals.browser,
+        ...globals.node,
+        ...globals.mocha,
+        ...globals.jest,
+        __DEBUG_SERVER_ERRORS__: true,
+        __SERVER_ERRORS__: true,
+      },
+
+      parserOptions: {
+        requireConfigFile: false,
+
+        babelOptions: {
+          presets: ['@babel/preset-env', '@babel/preset-react'],
+        },
+      },
+    },
+
+    settings: {
+      'import/core-modules': ['react-redux'],
+
+      'import/resolver': {
+        alias: [['Assets', './spec/dummy/client/app/assets']],
+
+        node: {
+          extensions: ['.js', '.jsx', '.ts', '.d.ts'],
+        },
+      },
+    },
+
+    rules: {
+      'no-shadow': 'off',
+      'no-console': 'off',
+      'function-paren-newline': 'off',
+      'object-curly-newline': 'off',
+      'no-restricted-syntax': ['error', 'SequenceExpression'],
+
+      'import/extensions': [
+        'error',
+        'ignorePackages',
+        {
+          js: 'never',
+          jsx: 'never',
+          ts: 'never',
+        },
+      ],
+
+      'import/first': 'off',
+      'import/no-extraneous-dependencies': 'off',
+      // The rule seems broken: it's reporting errors on imports in files using `export` too,
+      // not just `module.exports`.
+      'import/no-import-module-exports': 'off',
+      'import/no-unresolved': [
+        'error',
+        {
+          ignore: ['\\.res\\.js$'],
+        },
+      ],
+      'react/destructuring-assignment': [
+        'error',
+        'always',
+        {
+          ignoreClassFields: true,
+        },
+      ],
+      'react/forbid-prop-types': 'off',
+      'react/function-component-definition': [
+        'error',
+        {
+          namedComponents: ['arrow-function', 'function-declaration'],
+          unnamedComponents: 'arrow-function',
+        },
+      ],
+      'react/jsx-props-no-spreading': 'off',
+      'react/static-property-placement': 'off',
+      'jsx-a11y/anchor-is-valid': 'off',
+    },
+  },
+  {
+    files: ['lib/generators/react_on_rails/templates/**/*'],
+    rules: {
+      // It doesn't use package.json from the template
+      'import/no-unresolved': 'off',
+      // We have `const [name, setName] = useState(props.name)` so can't just destructure props
+      'react/destructuring-assignment': 'off',
+    },
+  },
+  {
+    files: ['**/*.ts', '**/*.tsx'],
+
+    extends: tsEslint.configs.recommended,
+
+    languageOptions: {
+      parserOptions: {
+        projectService: {
+          allowDefaultProject: ['eslint.config.ts', 'knip.ts'],
+        },
+      },
+    },
+
+    rules: {
+      '@typescript-eslint/no-namespace': 'off',
+      '@typescript-eslint/no-shadow': 'error',
+      '@typescript-eslint/no-unused-vars': [
+        'error',
+        {
+          caughtErrorsIgnorePattern: '^_',
+        },
+      ],
+    },
+  },
+  // must be the last config in the array
+  // https://github.com/prettier/eslint-plugin-prettier?tab=readme-ov-file#configuration-new-eslintconfigjs
+  prettierRecommended,
+]);
+
+export default config;

.prettierignore

@@ -10,6 +10,7 @@ const config: KnipConfig = {
         'node_package/src/registerServerComponent/client.ts!',
         'node_package/src/registerServerComponent/server.ts!',
         'node_package/src/RSCClientRoot.ts!',
+        'eslint.config.ts',
       ],
       project: ['node_package/src/**/*.[jt]s{x,}!', 'node_package/tests/**/*.[jt]s{x,}'],
       babel: {
@@ -27,10 +28,17 @@ const config: KnipConfig = {
         '@types/turbolinks',
         // The Knip ESLint plugin fails to detect these are transitively required by a config,
         // though we don't actually use its rules anywhere.
+        '@babel/eslint-parser',
+        '@babel/preset-react',
+        'eslint-config-shakacode',
+        'eslint-import-resolver-alias',
+        'eslint-plugin-import',
         'eslint-plugin-jsx-a11y',
         'eslint-plugin-react',
-        // Used in CI
-        '@arethetypeswrong/cli',
+        'eslint-plugin-react-hooks',
+        // These are used as transitive dependencies and missing from package.json
+        '@eslint/eslintrc',
+        '@eslint/js',
         // used by Jest
         'jsdom',
       ],

eslint.config.ts

@@ -1,7 +1,7 @@
 import { combineReducers } from 'redux';
 import { HELLO_WORLD_NAME_UPDATE } from '../constants/helloWorldConstants';
 
-const name = (state = '', action) => {
+const name = (state = '', action = {}) => {
   switch (action.type) {
     case HELLO_WORLD_NAME_UPDATE:
       return action.text;

knip.ts

@@ -14,11 +14,15 @@ type WaitingPromiseInfo<T> = {
 
 export default class CallbackRegistry<T> {
   private readonly registryType: string;
+
   private registeredItems = new Map<string, T>();
+
   private waitingPromises = new Map<string, WaitingPromiseInfo<T>>();
+
   private notUsedItems = new Set<string>();
 
   private timeoutEventsInitialized = false;
+
   private timedout = false;
 
   constructor(registryType: string) {

lib/generators/react_on_rails/templates/redux/base/app/javascript/bundles/HelloWorld/reducers/helloWorldReducer.js

@@ -1,3 +1,6 @@
+/* eslint-disable max-classes-per-file */
+/* eslint-disable react/no-deprecated -- while we need to support React 16 */
+
 import * as ReactDOM from 'react-dom';
 import type { ReactElement } from 'react';
 import type { RailsContext, RegisteredComponent, RenderFunction, Root } from './types';
@@ -40,8 +43,11 @@ const getDomId = (domIdOrElement: string | Element): string =>
   typeof domIdOrElement === 'string' ? domIdOrElement : domIdOrElement.getAttribute('data-dom-id') || '';
 class ComponentRenderer {
   private domNodeId: string;
+
   private state: 'unmounted' | 'rendering' | 'rendered';
+
   private root?: Root;
+
   private renderPromise?: Promise<void>;
 
   constructor(domIdOrElement: string | Element) {
@@ -166,6 +172,7 @@ You should return a React.Component always for the client side entry point.`);
 
 class StoreRenderer {
   private hydratePromise?: Promise<void>;
+
   private state: 'unmounted' | 'hydrating' | 'hydrated';
 
   constructor(storeDataElement: Element) {