chore upgrade ESLint to latest 8.x and migrate to Flat Config #2311

algreasley · 2024-06-19T22:35:31Z

ESLint:

Switch to Flat Config
upgrade to latest 8.x (not 9.x as eslint-plugin-react is not compatible yet, but config will be ready for that being the default in v9+)
run migration tool (npx @eslint/migrate-config .eslintrc.yml )
... then later fixed up all the migration cruft, as it was hard to see what was what
added hooks plugin (bit surprised it wasn't already there .. only a handful of errors to deal with tho)
VS Code ESLint extension requires latest IDE AND an eslint plugin setting to switch on Flat Config (bit daft as eslint v8 just auto-detects and off it goes)

Prettier

upgrade to 3.x
update config to js, remove redundant entries
re-format (see comments for the new types of re-formatting)

.prettierignore

.prettierrc.yml

packages/client/e2e/fixtures.ts

packages/client/eslint.config.js

algreasley · 2024-06-24T11:29:34Z

packages/client/eslint.config.js

+      },
+    },
+
+    rules: {


rules are exactly the same as before, thankfully so this part of the config "upgrade" was a straightforward lift-and-shift ..

packages/client/src/client/App/Credit/common/CreditRfqTimer.tsx

packages/client/src/client/App/Trades/TradesGrid/components/ComparatorSelect.tsx

packages/client/src/client/App/Trades/TradesState/filterState/dateFilterState.ts

packages/client/src/client/App/Trades/TradesState/sortState.ts

packages/client/src/client/BrowserUnsupported.tsx

github-actions · 2024-06-24T11:55:03Z

(auto-deploy) A deployment has been created for this Pull Request

Preview links

As part of the code review process, please ensure that you test against the following

Version	URL
Web	https://web.env.reactivetrader.com/pull/2311
OpenFin - FX	fins://openfin.env.reactivetrader.com/pull/2311/config/rt-fx.json
OpenFin - Credit	fins://openfin.env.reactivetrader.com/pull/2311/config/rt-credit.json
OpenFin - Launcher	fins://openfin.env.reactivetrader.com/pull/2311/config/launcher.json
OpenFin - Workspace	fins://openfin.env.reactivetrader.com/pull/2311/workspace/config/workspace.json
Finsemble	https://finsemble.env.reactivetrader.com/pull/2311

Performance

Please ensure that this PR does not degrade the performance of the UI. We should maintain a performance score of 95+.

https://developers.google.com/speed/pagespeed/insights/?url=https://web.env.reactivetrader.com/pull/2311

grancalavera · 2024-06-24T16:02:05Z

packages/client/eslint.config.js

+    "plugin:react/recommended",
+    "plugin:react/jsx-runtime",
+    "plugin:@typescript-eslint/recommended",
+    "prettier",


does this make eslint play nice with prettier? Is this to be able to use prettier as the formatter in vscode?

yeah, Ignacio did the detail work on this, but that's the idea - prettier for layout, lint for correctness - and I am 100% behind that decision / philosophy - there's a good article on the different ways you can go with this.
eslint-config-prettier is the option for doing formatting separately (presume it just switches stuff off)

now the extra config negating eslint rules no longer necessary, it seems, as stylistic rules have been deprecated in these recent versions of the linting packages .. happy days 🙂

algreasley · 2024-06-25T22:32:48Z

packages/client/package.json

    "lint:cache": "npm run lint -- --cache",
-    "_format": "prettier README.md \"src/**/*.{js,jsx,ts,tsx,json,md}\" \"e2e/**/*.ts\"",
+    "_format": "prettier README.md src e2e",


same for prettier, less complicated .. just check everything that the tool understands

algreasley · 2024-06-25T22:38:08Z

packages/client/src/client/App/Analytics/ProfitAndLoss/LineChart/Tooltip.tsx

@@ -139,7 +140,7 @@ export const Tooltip = ({ svgRef }: { svgRef: RefObject<SVGSVGElement> }) => {
      svg.removeEventListener("pointermove", onMove)
      svg.removeEventListener("pointerleave", onLeave)
    }
-  }, [svgRef])
+  }, [svgRef, tooltipDivRef])


first of a handful of corrected hooks, with the new eslint plugin

algreasley · 2024-06-25T22:38:45Z

packages/client/src/client/App/Credit/CreditRfqs/CreditRfqCards/CardFooter.tsx

-      ? useCreditDealerById(acceptedDealerId)
-      : undefined
-    )?.name ?? "Unknown Dealer"
+  const dealerName = useDealerNameById(acceptedDealerId)


significant change .. tidied this up properly this time, the hook/stream can deal with the missing dealer!

algreasley · 2024-06-25T22:39:29Z

packages/client/src/client/App/Header/theme-switcher/ThemeSwitcher.tsx

@@ -83,8 +83,4 @@ const ThemeStorageSwitch = ({
  )
 }

