EVG-20048: Introduce check-file plugin #2109
Conversation
Passing run #14216 ↗︎
Details:
Review all test suite changes for PR #2109 ↗︎ |
||||||||||||||||||||
.eslintrc.js
Outdated
| @@ -96,6 +96,18 @@ module.exports = { | |||
| // Rules for prettier. | |||
| "prettier/prettier": errorIfStrict, // Makes Prettier issues warnings rather than errors. | |||
| "sort-destructure-keys/sort-destructure-keys": errorIfStrict, | |||
| "check-file/filename-naming-convention": [ | |||
| "warn", | |||
There was a problem hiding this comment.
Use our constant.
| "warn", | |
| WARN, |
There was a problem hiding this comment.
or alternatively errorIfStrict is there any reason we wouldn't want to enforce files are consistently named?
There was a problem hiding this comment.
What do you think about using the WARN flag until all of the problems are resolved and then apply errorIfStrict? Alternatively we can do them at once but there will be over 42 renamed files and imports.
.eslintrc.js
Outdated
| "**/*.graphql": "KEBAB_CASE", | ||
| "cypress/integration/**/*.ts": "SNAKE_CASE", | ||
| "scripts/**/*.{js,ts}": "KEBAB_CASE", | ||
| "src/**/*.{js,ts}": "CAMEL_CASE", |
There was a problem hiding this comment.
You can use !(something) syntax to not match on values that can be hooks.
e.g.
src/**/!(use)*.ts Will match all ts files that do not have the use keyword aka are not hooks.
Checkout for an easy way to test out glob match rules.
https://www.digitalocean.com/community/tools/glob?comments=true&glob=src/**/!(use)*.tsx&matches=false&tests=src/hooks/useTestFile.ts&tests=src/components/SomeRandomComponent.tsx
|
converting to draft to resolve the lint errors |
|
Removed my review request, please retag me when it's ready. |
There was a problem hiding this comment.
This doesn't actually need the .tsx extension because they don't contain any tsx but I changed it to tsx because to match the current pattern. I see how tsx could make sense since the file exports the entry to a page which has a bunch of tsx.
There was a problem hiding this comment.
I don't think this change is necessary.
There was a problem hiding this comment.
If we change this back to .ts then I will either need to add an exception since it's currently pascal case, or convert it into camelcase. I prefer converting it into camel case.
| @@ -108,34 +101,7 @@ const ExpandedBlock = styled.pre` | |||
| padding: ${size.s} ${size.l}; | |||
| `; | |||
|
|
|||
| export const getResourceRoute = ( | |||
There was a problem hiding this comment.
Moved this function so this file exports the hook only.
There was a problem hiding this comment.
I don't think this change is necessary.
src/components/styles/index.ts
Outdated
| @@ -1,6 +1,6 @@ | |||
| import { Divider } from "./Divider"; | |||
There was a problem hiding this comment.
This file should be lower case and doesn't need the TSX extension. Divider.tsx
| "PASCAL_CASE", | ||
| // tsx exceptions | ||
| "src/**/(use|getFormSchema|index)*.tsx": "CAMEL_CASE", | ||
| }, |
There was a problem hiding this comment.
Can we further trim these down? So we don't have 4 different conventions.
There was a problem hiding this comment.
This is the smallest set that makes sense to me. Let me know what you want to trim and why.
.eslintrc.js
Outdated
| // JS and TS with exceptions | ||
| "src/**/!(vite-env.d|custom-queries)*.{js,ts}": "CAMEL_CASE", | ||
| // All tsx with exceptions | ||
| "src/**/!(use|getFormSchema|index|test-utils|toast-decorator|schemaFields|getColumnsTemplate|githubPRLinkify|jiraLinkify)*.tsx": |
There was a problem hiding this comment.
Should we hardcode all of these exceptions or can we update these to match the pattern we have established?
e.g. jiraLinkify -> JiraLinkify.tsx
There was a problem hiding this comment.
I reviewed these and think these makes sense as is because I think pascal case should be used to represent react components only. Although these files use JSX, they aren't really React components.
githubPRLinkifyandjiraLinkifyshouldn't be React components so they are easily composable.userepresents hooksgetFormSchemasometimes uses JSX but is always destructuredindexisn't generally capitalizedtest-utilsandtoast-decoratoruse JSX but are utils and not componentsgetColumnsTemplatehas the same reasoning asgetFormSchema
EVG-20048