Add import and style sorting config + capabilities. #132
Conversation
packages/webapp/.eslintrc.cjs
Outdated
| plugins: ["@typescript-eslint"], | ||
| project: "./tsconfig.json", | ||
| "sourceType": "module", | ||
| "ecmaVersion": "latest", |
There was a problem hiding this comment.
What's with the weird indentation here?
There was a problem hiding this comment.
It doesn't seem to appear on my local repo. 😶
There was a problem hiding this comment.
It's because of mixed tabs and spaces. We should add .cjs and .mjs files to the list of those who get formatted by prettier, cand you do that?
packages/webapp/.eslintrc.cjs
Outdated
| "import/no-duplicates": "error", | ||
| "simple-import-sort/imports": [ | ||
| "error", | ||
| { |
There was a problem hiding this comment.
Weird alignment, is this not caught by the formatter?
packages/webapp/.eslintrc.cjs
Outdated
| // Style imports. | ||
| ["^.+\\.s?css$"], | ||
| // Side effect imports. | ||
| ["^\\u0000"] |
There was a problem hiding this comment.
One rule that would be great is to make sure the internal imports, which we prefix with src/ are in their own separate section (after the "internal packages" would be ideal — we should also change that comment to actually say "external packages" (stuff like wagmi etc))
packages/webapp/.eslintrc.cjs
Outdated
| plugins: ["@typescript-eslint"], | ||
| project: "./tsconfig.json", | ||
| "sourceType": "module", | ||
| "ecmaVersion": "latest", |
There was a problem hiding this comment.
It's because of mixed tabs and spaces. We should add .cjs and .mjs files to the list of those who get formatted by prettier, cand you do that?
| import { convertStringToSafeNumber } from "src/utils/js-utils" | ||
|
|
||
| import BoardCard from "./boardCard" | ||
| import DraggedCard from "./draggedCard" | ||
| import HandCard from "./handCard" |
There was a problem hiding this comment.
These should use the src/ format. I know it's not formatting related, but could you fix it?
I think we could use this type of rule to prevent them, but I wonder if we'd need to be able to make exception to import files form other packages (e.g. deployment addresses) — I think we could get around that by copying files however. Could you look into it?
There was a problem hiding this comment.
added this rule, running make check and make format doesn't pop up any errors.
packages/webapp/src/components/modals/inGameMenuModalContent.tsx
Outdated
Show resolved
Hide resolved
| { | ||
| patterns: ["!./*", "!../*"], | ||
| }, | ||
| ], |
There was a problem hiding this comment.
So I was confused why this would work because this is the pattern used in the link I gave to ONLY ALLOW relative links.
Turns out:
-
make checkdoes not actually run the eslint checks, it needs to bepnpm next lint --max-warnings 0and notpnpm eslint(but let's actually callmake lintfrommake check, that way we avoid duplication) -
Even so, this should forbid every single one of our imports... turns out it does not work, probably
!is not a thing as an operator. -
BUT, if you remove the
!, then it does exactly what we want, and indeed catches a few relative imports that slipped through the cracks :)
Can you fix make check, this file and eliminate the remaining relative imports? And also add a comment in this file to explain we forbid relative imports and absolute imports starting with src/ should be used instead?
Oh, also you'll need to exclude generated.ts from eslint (ignorePatterns option).
I see we're also excluding src/hooks/useScrollBox.ts from linting, seems to be about React useEffect dependencies, would be good to remove it from the ignore list and either fix or simply add comments to disable eslint on those useEffect statements.
|
Adds auto class sorting with Prettier for tailwind styles and import sorting with ESLint.
Also adds the eslint
fixcommand to theformatcommand in the webapp Makefile, sincecheckwill now show an import sort errors.