Migrate to a monorepo structure (#2909)
This commit is contained in:
Charles Bochet
@ -0,0 +1,4 @@
|
||||
{
|
||||
"label": "Advanced",
|
||||
"position": 3
|
||||
}
|
||||
@ -0,0 +1,330 @@
|
||||
---
|
||||
title: Best Practices
|
||||
sidebar_position: 3
|
||||
sidebar_custom_props:
|
||||
icon: TbChecklist
|
||||
---
|
||||
|
||||
This document outlines the best practices you should follow when working on the frontend.
|
||||
|
||||
## State management
|
||||
|
||||
React and Recoil handle state management in the codebase.
|
||||
|
||||
### Use `useRecoilState` to store state
|
||||
|
||||
It's good practice to create as many atoms as you need to store your state.
|
||||
|
||||
:::tip
|
||||
|
||||
It's better to use extra atoms than trying to be too concise with props drilling.
|
||||
|
||||
:::
|
||||
|
||||
```tsx
|
||||
export const myAtomState = atom({
|
||||
key: 'myAtomState',
|
||||
default: 'default value',
|
||||
});
|
||||
|
||||
export const MyComponent = () => {
|
||||
const [myAtom, setMyAtom] = useRecoilState(myAtomState);
|
||||
|
||||
return (
|
||||
<div>
|
||||
<input
|
||||
value={myAtom}
|
||||
onChange={(e) => setMyAtom(e.target.value)}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
```
|
||||
|
||||
### Do not use `useRef` to store state
|
||||
|
||||
Avoid using `useRef` to store state.
|
||||
|
||||
If you want to store state, you should use `useState` or `useRecoilState`.
|
||||
|
||||
See [how to manage re-renders](#managing-re-renders) if you feel like you need `useRef` to prevent some re-renders from happening.
|
||||
|
||||
## Managing re-renders
|
||||
|
||||
Re-renders can be hard to manage in React.
|
||||
|
||||
Here are some rules to follow to avoid unnecessary re-renders.
|
||||
|
||||
Keep in mind that you can **always** avoid re-renders by understanding their cause.
|
||||
|
||||
### Work at the root level
|
||||
|
||||
Avoiding re-renders in new features is now made easy by eliminating them at the root level.
|
||||
|
||||
The `PageChangeEffect` sidecar component contains just one `useEffect` that holds all the logic to execute on a page change.
|
||||
|
||||
That way you know that there's just one place that can trigger a re-render.
|
||||
|
||||
### Always think twice before adding `useEffect` in your codebase
|
||||
|
||||
Re-renders are often caused by unnecessary `useEffect`.
|
||||
|
||||
You should think whether you need `useEffect`, or if you can move the logic in a event handler function.
|
||||
|
||||
You'll find it generally easy to move the logic in a `handleClick` or `handleChange` function.
|
||||
|
||||
You can also find them in libraries like Apollo: `onCompleted`, `onError`, etc.
|
||||
|
||||
### Use a sibling component to extract `useEffect` or data fetching logic
|
||||
|
||||
If you feel like you need to add a `useEffect` in your root component, you should consider extracting it in a sidecar component.
|
||||
|
||||
You can apply the same for data fetching logic, with Apollo hooks.
|
||||
|
||||
```tsx
|
||||
// ❌ Bad, will cause re-renders even if data is not changing,
|
||||
// because useEffect needs to be re-evaluated
|
||||
export const PageComponent = () => {
|
||||
const [data, setData] = useRecoilState(dataState);
|
||||
const [someDependency] = useRecoilState(someDependencyState);
|
||||
|
||||
useEffect(() => {
|
||||
if(someDependency !== data) {
|
||||
setData(someDependency);
|
||||
}
|
||||
}, [someDependency]);
|
||||
|
||||
return <div>{data}</div>;
|
||||
};
|
||||
|
||||
export const App = () => (
|
||||
<RecoilRoot>
|
||||
<PageComponent />
|
||||
</RecoilRoot>
|
||||
);
|
||||
```
|
||||
|
||||
```tsx
|
||||
// ✅ Good, will not cause re-renders if data is not changing,
|
||||
// because useEffect is re-evaluated in another sibling component
|
||||
export const PageComponent = () => {
|
||||
const [data, setData] = useRecoilState(dataState);
|
||||
|
||||
return <div>{data}</div>;
|
||||
};
|
||||
|
||||
export const PageData = () => {
|
||||
const [data, setData] = useRecoilState(dataState);
|
||||
const [someDependency] = useRecoilState(someDependencyState);
|
||||
|
||||
useEffect(() => {
|
||||
if(someDependency !== data) {
|
||||
setData(someDependency);
|
||||
}
|
||||
}, [someDependency]);
|
||||
|
||||
return <></>;
|
||||
};
|
||||
|
||||
export const App = () => (
|
||||
<RecoilRoot>
|
||||
<PageData />
|
||||
<PageComponent />
|
||||
</RecoilRoot>
|
||||
);
|
||||
```
|
||||
|
||||
### Use recoil family states and recoil family selectors
|
||||
|
||||
Recoil family states and selectors are a great way to avoid re-renders.
|
||||
|
||||
They are useful when you need to store a list of items.
|
||||
|
||||
### You shouldn't use `React.memo(MyComponent)`
|
||||
|
||||
Avoid using `React.memo()` because it does not solve the cause of the re-render, but instead breaks the re-render chain, which can lead to unexpected behavior and make the code very hard to refactor.
|
||||
|
||||
### Limit `useCallback` or `useMemo` usage
|
||||
|
||||
They are often not necessary and will make the code harder to read and maintain for a gain of performance that is unnoticeable.
|
||||
|
||||
## Console.logs
|
||||
|
||||
`console.log` statements are invaluable during development, offering real-time insights into variable values and code flow. But, leaving them in production code can lead to several issues:
|
||||
|
||||
1. **Performance**: Excessive logging can affect the runtime performance, specially on client-side applications.
|
||||
|
||||
2. **Security**: Logging sensitive data can expose critical information to anyone who inspects the browser's console.
|
||||
|
||||
3. **Cleanliness**: Filling up the console with logs can obscure important warnings or errors that developers or tools need to see.
|
||||
|
||||
4. **Professionalism**: End users or clients checking the console and seeing a myriad of log statements might question the code's quality and polish.
|
||||
|
||||
Make sure you remove all `console.logs` before pushing the code to production.
|
||||
|
||||
## Naming
|
||||
|
||||
### Variable Naming
|
||||
|
||||
Variable names ought to precisely depict the purpose or function of the variable.
|
||||
|
||||
#### The issue with generic names
|
||||
Generic names in programming are not ideal because they lack specificity, leading to ambiguity and reduced code readability. Such names fail to convey the variable or function's purpose, making it challenging for developers to understand the code's intent without deeper investigation. This can result in increased debugging time, higher susceptibility to errors, and difficulties in maintenance and collaboration. Meanwhile, descriptive naming makes the code self-explanatory and easier to navigate, enhancing code quality and developer productivity.
|
||||
|
||||
```tsx
|
||||
// ❌ Bad, uses a generic name that doesn't communicate its
|
||||
// purpose or content clearly
|
||||
const [value, setValue] = useState('');
|
||||
```
|
||||
|
||||
```tsx
|
||||
// ✅ Good, uses a descriptive name
|
||||
const [email, setEmail] = useState('');
|
||||
```
|
||||
|
||||
#### Some words to avoid in variable names
|
||||
|
||||
- dummy
|
||||
|
||||
### Event handlers
|
||||
|
||||
Event handler names should start with `handle`, while `on` is a prefix used to name events in components props.
|
||||
|
||||
```tsx
|
||||
// ❌ Bad
|
||||
const onEmailChange = (val: string) => {
|
||||
// ...
|
||||
};
|
||||
```
|
||||
|
||||
```tsx
|
||||
// ✅ Good
|
||||
const handleEmailChange = (val: string) => {
|
||||
// ...
|
||||
};
|
||||
```
|
||||
|
||||
## Optional Props
|
||||
|
||||
Avoid passing the default value for an optional prop.
|
||||
|
||||
**EXAMPLE**
|
||||
|
||||
Take the`EmailField` component defined below:
|
||||
|
||||
```tsx
|
||||
type EmailFieldProps = {
|
||||
value: string;
|
||||
disabled?: boolean;
|
||||
};
|
||||
|
||||
const EmailField = ({ value, disabled = false }: EmailFieldProps) => (
|
||||
<TextInput value={value} disabled={disabled} fullWidth />
|
||||
);
|
||||
```
|
||||
|
||||
**Usage**
|
||||
|
||||
```tsx
|
||||
// ❌ Bad, passing in the same value as the default value adds no value
|
||||
const Form = () => <EmailField value="username@email.com" disabled={false} />;
|
||||
```
|
||||
|
||||
```tsx
|
||||
// ✅ Good, assumes the default value
|
||||
const Form = () => <EmailField value="username@email.com" />;
|
||||
```
|
||||
|
||||
## Component as props
|
||||
|
||||
Try as much as possible to pass uninstantiated components as props, so children can decide on their own of what props they need to pass.
|
||||
|
||||
The most common example for that is icon components:
|
||||
|
||||
```tsx
|
||||
const SomeParentComponent = () => <MyComponent Icon={MyIcon} />;
|
||||
|
||||
// In MyComponent
|
||||
const MyComponent = ({ MyIcon }: { MyIcon: IconComponent }) => {
|
||||
const theme = useTheme();
|
||||
|
||||
return (
|
||||
<div>
|
||||
<MyIcon size={theme.icon.size.md}>
|
||||
</div>
|
||||
)
|
||||
};
|
||||
```
|
||||
|
||||
For React to understand that the component is a component, you need to use PascalCase, to later instantiate it with `<MyIcon>`
|
||||
|
||||
## Prop Drilling: Keep It Minimal
|
||||
|
||||
Prop drilling, in the React context, refers to the practice of passing state variables and their setters through many component layers, even if intermediary components don't use them. While sometimes necessary, excessive prop drilling can lead to:
|
||||
|
||||
1. **Decreased Readability**: Tracing where a prop originates or where it's utilized can become convoluted in a deeply nested component structure.
|
||||
|
||||
2. **Maintenance Challenges**: Changes in one component's prop structure might require adjustments in several components, even if they don't directly use the prop.
|
||||
|
||||
3. **Reduced Component Reusability**: A component receiving a lot of props solely for passing them down becomes less general-purpose and harder to reuse in different contexts.
|
||||
|
||||
If you feel that you are using excessive prop drilling, see [state management best practices](/contributor/frontend/advanced/best-practices#state-management).
|
||||
|
||||
## Imports
|
||||
|
||||
When importing, opt for the designated aliases rather than specifying complete or relative paths.
|
||||
|
||||
**The Aliases**
|
||||
|
||||
```js
|
||||
{
|
||||
alias: {
|
||||
"~": path.resolve(__dirname, "src"),
|
||||
"@": path.resolve(__dirname, "src/modules"),
|
||||
"@testing": path.resolve(__dirname, "src/testing"),
|
||||
},
|
||||
}
|
||||
```
|
||||
|
||||
**Usage**
|
||||
```tsx
|
||||
// ❌ Bad, specifies the entire relative path
|
||||
import {
|
||||
CatalogDecorator
|
||||
} from '../../../../../testing/decorators/CatalogDecorator';
|
||||
import {
|
||||
ComponentDecorator
|
||||
} from '../../../../../testing/decorators/ComponentDecorator';
|
||||
```
|
||||
|
||||
```tsx
|
||||
// ✅ Good, utilises the designated aliases
|
||||
import { CatalogDecorator } from '~/testing/decorators/CatalogDecorator';
|
||||
import { ComponentDecorator } from '~/testing/decorators/ComponentDecorator';=
|
||||
```
|
||||
|
||||
## Schema Validation
|
||||
|
||||
[Zod](https://github.com/colinhacks/zod) is the schema validator for untyped objects:
|
||||
|
||||
```js
|
||||
const validationSchema = z
|
||||
.object({
|
||||
exist: z.boolean(),
|
||||
email: z
|
||||
.string()
|
||||
.email('Email must be a valid email'),
|
||||
password: z
|
||||
.string()
|
||||
.regex(PASSWORD_REGEX, 'Password must contain at least 8 characters'),
|
||||
})
|
||||
.required();
|
||||
|
||||
type Form = z.infer<typeof validationSchema>;
|
||||
```
|
||||
|
||||
|
||||
## Breaking Changes
|
||||
|
||||
Always perform thorough manual testing before proceeding to guarantee that modifications haven’t caused disruptions elsewhere, given that tests have not yet been extensively integrated.
|
||||
|
||||
@ -0,0 +1,23 @@
|
||||
---
|
||||
title: Hotkeys
|
||||
sidebar_position: 11
|
||||
sidebar_custom_props:
|
||||
icon: TbKeyboard
|
||||
---
|
||||
|
||||
You can intercept any hotkey combination and execute a custom action.
|
||||
|
||||
There's a thin wrapper on top of [react-hotkeys-hook](https://react-hotkeys-hook.vercel.app/docs/intro) that makes it more performant and avoids unnecessary re-renders.
|
||||
|
||||
There's also a wrapper hook `useScopedHotkeys` that makes it easy to manage scopes.
|
||||
|
||||
```ts
|
||||
useScopedHotkeys(
|
||||
'ctrl+k,meta+k',
|
||||
() => {
|
||||
openCommandMenu();
|
||||
},
|
||||
AppHotkeyScope.CommandMenu,
|
||||
[openCommandMenu],
|
||||
);
|
||||
```
|
||||
@ -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