docs(client): document frontend conventions and enforce via ESLint
Capture the five recurring code-review patterns from the recent PR series
(#3591–#3596) as enforceable rules so they don't have to be raised again
in review.
## Conventions
- Prefer MUI native props over `sx` (e.g. color={...}, mt={...}, width={...}).
- Use the full theme path for colors: theme.palette.text.secondary, never
the "text.secondary" shorthand string. The shorthand is invisible to
grep, which makes future audits and refactors miss callsites.
- No hardcoded numeric or string literals — use LAYOUT, typographyLevels,
theme.shape.borderRadius, theme.palette tokens.
- Use useTheme() inside components; don't import { theme } from
@/Utils/Theme/Theme (factory output, not a live mode-aware instance).
- Pair runtime tuples with derived types so they can't drift.
## Files
- docs/frontend-conventions.md — long-form doc with before/after examples
drawn from the actual review fixes, plus a pre-PR checklist.
- CLAUDE.md — short summary in the Code Conventions section pointing to
the doc, so it's loaded into agent context at session start.
## ESLint enforcement
- client/.eslintrc.cjs — four no-restricted-syntax selectors for each rule:
- color="…" / bgcolor="…" / borderColor="…" with a dotted shorthand value
- sx={{ key: ... }} where key has a native equivalent, only on hoistable
components (Box, Stack, Typography, Grid, Container, Paper, Card*,
Divider, Link, Chip) — Dialog/Drawer/Menu etc. only accept sx.
- import { theme } from "@/Utils/Theme/Theme"
- Typography fontSize={13} (the body default — drop the prop).
- TypeScript files are now actually parsed: added @typescript-eslint/parser
and @typescript-eslint/eslint-plugin and a TS override that extends the
React plugin configs. Several @typescript-eslint/recommended rules
disabled to avoid noise on the existing codebase.
- client/package.json — lint script now covers .ts/.tsx; new lint-strict
script for CI use once the codebase is conformant; --max-warnings 0
removed from `lint` so existing-code warnings don't block the workflow.
## Surface
Running `npm run lint` after this change reports ~280 warnings against
the existing codebase, almost all from rule #1 (sx-vs-native). These are
intentionally warnings, not errors — they surface the work without
blocking. Each warning links to the rule number in the conventions doc
so the reasoning is one click away.
This commit is contained in:
1 parent
fe8213f8bb
commit
aa7051095a
5 files changed
+699
-1
No files matched your search
@@ -157,6 +157,14 @@ When working on anything related to check scheduling, incident lifecycle, or not
|
||||
|
||||
## Code Conventions
|
||||
|
||||
### Frontend conventions (mandatory for `client/src`)
|
||||
Read `docs/frontend-conventions.md` before touching any `.tsx` file. Five rules, all enforced in code review:
|
||||
1. Prefer MUI native props over `sx` (e.g. `color={…}`, `bgcolor={…}`, `mt={…}` — not `sx={{ color, bgcolor, mt }}`).
|
||||
2. Use the full theme path for colors: `color={theme.palette.text.secondary}`, never `color="text.secondary"` (greppability).
|
||||
3. No hardcoded literals — use `LAYOUT.*`, `typographyLevels.*`, `theme.shape.borderRadius`, `theme.palette.*`.
|
||||
4. Use `useTheme()` inside components; don't `import { theme } from "@/Utils/Theme/Theme"`.
|
||||
5. Pair runtime tuples with derived types: `const X = [...] as const; type X = (typeof X)[number]`.
|
||||
|
||||
### Internationalization
|
||||
All user-facing strings must use the translation function:
|
||||
```javascript
|
||||
|
||||
@@ -1,3 +1,104 @@
|
||||
// MUI sx keys that have a native (system) prop equivalent.
|
||||
// Flagged when found inside an sx={{ ... }} object literal — see frontend-conventions.md rule #1.
|
||||
const SX_NATIVE_EQUIVALENT_KEYS = [
|
||||
"color",
|
||||
"bgcolor",
|
||||
"backgroundColor",
|
||||
"borderRadius",
|
||||
"border",
|
||||
"borderTop",
|
||||
"borderBottom",
|
||||
"borderLeft",
|
||||
"borderRight",
|
||||
"width",
|
||||
"height",
|
||||
"minHeight",
|
||||
"maxWidth",
|
||||
"minWidth",
|
||||
"maxHeight",
|
||||
"p",
|
||||
"pt",
|
||||
"pb",
|
||||
"px",
|
||||
"py",
|
||||
"pl",
|
||||
"pr",
|
||||
"m",
|
||||
"mt",
|
||||
"mb",
|
||||
"mx",
|
||||
"my",
|
||||
"ml",
|
||||
"mr",
|
||||
"display",
|
||||
"textAlign",
|
||||
"fontWeight",
|
||||
"fontSize",
|
||||
"lineHeight",
|
||||
"gap",
|
||||
];
|
||||
|
||||
// MUI components that accept system props as top-level attributes.
|
||||
// sx={{ color, p, mt, ... }} on these can be hoisted into native props.
|
||||
// Dialog*, Drawer*, Menu*, Tab*, Toolbar etc. only accept sx — don't flag those.
|
||||
const SX_HOISTABLE_COMPONENTS = [
|
||||
"Box",
|
||||
"Stack",
|
||||
"Typography",
|
||||
"Grid",
|
||||
"Container",
|
||||
"Paper",
|
||||
"Card",
|
||||
"CardContent",
|
||||
"CardActions",
|
||||
"CardHeader",
|
||||
"Divider",
|
||||
"Link",
|
||||
"Chip",
|
||||
];
|
||||
|
||||
const sxKeySelector = SX_HOISTABLE_COMPONENTS.flatMap((component) =>
|
||||
SX_NATIVE_EQUIVALENT_KEYS.map(
|
||||
(k) =>
|
||||
`JSXOpeningElement[name.name='${component}'] > JSXAttribute[name.name='sx'] > JSXExpressionContainer > ObjectExpression > Property[key.name='${k}']`
|
||||
)
|
||||
).join(", ");
|
||||
|
||||
const FRONTEND_CONVENTION_RULES = {
|
||||
// Rule #2 — color/bgcolor/borderColor as a string shorthand like "text.secondary".
|
||||
// Catches: <Typography color="text.secondary" /> — should be color={theme.palette.text.secondary}.
|
||||
"no-restricted-syntax": [
|
||||
"warn",
|
||||
{
|
||||
selector:
|
||||
"JSXAttribute[name.name=/^(color|bgcolor|borderColor)$/] > Literal[value=/\\./]",
|
||||
message:
|
||||
'Use the full theme path instead of a string shorthand. e.g. color={theme.palette.text.secondary} not color="text.secondary". See docs/frontend-conventions.md rule #2.',
|
||||
},
|
||||
{
|
||||
// Rule #1 — sx={{ key: ... }} where key has a native prop equivalent.
|
||||
selector: sxKeySelector,
|
||||
message:
|
||||
"Hoist this key out of `sx` onto the component as a native MUI prop (e.g. color={...}, mt={...}, width={...}). See docs/frontend-conventions.md rule #1.",
|
||||
},
|
||||
{
|
||||
// Rule #4 — importing the singleton theme from the Theme module.
|
||||
// Catches: import { theme } from "@/Utils/Theme/Theme" — should be useTheme() inside the component.
|
||||
selector:
|
||||
"ImportDeclaration[source.value='@/Utils/Theme/Theme'] > ImportSpecifier[imported.name='theme']",
|
||||
message:
|
||||
"Don't import { theme } from @/Utils/Theme/Theme — use the useTheme() hook inside the component. See docs/frontend-conventions.md rule #4.",
|
||||
},
|
||||
{
|
||||
// Rule #3 (subset) — Typography fontSize={13} is the body default; drop it.
|
||||
selector:
|
||||
"JSXOpeningElement[name.name='Typography'] JSXAttribute[name.name='fontSize'] > JSXExpressionContainer > Literal[value=13]",
|
||||
message:
|
||||
"fontSize={13} is the Typography body default — drop the prop. For other sizes use typographyLevels tokens. See docs/frontend-conventions.md rule #3.",
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
module.exports = {
|
||||
root: true,
|
||||
env: { browser: true, es2020: true },
|
||||
@@ -15,9 +116,43 @@ module.exports = {
|
||||
"react/jsx-no-target-blank": "off",
|
||||
"react-refresh/only-export-components": ["warn", { allowConstantExport: true }],
|
||||
"react/no-unescaped-entities": "off",
|
||||
...FRONTEND_CONVENTION_RULES,
|
||||
},
|
||||
globals: {
|
||||
__APP_VERSION__: "readonly",
|
||||
process: "readonly",
|
||||
},
|
||||
overrides: [
|
||||
{
|
||||
files: ["**/*.ts", "**/*.tsx"],
|
||||
parser: "@typescript-eslint/parser",
|
||||
parserOptions: { ecmaVersion: "latest", sourceType: "module" },
|
||||
extends: [
|
||||
"eslint:recommended",
|
||||
"plugin:@typescript-eslint/recommended",
|
||||
"plugin:react/recommended",
|
||||
"plugin:react/jsx-runtime",
|
||||
"plugin:react-hooks/recommended",
|
||||
],
|
||||
plugins: ["@typescript-eslint", "react-refresh"],
|
||||
rules: {
|
||||
"react/jsx-no-target-blank": "off",
|
||||
"react-refresh/only-export-components": ["warn", { allowConstantExport: true }],
|
||||
"react/no-unescaped-entities": "off",
|
||||
"no-unused-vars": "off",
|
||||
"@typescript-eslint/no-unused-vars": [
|
||||
"warn",
|
||||
{ argsIgnorePattern: "^_", varsIgnorePattern: "^_" },
|
||||
],
|
||||
// TS handles these; eslint duplicates create noise on existing files.
|
||||
"@typescript-eslint/no-explicit-any": "off",
|
||||
"@typescript-eslint/ban-ts-comment": "off",
|
||||
"@typescript-eslint/ban-types": "off",
|
||||
"@typescript-eslint/no-empty-function": "off",
|
||||
"@typescript-eslint/no-empty-interface": "off",
|
||||
"@typescript-eslint/no-non-null-assertion": "off",
|
||||
...FRONTEND_CONVENTION_RULES,
|
||||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
Generated
+341
@@ -49,6 +49,8 @@
|
||||
"@types/node": "24.5.2",
|
||||
"@types/react": "^18.2.66",
|
||||
"@types/react-dom": "^18.2.22",
|
||||
"@typescript-eslint/eslint-plugin": "^7.18.0",
|
||||
"@typescript-eslint/parser": "^7.18.0",
|
||||
"@vitejs/plugin-react": "^4.2.1",
|
||||
"eslint": "^8.57.0",
|
||||
"eslint-plugin-react": "^7.34.1",
|
||||
@@ -2350,6 +2352,238 @@
|
||||
"version": "0.0.6",
|
||||
"license": "MIT"
|
||||
},
|
||||
"node_modules/@typescript-eslint/eslint-plugin": {
|
||||
"version": "7.18.0",
|
||||
"resolved": "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-7.18.0.tgz",
|
||||
"integrity": "sha512-94EQTWZ40mzBc42ATNIBimBEDltSJ9RQHCC8vc/PDbxi4k8dVwUAv4o98dk50M1zB+JGFxp43FP7f8+FP8R6Sw==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"@eslint-community/regexpp": "^4.10.0",
|
||||
"@typescript-eslint/scope-manager": "7.18.0",
|
||||
"@typescript-eslint/type-utils": "7.18.0",
|
||||
"@typescript-eslint/utils": "7.18.0",
|
||||
"@typescript-eslint/visitor-keys": "7.18.0",
|
||||
"graphemer": "^1.4.0",
|
||||
"ignore": "^5.3.1",
|
||||
"natural-compare": "^1.4.0",
|
||||
"ts-api-utils": "^1.3.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": "^18.18.0 || >=20.0.0"
|
||||
},
|
||||
"funding": {
|
||||
"type": "opencollective",
|
||||
"url": "https://opencollective.com/typescript-eslint"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@typescript-eslint/parser": "^7.0.0",
|
||||
"eslint": "^8.56.0"
|
||||
},
|
||||
"peerDependenciesMeta": {
|
||||
"typescript": {
|
||||
"optional": true
|
||||
}
|
||||
}
|
||||
},
|
||||
"node_modules/@typescript-eslint/parser": {
|
||||
"version": "7.18.0",
|
||||
"resolved": "https://registry.npmjs.org/@typescript-eslint/parser/-/parser-7.18.0.tgz",
|
||||
"integrity": "sha512-4Z+L8I2OqhZV8qA132M4wNL30ypZGYOQVBfMgxDH/K5UX0PNqTu1c6za9ST5r9+tavvHiTWmBnKzpCJ/GlVFtg==",
|
||||
"dev": true,
|
||||
"license": "BSD-2-Clause",
|
||||
"dependencies": {
|
||||
"@typescript-eslint/scope-manager": "7.18.0",
|
||||
"@typescript-eslint/types": "7.18.0",
|
||||
"@typescript-eslint/typescript-estree": "7.18.0",
|
||||
"@typescript-eslint/visitor-keys": "7.18.0",
|
||||
"debug": "^4.3.4"
|
||||
},
|
||||
"engines": {
|
||||
"node": "^18.18.0 || >=20.0.0"
|
||||
},
|
||||
"funding": {
|
||||
"type": "opencollective",
|
||||
"url": "https://opencollective.com/typescript-eslint"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"eslint": "^8.56.0"
|
||||
},
|
||||
"peerDependenciesMeta": {
|
||||
"typescript": {
|
||||
"optional": true
|
||||
}
|
||||
}
|
||||
},
|
||||
"node_modules/@typescript-eslint/scope-manager": {
|
||||
"version": "7.18.0",
|
||||
"resolved": "https://registry.npmjs.org/@typescript-eslint/scope-manager/-/scope-manager-7.18.0.tgz",
|
||||
"integrity": "sha512-jjhdIE/FPF2B7Z1uzc6i3oWKbGcHb87Qw7AWj6jmEqNOfDFbJWtjt/XfwCpvNkpGWlcJaog5vTR+VV8+w9JflA==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"@typescript-eslint/types": "7.18.0",
|
||||
"@typescript-eslint/visitor-keys": "7.18.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": "^18.18.0 || >=20.0.0"
|
||||
},
|
||||
"funding": {
|
||||
"type": "opencollective",
|
||||
"url": "https://opencollective.com/typescript-eslint"
|
||||
}
|
||||
},
|
||||
"node_modules/@typescript-eslint/type-utils": {
|
||||
"version": "7.18.0",
|
||||
"resolved": "https://registry.npmjs.org/@typescript-eslint/type-utils/-/type-utils-7.18.0.tgz",
|
||||
"integrity": "sha512-XL0FJXuCLaDuX2sYqZUUSOJ2sG5/i1AAze+axqmLnSkNEVMVYLF+cbwlB2w8D1tinFuSikHmFta+P+HOofrLeA==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"@typescript-eslint/typescript-estree": "7.18.0",
|
||||
"@typescript-eslint/utils": "7.18.0",
|
||||
"debug": "^4.3.4",
|
||||
"ts-api-utils": "^1.3.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": "^18.18.0 || >=20.0.0"
|
||||
},
|
||||
"funding": {
|
||||
"type": "opencollective",
|
||||
"url": "https://opencollective.com/typescript-eslint"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"eslint": "^8.56.0"
|
||||
},
|
||||
"peerDependenciesMeta": {
|
||||
"typescript": {
|
||||
"optional": true
|
||||
}
|
||||
}
|
||||
},
|
||||
"node_modules/@typescript-eslint/types": {
|
||||
"version": "7.18.0",
|
||||
"resolved": "https://registry.npmjs.org/@typescript-eslint/types/-/types-7.18.0.tgz",
|
||||
"integrity": "sha512-iZqi+Ds1y4EDYUtlOOC+aUmxnE9xS/yCigkjA7XpTKV6nCBd3Hp/PRGGmdwnfkV2ThMyYldP1wRpm/id99spTQ==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": "^18.18.0 || >=20.0.0"
|
||||
},
|
||||
"funding": {
|
||||
"type": "opencollective",
|
||||
"url": "https://opencollective.com/typescript-eslint"
|
||||
}
|
||||
},
|
||||
"node_modules/@typescript-eslint/typescript-estree": {
|
||||
"version": "7.18.0",
|
||||
"resolved": "https://registry.npmjs.org/@typescript-eslint/typescript-estree/-/typescript-estree-7.18.0.tgz",
|
||||
"integrity": "sha512-aP1v/BSPnnyhMHts8cf1qQ6Q1IFwwRvAQGRvBFkWlo3/lH29OXA3Pts+c10nxRxIBrDnoMqzhgdwVe5f2D6OzA==",
|
||||
"dev": true,
|
||||
"license": "BSD-2-Clause",
|
||||
"dependencies": {
|
||||
"@typescript-eslint/types": "7.18.0",
|
||||
"@typescript-eslint/visitor-keys": "7.18.0",
|
||||
"debug": "^4.3.4",
|
||||
"globby": "^11.1.0",
|
||||
"is-glob": "^4.0.3",
|
||||
"minimatch": "^9.0.4",
|
||||
"semver": "^7.6.0",
|
||||
"ts-api-utils": "^1.3.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": "^18.18.0 || >=20.0.0"
|
||||
},
|
||||
"funding": {
|
||||
"type": "opencollective",
|
||||
"url": "https://opencollective.com/typescript-eslint"
|
||||
},
|
||||
"peerDependenciesMeta": {
|
||||
"typescript": {
|
||||
"optional": true
|
||||
}
|
||||
}
|
||||
},
|
||||
"node_modules/@typescript-eslint/typescript-estree/node_modules/brace-expansion": {
|
||||
"version": "2.1.0",
|
||||
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.0.tgz",
|
||||
"integrity": "sha512-TN1kCZAgdgweJhWWpgKYrQaMNHcDULHkWwQIspdtjV4Y5aurRdZpjAqn6yX3FPqTA9ngHCc4hJxMAMgGfve85w==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"balanced-match": "^1.0.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@typescript-eslint/typescript-estree/node_modules/minimatch": {
|
||||
"version": "9.0.9",
|
||||
"resolved": "https://registry.npmjs.org/minimatch/-/minimatch-9.0.9.tgz",
|
||||
"integrity": "sha512-OBwBN9AL4dqmETlpS2zasx+vTeWclWzkblfZk7KTA5j3jeOONz/tRCnZomUyvNg83wL5Zv9Ss6HMJXAgL8R2Yg==",
|
||||
"dev": true,
|
||||
"license": "ISC",
|
||||
"dependencies": {
|
||||
"brace-expansion": "^2.0.2"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=16 || 14 >=14.17"
|
||||
},
|
||||
"funding": {
|
||||
"url": "https://github.com/sponsors/isaacs"
|
||||
}
|
||||
},
|
||||
"node_modules/@typescript-eslint/typescript-estree/node_modules/semver": {
|
||||
"version": "7.7.4",
|
||||
"resolved": "https://registry.npmjs.org/semver/-/semver-7.7.4.tgz",
|
||||
"integrity": "sha512-vFKC2IEtQnVhpT78h1Yp8wzwrf8CM+MzKMHGJZfBtzhZNycRFnXsHk6E5TxIkkMsgNS7mdX3AGB7x2QM2di4lA==",
|
||||
"dev": true,
|
||||
"license": "ISC",
|
||||
"bin": {
|
||||
"semver": "bin/semver.js"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=10"
|
||||
}
|
||||
},
|
||||
"node_modules/@typescript-eslint/utils": {
|
||||
"version": "7.18.0",
|
||||
"resolved": "https://registry.npmjs.org/@typescript-eslint/utils/-/utils-7.18.0.tgz",
|
||||
"integrity": "sha512-kK0/rNa2j74XuHVcoCZxdFBMF+aq/vH83CXAOHieC+2Gis4mF8jJXT5eAfyD3K0sAxtPuwxaIOIOvhwzVDt/kw==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"@eslint-community/eslint-utils": "^4.4.0",
|
||||
"@typescript-eslint/scope-manager": "7.18.0",
|
||||
"@typescript-eslint/types": "7.18.0",
|
||||
"@typescript-eslint/typescript-estree": "7.18.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": "^18.18.0 || >=20.0.0"
|
||||
},
|
||||
"funding": {
|
||||
"type": "opencollective",
|
||||
"url": "https://opencollective.com/typescript-eslint"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"eslint": "^8.56.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@typescript-eslint/visitor-keys": {
|
||||
"version": "7.18.0",
|
||||
"resolved": "https://registry.npmjs.org/@typescript-eslint/visitor-keys/-/visitor-keys-7.18.0.tgz",
|
||||
"integrity": "sha512-cDF0/Gf81QpY3xYyJKDV14Zwdmid5+uuENhjH2EqFaF0ni+yAyq/LzMaIJdhNJXZI7uLzwIlA+V7oWoyn6Curg==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"@typescript-eslint/types": "7.18.0",
|
||||
"eslint-visitor-keys": "^3.4.3"
|
||||
},
|
||||
"engines": {
|
||||
"node": "^18.18.0 || >=20.0.0"
|
||||
},
|
||||
"funding": {
|
||||
"type": "opencollective",
|
||||
"url": "https://opencollective.com/typescript-eslint"
|
||||
}
|
||||
},
|
||||
"node_modules/@ungap/structured-clone": {
|
||||
"version": "1.3.0",
|
||||
"dev": true,
|
||||
@@ -2557,6 +2791,16 @@
|
||||
"url": "https://github.com/sponsors/ljharb"
|
||||
}
|
||||
},
|
||||
"node_modules/array-union": {
|
||||
"version": "2.1.0",
|
||||
"resolved": "https://registry.npmjs.org/array-union/-/array-union-2.1.0.tgz",
|
||||
"integrity": "sha512-HGyxoOTYUyCM6stUe6EJgnd4EoewAI7zMdfqO+kGjnlZmBDz/cR5pf8r/cR4Wq60sL/p0IkcjUEEPwS3GFrIyw==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=8"
|
||||
}
|
||||
},
|
||||
"node_modules/array.prototype.findlast": {
|
||||
"version": "1.2.5",
|
||||
"dev": true,
|
||||
@@ -3272,6 +3516,19 @@
|
||||
"node": ">=6"
|
||||
}
|
||||
},
|
||||
"node_modules/dir-glob": {
|
||||
"version": "3.0.1",
|
||||
"resolved": "https://registry.npmjs.org/dir-glob/-/dir-glob-3.0.1.tgz",
|
||||
"integrity": "sha512-WkrWp9GR4KXfKGYzOLmTuGVi1UWFfws377n9cc55/tb6DuqyF6pcQ5AbiHEshaDpY9v6oaSr2XCDidGmMwdzIA==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"path-type": "^4.0.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=8"
|
||||
}
|
||||
},
|
||||
"node_modules/doctrine": {
|
||||
"version": "3.0.0",
|
||||
"dev": true,
|
||||
@@ -3814,6 +4071,36 @@
|
||||
"node": ">=6.0.0"
|
||||
}
|
||||
},
|
||||
"node_modules/fast-glob": {
|
||||
"version": "3.3.3",
|
||||
"resolved": "https://registry.npmjs.org/fast-glob/-/fast-glob-3.3.3.tgz",
|
||||
"integrity": "sha512-7MptL8U0cqcFdzIzwOTHoilX9x5BrNqye7Z/LuC7kCMRio1EMSyqRK3BEAUD7sXRq4iT4AzTVuZdhgQ2TCvYLg==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"@nodelib/fs.stat": "^2.0.2",
|
||||
"@nodelib/fs.walk": "^1.2.3",
|
||||
"glob-parent": "^5.1.2",
|
||||
"merge2": "^1.3.0",
|
||||
"micromatch": "^4.0.8"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=8.6.0"
|
||||
}
|
||||
},
|
||||
"node_modules/fast-glob/node_modules/glob-parent": {
|
||||
"version": "5.1.2",
|
||||
"resolved": "https://registry.npmjs.org/glob-parent/-/glob-parent-5.1.2.tgz",
|
||||
"integrity": "sha512-AOIgSQCepiJYwP3ARnGx+5VnTu2HBYdzbGP45eLw1vr3zB3vZLeyed1sC9hnbcOc9/SrMyM5RPQrkGz4aS9Zow==",
|
||||
"dev": true,
|
||||
"license": "ISC",
|
||||
"dependencies": {
|
||||
"is-glob": "^4.0.1"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">= 6"
|
||||
}
|
||||
},
|
||||
"node_modules/fast-json-stable-stringify": {
|
||||
"version": "2.1.0",
|
||||
"dev": true,
|
||||
@@ -4170,6 +4457,27 @@
|
||||
"url": "https://github.com/sponsors/ljharb"
|
||||
}
|
||||
},
|
||||
"node_modules/globby": {
|
||||
"version": "11.1.0",
|
||||
"resolved": "https://registry.npmjs.org/globby/-/globby-11.1.0.tgz",
|
||||
"integrity": "sha512-jhIXaOzy1sb8IyocaruWSn1TjmnBVs8Ayhcy83rmxNJ8q2uWKCAj3CnJY+KpGSXCueAPc0i05kVvVKtP1t9S3g==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"array-union": "^2.1.0",
|
||||
"dir-glob": "^3.0.1",
|
||||
"fast-glob": "^3.2.9",
|
||||
"ignore": "^5.2.0",
|
||||
"merge2": "^1.4.1",
|
||||
"slash": "^3.0.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=10"
|
||||
},
|
||||
"funding": {
|
||||
"url": "https://github.com/sponsors/sindresorhus"
|
||||
}
|
||||
},
|
||||
"node_modules/gopd": {
|
||||
"version": "1.2.0",
|
||||
"license": "MIT",
|
||||
@@ -5137,6 +5445,16 @@
|
||||
"node": ">= 0.4"
|
||||
}
|
||||
},
|
||||
"node_modules/merge2": {
|
||||
"version": "1.4.1",
|
||||
"resolved": "https://registry.npmjs.org/merge2/-/merge2-1.4.1.tgz",
|
||||
"integrity": "sha512-8q7VEgMJW4J8tcfVPy8g09NcQwZdbwFEqhe/WZkoIzjn/3TGDwtOCYtXGxA3O8tPzpczCCDgv+P2P5y00ZJOOg==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">= 8"
|
||||
}
|
||||
},
|
||||
"node_modules/micromatch": {
|
||||
"version": "4.0.8",
|
||||
"resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.8.tgz",
|
||||
@@ -6393,6 +6711,16 @@
|
||||
"url": "https://github.com/sponsors/isaacs"
|
||||
}
|
||||
},
|
||||
"node_modules/slash": {
|
||||
"version": "3.0.0",
|
||||
"resolved": "https://registry.npmjs.org/slash/-/slash-3.0.0.tgz",
|
||||
"integrity": "sha512-g9Q1haeby36OSStwb4ntCGGGaKsaVSjQ68fBxoQcutl5fS1vuY18H3wSt3jFyFtrkx+Kz0V1G85A4MyAdDMi2Q==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=8"
|
||||
}
|
||||
},
|
||||
"node_modules/slice-ansi": {
|
||||
"version": "7.1.2",
|
||||
"resolved": "https://registry.npmjs.org/slice-ansi/-/slice-ansi-7.1.2.tgz",
|
||||
@@ -6793,6 +7121,19 @@
|
||||
"node": ">=8.0"
|
||||
}
|
||||
},
|
||||
"node_modules/ts-api-utils": {
|
||||
"version": "1.4.3",
|
||||
"resolved": "https://registry.npmjs.org/ts-api-utils/-/ts-api-utils-1.4.3.tgz",
|
||||
"integrity": "sha512-i3eMG77UTMD0hZhgRS562pv83RC6ukSAC2GMNWc+9dieh/+jDM5u5YG+NHX6VNDRHQcHwmsTHctP9LhbC3WxVw==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=16"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"typescript": ">=4.2.0"
|
||||
}
|
||||
},
|
||||
"node_modules/tslib": {
|
||||
"version": "2.8.1",
|
||||
"license": "0BSD"
|
||||
|
||||
+4
-1
@@ -7,7 +7,8 @@
|
||||
"dev": "vite",
|
||||
"build": "tsc -b && vite build",
|
||||
"build-dev": "tsc -b && vite build --mode development",
|
||||
"lint": "eslint . --ext js,jsx --report-unused-disable-directives --max-warnings 0",
|
||||
"lint": "eslint . --ext js,jsx,ts,tsx --report-unused-disable-directives",
|
||||
"lint-strict": "eslint . --ext js,jsx,ts,tsx --report-unused-disable-directives --max-warnings 0",
|
||||
"preview": "vite preview",
|
||||
"format": "prettier --write .",
|
||||
"format-check": "prettier --check ."
|
||||
@@ -57,6 +58,8 @@
|
||||
"@types/node": "24.5.2",
|
||||
"@types/react": "^18.2.66",
|
||||
"@types/react-dom": "^18.2.22",
|
||||
"@typescript-eslint/eslint-plugin": "^7.18.0",
|
||||
"@typescript-eslint/parser": "^7.18.0",
|
||||
"@vitejs/plugin-react": "^4.2.1",
|
||||
"eslint": "^8.57.0",
|
||||
"eslint-plugin-react": "^7.34.1",
|
||||
|
||||
@@ -0,0 +1,211 @@
|
||||
# Frontend conventions
|
||||
|
||||
Style and structural rules the client codebase follows. These are extracted from review feedback on the PR series #3591–#3596 and are non-negotiable for new code. They exist to keep the codebase greppable, maintainable, and consistent with the existing theme.
|
||||
|
||||
> If you're touching a file in `client/src`, every rule below applies. The "why" matters as much as the "what" — read the reasoning so you can judge edge cases.
|
||||
|
||||
---
|
||||
|
||||
## 1. Prefer MUI native (system) props over `sx`
|
||||
|
||||
MUI components accept layout, spacing, color, border, typography and sizing values as top-level props. Use those. Reach for `sx` only when no native prop exists for the value you need (e.g. `textDecoration`, pseudo-selectors, complex selectors, conditional CSS).
|
||||
|
||||
**Why:** native props are easier to read in JSX, easier to grep, easier to refactor, and don't force the reader to mentally parse a nested object literal for things that are conceptually one attribute. A `sx` block also can't be split-line-edited cleanly when most of its keys are simple primitives.
|
||||
|
||||
**How to apply:** any time you write `sx={{ … }}`, ask whether each key has a native equivalent. If yes, hoist it.
|
||||
|
||||
### Canonical mappings
|
||||
|
||||
| `sx` key | Native prop |
|
||||
|---|---|
|
||||
| `color` | `color` |
|
||||
| `backgroundColor` | `bgcolor` |
|
||||
| `borderRadius` | `borderRadius` |
|
||||
| `border`, `borderTop`, … | `border`, `borderTop`, … |
|
||||
| `width`, `height`, `minHeight`, `maxWidth` | same names as native |
|
||||
| `p`, `pt`, `pb`, `px`, `py` | same names as native |
|
||||
| `m`, `mt`, `mb`, `mx`, `my` | same names as native |
|
||||
| `display` | `display` |
|
||||
| `textAlign` | `textAlign` |
|
||||
| `fontWeight`, `fontSize`, `lineHeight` | same names as native |
|
||||
| `gap` (on Stack) | `gap` |
|
||||
|
||||
### Examples
|
||||
|
||||
```tsx
|
||||
// BAD — everything inside sx
|
||||
<Box
|
||||
sx={{
|
||||
width: "100%",
|
||||
p: theme.spacing(LAYOUT.MD),
|
||||
borderRadius: 1,
|
||||
border: `1px solid ${alpha(tone, 0.45)}`,
|
||||
backgroundColor: alpha(tone, 0.08),
|
||||
textAlign: "left",
|
||||
}}
|
||||
/>
|
||||
|
||||
// GOOD — every key that has a native prop is hoisted
|
||||
<Box
|
||||
width={"100%"}
|
||||
p={theme.spacing(LAYOUT.MD)}
|
||||
borderRadius={theme.shape.borderRadius}
|
||||
border={`1px solid ${alpha(tone, 0.45)}`}
|
||||
bgcolor={alpha(tone, 0.08)}
|
||||
textAlign={"left"}
|
||||
/>
|
||||
```
|
||||
|
||||
```tsx
|
||||
// BAD
|
||||
<Typography sx={{ color: theme.palette.text.primary, lineHeight: 1.55, fontSize: 13 }}>
|
||||
|
||||
// GOOD — hoist what you can; drop fontSize:13 because it's the body default
|
||||
<Typography
|
||||
color={theme.palette.text.primary}
|
||||
lineHeight={1.55}
|
||||
>
|
||||
```
|
||||
|
||||
### When `sx` is still correct
|
||||
|
||||
Keep `sx` for things that don't have a native prop:
|
||||
|
||||
```tsx
|
||||
<Link
|
||||
component={RouterLink}
|
||||
to="/settings"
|
||||
color={theme.palette.primary.main}
|
||||
fontWeight={600}
|
||||
sx={{ textDecoration: "underline" }} // OK — no native prop for textDecoration
|
||||
/>
|
||||
```
|
||||
|
||||
Pseudo-selectors (`"&:hover"`), `transition`, `cursor`, `outline`, `overflow` (when not on Box/Stack), media queries, and conditional spread objects all stay in `sx`.
|
||||
|
||||
---
|
||||
|
||||
## 2. Use the full theme path for colors, never the shorthand string
|
||||
|
||||
Prop forms like `color="text.secondary"`, `color="primary.main"`, `bgcolor="background.paper"` work, but they are **forbidden in this codebase**.
|
||||
|
||||
**Why:** the shorthand is invisible to grep. If a future change needs to find every place that consumes `theme.palette.text.secondary` — to retheme, to swap, to audit — the shorthand strings are missed. That is a maintenance trap. It's worth a few extra characters at every callsite to keep the codebase searchable.
|
||||
|
||||
**How to apply:** always destructure or reference the theme value explicitly.
|
||||
|
||||
```tsx
|
||||
// BAD
|
||||
<Typography color="text.secondary">…</Typography>
|
||||
<Typography color="primary.main">…</Typography>
|
||||
|
||||
// GOOD
|
||||
const theme = useTheme();
|
||||
<Typography color={theme.palette.text.secondary}>…</Typography>
|
||||
<Typography color={theme.palette.primary.main}>…</Typography>
|
||||
```
|
||||
|
||||
This applies to every theme-aware string prop (`color`, `bgcolor`, `borderColor`, etc.), and inside `sx` if you really need it there.
|
||||
|
||||
---
|
||||
|
||||
## 3. No hardcoded numeric or string literals — use tokens
|
||||
|
||||
The theme exposes tokens for every kind of magic value. Use them. Reserve raw numbers for values that are genuinely one-off (and even then, push back on yourself first).
|
||||
|
||||
**Why:** tokens are the single source of truth. A literal `2` in a `mt` prop is invisible — it could mean `LAYOUT.XXS`, a rounding correction, or a thinko. The reader can't tell. With a token, intent is in the name.
|
||||
|
||||
**How to apply:** before writing a literal, check whether one of these tokens covers it. If it nearly does but not exactly, extend the token rather than hardcoding (Alex did exactly this — he added `LAYOUT.XXS = 2` rather than letting `mt: "2px"` stand).
|
||||
|
||||
### Token sources
|
||||
|
||||
- **Spacing / layout** — `client/src/Utils/Theme/constants.ts`
|
||||
- `LAYOUT.XXS = 2` · `LAYOUT.XS = 4` · `LAYOUT.SM = 6` · `LAYOUT.MD = 8` · `LAYOUT.LG = 10` · `LAYOUT.XL = 12` · `LAYOUT.XXL = 16`
|
||||
- `SPACING` for `theme.spacing(...)` multipliers
|
||||
- **Typography sizes** — `client/src/Utils/Theme/Palette.ts` → `typographyLevels`
|
||||
- `xs / s / m / l / xl / xxl` (rem values)
|
||||
- **Hover / interaction** — `HOVER.DARKEN` etc.
|
||||
- **Shape** — `theme.shape.borderRadius` (don't write `borderRadius: 1` or `borderRadius: 4`)
|
||||
- **Colors** — `theme.palette.*` (rule 2 above)
|
||||
|
||||
### Examples
|
||||
|
||||
```tsx
|
||||
// BAD
|
||||
<Box sx={{ mt: "2px" }} />
|
||||
<Stack spacing={theme.spacing(2)} />
|
||||
<Box sx={{ borderRadius: 1 }} />
|
||||
<Typography fontSize={18} />
|
||||
<Typography fontSize={13} /> // 13 is the body default — drop the prop entirely
|
||||
|
||||
// GOOD
|
||||
<Box mt={LAYOUT.XXS} />
|
||||
<Stack spacing={theme.spacing(LAYOUT.XXS)} />
|
||||
<Box borderRadius={theme.shape.borderRadius} />
|
||||
<Typography fontSize={typographyLevels.xl} />
|
||||
<Typography /> // inherits 13px from theme
|
||||
```
|
||||
|
||||
### Token naming gotcha
|
||||
|
||||
`typographyLevels.xl` was historically used for both 18px and 23px callsites. Don't reuse a token for two different purposes — split it (`xl` = 18, `xxl` = 23) and update every callsite. If you find an ambiguous token while editing, fix it the same way.
|
||||
|
||||
---
|
||||
|
||||
## 4. Use `useTheme()` inside components, don't import the theme module
|
||||
|
||||
The `theme` object exported from `@/Utils/Theme/Theme` is the *factory output*, not a live, mode-aware instance. Importing it directly couples a component to the wrong reference and breaks dark-mode reactivity. It also breaks the build at import time in some module-graph configurations (this happened in PR #3596 and required a follow-up commit).
|
||||
|
||||
**Why:** themes can change at runtime (mode toggle), and the hook is the only correct way to read the active theme. Imports also create circular-graph risk because `Theme.ts` itself depends on `Palette.ts`.
|
||||
|
||||
**How to apply:** inside any component or component-helper that needs theme values:
|
||||
|
||||
```tsx
|
||||
// BAD
|
||||
import { theme } from "@/Utils/Theme/Theme";
|
||||
const SectionHeading = ({ children }) => (
|
||||
<Typography color={theme.palette.text.secondary}>{children}</Typography>
|
||||
);
|
||||
|
||||
// GOOD
|
||||
import { useTheme } from "@mui/material/styles";
|
||||
const SectionHeading = ({ children }) => {
|
||||
const theme = useTheme();
|
||||
return <Typography color={theme.palette.text.secondary}>{children}</Typography>;
|
||||
};
|
||||
```
|
||||
|
||||
Module-level utility files that don't render React (pure helpers) should accept `theme` as a parameter rather than importing it.
|
||||
|
||||
---
|
||||
|
||||
## 5. Discriminated unions: pair the runtime list with the type
|
||||
|
||||
When you have a closed set of string literals that's used both as a type and as runtime data (severities, statuses, kinds), declare the runtime tuple first and derive the type from it. Don't declare them as two parallel literals that can drift.
|
||||
|
||||
**Why:** if the list and the type aren't tied to each other, adding a new value to one and forgetting the other compiles fine and breaks at runtime. Deriving the type from the tuple makes them inseparable.
|
||||
|
||||
```tsx
|
||||
// BAD
|
||||
type Severity = "info" | "warning" | "error";
|
||||
|
||||
// GOOD
|
||||
export const Severities = ["info", "warning", "error"] as const;
|
||||
export type Severity = (typeof Severities)[number];
|
||||
```
|
||||
|
||||
Export both when callers need to iterate (e.g. for a Storybook stories matrix or a select dropdown).
|
||||
|
||||
---
|
||||
|
||||
## Pre-PR checklist
|
||||
|
||||
Before you open a PR, scan your diff for these:
|
||||
|
||||
- [ ] Any `sx={{ … }}` block that contains keys with a native-prop equivalent (rule 1)
|
||||
- [ ] Any `color="text.X"`, `bgcolor="background.X"`, or any short-string palette path (rule 2)
|
||||
- [ ] Any raw number or px-string in spacing / margin / padding / fontSize / borderRadius props that should be a token (rule 3)
|
||||
- [ ] Any `import { theme } from "@/Utils/Theme/Theme"` inside a component (rule 4)
|
||||
- [ ] Any `type X = "a" | "b" | "c"` for a closed set used at runtime (rule 5)
|
||||
- [ ] Run `npm run build` and `npm run format-check` in both `client/` and `server/` (per global instructions)
|
||||
|
||||
A clean PR on these axes is a PR that ships without a normalize follow-up.
|
||||
Reference in new issue
Block a user