Improve Documentation (#3795)
* Begin docs improvement * Keep improving documentation * Upgrade Docusarus * Fix broken links
This commit is contained in:
Félix Malfait
291
packages/twenty-docs/docs/contributor/frontend/style-guide.mdx
Normal file
291
packages/twenty-docs/docs/contributor/frontend/style-guide.mdx
Normal file
@ -0,0 +1,291 @@
|
||||
---
|
||||
title: Style Guide
|
||||
sidebar_position: 4
|
||||
sidebar_custom_props:
|
||||
icon: TbPencil
|
||||
---
|
||||
|
||||
This document includes the rules to follow when writing code.
|
||||
|
||||
The goal here is to have a consistent codebase, which is easy to read and easy to maintain.
|
||||
|
||||
For this, it's better to be a bit more verbose than to be too concise.
|
||||
|
||||
Always keep in mind that people read code more often than they write it, specially on an open source project, where anyone can contribute.
|
||||
|
||||
There are a lot of rules that are not defined here, but that are automatically checked by linters.
|
||||
|
||||
## React
|
||||
|
||||
### Use functional components
|
||||
|
||||
Always use TSX functional components.
|
||||
|
||||
Do not use default `import` with `const`, because it's harder to read and harder to import with code completion.
|
||||
|
||||
```tsx
|
||||
// ❌ Bad, harder to read, harder to import with code completion
|
||||
const MyComponent = () => {
|
||||
return <div>Hello World</div>;
|
||||
};
|
||||
|
||||
export default MyComponent;
|
||||
|
||||
// ✅ Good, easy to read, easy to import with code completion
|
||||
export function MyComponent() {
|
||||
return <div>Hello World</div>;
|
||||
};
|
||||
```
|
||||
|
||||
### Props
|
||||
|
||||
Create the type of the props and call it `(ComponentName)Props` if there's no need to export it.
|
||||
|
||||
Use props destructuring.
|
||||
|
||||
```tsx
|
||||
// ❌ Bad, no type
|
||||
export const MyComponent = (props) => <div>Hello {props.name}</div>;
|
||||
|
||||
// ✅ Good, type
|
||||
type MyComponentProps = {
|
||||
name: string;
|
||||
};
|
||||
|
||||
export const MyComponent = ({ name }: MyComponentProps) => <div>Hello {name}</div>;
|
||||
```
|
||||
|
||||
#### Refrain from using `React.FC` or `React.FunctionComponent` to define prop types
|
||||
|
||||
```tsx
|
||||
/* ❌ - Bad, defines the component type annotations with `FC`
|
||||
* - With `React.FC`, the component implicitly accepts a `children` prop
|
||||
* even if it's not defined in the prop type. This might not always be
|
||||
* desirable, especially if the component doesn't intend to render
|
||||
* children.
|
||||
*/
|
||||
const EmailField: React.FC<{
|
||||
value: string;
|
||||
}> = ({ value }) => <TextInput value={value} disabled fullWidth />;
|
||||
```
|
||||
|
||||
```tsx
|
||||
/* ✅ - Good, a separate type (OwnProps) is explicitly defined for the
|
||||
* component's props
|
||||
* - This method doesn't automatically include the children prop. If
|
||||
* you want to include it, you have to specify it in OwnProps.
|
||||
*/
|
||||
type EmailFieldProps = {
|
||||
value: string;
|
||||
};
|
||||
|
||||
const EmailField = ({ value }: EmailFieldProps) => (
|
||||
<TextInput value={value} disabled fullWidth />
|
||||
);
|
||||
```
|
||||
|
||||
#### No Single Variable Prop Spreading in JSX Elements
|
||||
|
||||
Avoid using single variable prop spreading in JSX elements, like `{...props}`. This practice often results in code that is less readable and harder to maintain because it's unclear which props the component is receiving.
|
||||
|
||||
```tsx
|
||||
/* ❌ - Bad, spreads a single variable prop into the underlying component
|
||||
*/
|
||||
const MyComponent = (props: OwnProps) => {
|
||||
return <OtherComponent {...props} />;
|
||||
}
|
||||
```
|
||||
|
||||
```tsx
|
||||
/* ✅ - Good, Explicitly lists all props
|
||||
* - Enhances readability and maintainability
|
||||
*/
|
||||
const MyComponent = ({ prop1, prop2, prop3 }: MyComponentProps) => {
|
||||
return <OtherComponent {...{ prop1, prop2, prop3 }} />;
|
||||
};
|
||||
```
|
||||
|
||||
Rationale:
|
||||
- At a glance, it's clearer which props the code passes down, making it easier to understand and maintain.
|
||||
- It helps to prevent tight coupling between components via their props.
|
||||
- Linting tools make it easier to identify misspelled or unused props when you list props explicitly.
|
||||
|
||||
## JavaScript
|
||||
|
||||
### Use nullish-coalescing operator `??`
|
||||
|
||||
```tsx
|
||||
// ❌ Bad, can return 'default' even if value is 0 or ''
|
||||
const value = process.env.MY_VALUE || 'default';
|
||||
|
||||
// ✅ Good, will return 'default' only if value is null or undefined
|
||||
const value = process.env.MY_VALUE ?? 'default';
|
||||
```
|
||||
|
||||
### Use optional chaining `?.`
|
||||
|
||||
```tsx
|
||||
// ❌ Bad
|
||||
onClick && onClick();
|
||||
|
||||
// ✅ Good
|
||||
onClick?.();
|
||||
```
|
||||
|
||||
## TypeScript
|
||||
|
||||
### Use `type` instead of `interface`
|
||||
|
||||
Always use `type` instead of `interface`, because they almost always overlap, and `type` is more flexible.
|
||||
|
||||
```tsx
|
||||
// ❌ Bad
|
||||
interface MyInterface {
|
||||
name: string;
|
||||
}
|
||||
|
||||
// ✅ Good
|
||||
type MyType = {
|
||||
name: string;
|
||||
};
|
||||
```
|
||||
|
||||
### Use string literals instead of enums
|
||||
|
||||
[String literals](https://www.typescriptlang.org/docs/handbook/2/everyday-types.html#literal-types) are the go-to way to handle enum-like values in TypeScript. They are easier to extend with Pick and Omit, and offer a better developer experience, specially with code completion.
|
||||
|
||||
You can see why TypeScript recommends avoiding enums [here](https://www.typescriptlang.org/docs/handbook/2/everyday-types.html#enums).
|
||||
|
||||
```tsx
|
||||
// ❌ Bad, utilizes an enum
|
||||
enum Color {
|
||||
Red = "red",
|
||||
Green = "green",
|
||||
Blue = "blue",
|
||||
}
|
||||
|
||||
let color = Color.Red;
|
||||
```
|
||||
|
||||
```tsx
|
||||
// ✅ Good, utilizes a string literal
|
||||
let color: "red" | "green" | "blue" = "red";
|
||||
```
|
||||
|
||||
#### GraphQL and internal libraries
|
||||
|
||||
You should use enums that GraphQL codegen generates.
|
||||
|
||||
It's also better to use an enum when using an internal library, so the internal library doesn't have to expose a string literal type that is not related to the internal API.
|
||||
|
||||
Example:
|
||||
|
||||
```TSX
|
||||
const {
|
||||
setHotkeyScopeAndMemorizePreviousScope,
|
||||
goBackToPreviousHotkeyScope,
|
||||
} = usePreviousHotkeyScope();
|
||||
|
||||
setHotkeyScopeAndMemorizePreviousScope(
|
||||
RelationPickerHotkeyScope.RelationPicker,
|
||||
);
|
||||
```
|
||||
|
||||
## Styling
|
||||
|
||||
### Use StyledComponents
|
||||
|
||||
Style the components with [styled-components](https://emotion.sh/docs/styled).
|
||||
|
||||
```tsx
|
||||
// ❌ Bad
|
||||
<div className="my-class">Hello World</div>
|
||||
```
|
||||
|
||||
```tsx
|
||||
// ✅ Good
|
||||
const StyledTitle = styled.div`
|
||||
color: red;
|
||||
`;
|
||||
```
|
||||
|
||||
Prefix styled components with "Styled" to differentiate them from "real" components.
|
||||
|
||||
```tsx
|
||||
// ❌ Bad
|
||||
const Title = styled.div`
|
||||
color: red;
|
||||
`;
|
||||
```
|
||||
|
||||
```tsx
|
||||
// ✅ Good
|
||||
const StyledTitle = styled.div`
|
||||
color: red;
|
||||
`;
|
||||
```
|
||||
|
||||
### Theming
|
||||
|
||||
Utilizing the theme for the majority of component styling is the preferred approach.
|
||||
|
||||
#### Units of measurement
|
||||
|
||||
Avoid using `px` or `rem` values directly within the styled components. The necessary values are generally already defined in the theme, so it’s recommended to make use of the theme for these purposes.
|
||||
|
||||
#### Colors
|
||||
|
||||
Refrain from introducing new colors; instead, use the existing palette from the theme. Should there be a situation where the palette does not align, please leave a comment so that the team can rectify it.
|
||||
|
||||
|
||||
```tsx
|
||||
// ❌ Bad, directly specifies style values without utilizing the theme
|
||||
const StyledButton = styled.button`
|
||||
color: #333333;
|
||||
font-size: 1rem;
|
||||
font-weight: 400;
|
||||
margin-left: 4px;
|
||||
border-radius: 50px;
|
||||
`;
|
||||
```
|
||||
|
||||
```tsx
|
||||
// ✅ Good, utilizes the theme
|
||||
const StyledButton = styled.button`
|
||||
color: ${({ theme }) => theme.font.color.primary};
|
||||
font-size: ${({ theme }) => theme.font.size.md};
|
||||
font-weight: ${({ theme }) => theme.font.weight.regular};
|
||||
margin-left: ${({ theme }) => theme.spacing(1)};
|
||||
border-radius: ${({ theme }) => theme.border.rounded};
|
||||
`;
|
||||
```
|
||||
## Enforcing No-Type Imports
|
||||
|
||||
Avoid type imports. To enforce this standard, an ESLint rule checks for and reports any type imports. This helps maintain consistency and readability in the TypeScript code.
|
||||
|
||||
```tsx
|
||||
// ❌ Bad
|
||||
import { type Meta, type StoryObj } from '@storybook/react';
|
||||
|
||||
// ❌ Bad
|
||||
import type { Meta, StoryObj } from '@storybook/react';
|
||||
|
||||
// ✅ Good
|
||||
import { Meta, StoryObj } from '@storybook/react';
|
||||
```
|
||||
|
||||
### Why No-Type Imports
|
||||
|
||||
- **Consistency**: By avoiding type imports and using a single approach for both type and value imports, the codebase remains consistent in its module import style.
|
||||
|
||||
- **Readability**: No-type imports improve code readability by making it clear when you're importing values or types. This reduces ambiguity and makes it easier to understand the purpose of imported symbols.
|
||||
|
||||
- **Maintainability**: It enhances codebase maintainability because developers can identify and locate type-only imports when reviewing or modifying code.
|
||||
|
||||
### ESLint Rule
|
||||
|
||||
An ESLint rule, `@typescript-eslint/consistent-type-imports`, enforces the no-type import standard. This rule will generate errors or warnings for any type import violations.
|
||||
|
||||
Please note that this rule specifically addresses rare edge cases where unintentional type imports occur. TypeScript itself discourages this practice, as mentioned in the [TypeScript 3.8 release notes](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html). In most situations, you should not need to use type-only imports.
|
||||
|
||||
To ensure your code complies with this rule, make sure to run ESLint as part of your development workflow.
|
||||
Reference in New Issue
Block a user