-ThemeStorageSwitch.defaultProps = {


wot? defaultProps? .. what decade are we in? 😁

thanks to updated rules, this popped up ..

algreasley · 2024-06-25T22:40:24Z

packages/client/src/client/App/Trades/__tests__/FxTrades.filter.test.tsx

@@ -1,5 +1,4 @@
 import { act, fireEvent, render, screen } from "@testing-library/react"
-import userEvent from "@testing-library/user-event"


simple unused var rule, now applied to tests, as it should be

algreasley · 2024-06-25T22:41:42Z

packages/client/src/client/OpenFin/provider.ts

@@ -141,7 +141,6 @@ function interopOverride(
      clientIdentity: OpenFin.ClientIdentity,
    ) {
      // use accessor syntax for this.getClientState as it is not a public inherited method from InteropBroker
-      // eslint-disable-next-line @typescript-eslint/dot-notation


briefly upgraded to eslint v9, and this showed up all the unused disable directives .. which was nice of it (I think you can do that with eslint config in < v9, as we won't be upgrading for a while, given the plugin ecosystem state ..

algreasley · 2024-06-26T08:49:57Z

packages/client/eslint.config.js

+      react,
+      // https://github.com/facebook/react/tree/main/packages/eslint-plugin-react-hooks
+      // compat with ESLint v9, see https://github.com/facebook/react/issues/28313
+      "react-hooks": hooks,


added hooks plugin .. not FlatConfig-ready, so cannot just add config as tseslint above (this is the future model) ... just a few transgressions to fix

algreasley · 2024-06-26T08:50:26Z

packages/client/eslint.config.js

+      "simple-import-sort/exports": "error",
+
+      // as hooks plugin does not play well with Flat Config right now, do this
+      ...hooks.configs.recommended.rules,


hooks - manually spread rules alongside our custom rules .. bit messy, but works

algreasley marked this pull request as draft June 19, 2024 22:35

algreasley requested review from grancalavera, TobyLi96, Dev-Alex23 and SHolleworth June 20, 2024 08:34

algreasley force-pushed the chore/5873-upgrade-eslint-prettier branch from 3ca1e22 to 8f99fdc Compare June 24, 2024 11:24

algreasley commented Jun 24, 2024

View reviewed changes

algreasley marked this pull request as ready for review June 24, 2024 11:52

grancalavera reviewed Jun 24, 2024

View reviewed changes

grancalavera previously approved these changes Jun 24, 2024

View reviewed changes

algreasley dismissed grancalavera’s stale review via 52337ec June 25, 2024 16:46

algreasley force-pushed the chore/5873-upgrade-eslint-prettier branch 3 times, most recently from bf897a5 to d252a32 Compare June 25, 2024 22:04

algreasley added 4 commits June 25, 2024 23:08

chore: upgrade prettier to v3, js config and re-format

4cde919

chore: upgrade ESLint to latest 8.x and migrate to Flat Config

6f5c395

chore: revise and simplify ESLint flat config

b4623ba

chore: extend typecheck to config

6a40352

algreasley force-pushed the chore/5873-upgrade-eslint-prettier branch from d252a32 to df72dbe Compare June 25, 2024 22:27

chore: add hooks ESLint plugin

6125019

algreasley force-pushed the chore/5873-upgrade-eslint-prettier branch from df72dbe to 6125019 Compare June 25, 2024 22:42

algreasley commented Jun 26, 2024

View reviewed changes

algreasley merged commit 2fe3ed2 into master Jun 26, 2024
7 checks passed

algreasley deleted the chore/5873-upgrade-eslint-prettier branch June 26, 2024 13:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore upgrade ESLint to latest 8.x and migrate to Flat Config #2311

chore upgrade ESLint to latest 8.x and migrate to Flat Config #2311

algreasley commented Jun 19, 2024 •

edited

Loading

algreasley Jun 24, 2024

github-actions bot commented Jun 24, 2024

grancalavera Jun 24, 2024

algreasley Jun 24, 2024 •

edited

Loading

algreasley Jun 25, 2024

algreasley Jun 25, 2024

algreasley Jun 25, 2024

algreasley Jun 25, 2024

algreasley Jun 25, 2024

algreasley Jun 25, 2024

algreasley Jun 25, 2024

algreasley Jun 25, 2024

algreasley Jun 26, 2024

algreasley Jun 26, 2024

		@@ -1,5 +1,4 @@
		import { act, fireEvent, render, screen } from "@testing-library/react"
		import userEvent from "@testing-library/user-event"

chore upgrade ESLint to latest 8.x and migrate to Flat Config #2311

chore upgrade ESLint to latest 8.x and migrate to Flat Config #2311

Conversation

algreasley commented Jun 19, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jun 24, 2024

Preview links

Performance

Choose a reason for hiding this comment

algreasley Jun 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

algreasley commented Jun 19, 2024 •

edited

Loading

algreasley Jun 24, 2024 •

edited

Loading