This PR mixes various initiatives to improve visibility on sentry **1. Catch errors on workflow jobs** commit [catch workflowTriggerExceptions in job handle](1dbba8c9e2) @thomtrp **2. Fix type in messagingImportExceptionHandler** commit [fix type issue on messagingImportExceptionHandler](919bb3844c) @guillim **3. Catch invalid uuid errors thrown by Postgres by rightfully typing expected id as uuid** commits [use UUIDFilter instead of IDFilter to get graphqlError in case of malformed id](57cc315efe), [use UUIDFilter (2)](304553d770), [fix ids typed as UUID instead of ID](f95d6319cf) @Weiko ⚠️⚠️⚠️ when we deploy this PR we need to flush the schema types from redis as this PR changes them ⚠️⚠️⚠️ **4. Do not group UNKNOWN errors together** commit [do not group unknown errors together](c299b39c8f) Some CustomException classes have introduced UNKNOWN error codes as a default fallback error code. We use CustomException codes to group issues together, but we don't want to do it with UNKNOWN error as they may not have anything in common. For exemple [this sentry for UNKNOWN code](https://twenty-v7.sentry.io/issues/6605750776/events/a72272d8941b4fa2add9b1f39c196d3f/?environment=prod&environment=prod-eu&project=4507072499810304&query=Unknown&referrer=next-event&stream_index=0) groups together "Unknown error importing calendar events for calendar channel...", "Insufficent permissions...", to name a few. **5. Improve postgres error grouping** commit [group together postgres errors](567c25495e) Postgres error are thrown by typeORM as QueryFailedError. we have a lot of them on sentry where they are badly grouped They are currently grouped on sentry according to the stack trace, which leads them to sometimes be grouped even if they don't have anything in common : for exemple [this sentry for QueryFailedError](https://twenty-v7.sentry.io/issues/6563624590/events/2d636821e27a448595b647b4b5a7d6a8/?environment=prod&environment=prod-eu&project=4507072499810304&query=is%3Aunresolved%20%21issue.type%3A%5Bperformance_consecutive_db_queries%2Cperformance_consecutive_http%2Cperformance_file_io_main_thread%2Cperformance_db_main_thread%2Cperformance_n_plus_one_db_queries%2Cperformance_n_plus_one_api_calls%2Cperformance_p95_endpoint_regression%2Cperformance_slow_db_query%2Cperformance_render_blocking_asset_span%2Cperformance_uncompressed_assets%2Cperformance_http_overhead%2Cperformance_large_http_payload%5D%20timesSeen%3A%3E10&referrer=previous-event&sort=date&stream_index=0) groups together "user mapping not found for "postgres" and "invalide type for uuid: 'fallback-id'" to name a few. I attempted to improve the grouping by grouping them with a new custom fingerPrint composed of the [code returned by Postgres](https://www.postgresql.org/docs/current/errcodes-appendix.html) + the truncated operation name (Find, Aggregate, Check...). This is still not ideal as postgres code are quite broad - we could have the same error code for two Find operations with different causes. let's give this a try !
219 lines
5.2 KiB
Markdown
219 lines
5.2 KiB
Markdown
# State Management Guidelines
|
|
|
|
## Core State Management Principles
|
|
Twenty uses a combination of Recoil for global state and Apollo Client for server state management. This document outlines our state management conventions and best practices.
|
|
|
|
## Global State Management
|
|
|
|
### Recoil Usage
|
|
- Use Recoil for global application state
|
|
- Keep atoms small and focused
|
|
```typescript
|
|
// ✅ Correct
|
|
// states/theme.ts
|
|
export const themeState = atom<'light' | 'dark'>({
|
|
key: 'themeState',
|
|
default: 'light',
|
|
});
|
|
|
|
// states/user.ts
|
|
export const userState = atom<User | null>({
|
|
key: 'userState',
|
|
default: null,
|
|
});
|
|
|
|
// ❌ Incorrect
|
|
// states/globalState.ts
|
|
export const globalState = atom({
|
|
key: 'globalState',
|
|
default: {
|
|
theme: 'light',
|
|
user: null,
|
|
settings: {},
|
|
// ... many other unrelated pieces of state
|
|
},
|
|
});
|
|
```
|
|
|
|
### Atom Organization
|
|
- Place atoms in the `states/` directory
|
|
- Group related atoms in feature-specific files
|
|
```typescript
|
|
// states/workspace/atoms.ts
|
|
export const workspaceIdState = atom<string>({
|
|
key: 'workspaceIdState',
|
|
default: '',
|
|
});
|
|
|
|
export const workspaceSettingsState = atom<WorkspaceSettings>({
|
|
key: 'workspaceSettingsState',
|
|
default: defaultSettings,
|
|
});
|
|
```
|
|
|
|
## Server State Management
|
|
|
|
### Apollo Client Usage
|
|
- Use Apollo Client for all GraphQL operations
|
|
- Leverage Apollo's caching capabilities
|
|
```typescript
|
|
// ✅ Correct
|
|
const { data, loading } = useQuery(GET_USER_QUERY, {
|
|
variables: { id },
|
|
fetchPolicy: 'cache-first',
|
|
});
|
|
|
|
// ❌ Incorrect
|
|
const [user, setUser] = useState(null);
|
|
useEffect(() => {
|
|
fetch('/api/user/' + id).then(setUser);
|
|
}, [id]);
|
|
```
|
|
|
|
### Query Organization
|
|
- Separate operation files
|
|
- Use fragments for shared fields
|
|
```typescript
|
|
// queries/user.ts
|
|
export const UserFragment = gql`
|
|
fragment UserFields on User {
|
|
id
|
|
name
|
|
email
|
|
}
|
|
`;
|
|
|
|
export const GET_USER = gql`
|
|
query GetUser($id: UUID!) {
|
|
user(id: $id) {
|
|
...UserFields
|
|
}
|
|
}
|
|
${UserFragment}
|
|
`;
|
|
```
|
|
|
|
## State Management Best Practices
|
|
|
|
### Multiple Small Atoms
|
|
- Prefer multiple small atoms over prop drilling
|
|
- Keep atoms focused on specific features
|
|
```typescript
|
|
// ✅ Correct
|
|
export const selectedViewState = atom<string>({
|
|
key: 'selectedViewState',
|
|
default: '',
|
|
});
|
|
|
|
export const viewFiltersState = atom<ViewFilters>({
|
|
key: 'viewFiltersState',
|
|
default: {},
|
|
});
|
|
|
|
// ❌ Incorrect - Prop drilling
|
|
const ViewContainer = ({ selectedView, filters, onViewChange }) => {
|
|
return (
|
|
<ViewHeader view={selectedView} onViewChange={onViewChange}>
|
|
<ViewContent>
|
|
<ViewFilters filters={filters} />
|
|
</ViewContent>
|
|
</ViewHeader>
|
|
);
|
|
};
|
|
```
|
|
|
|
### No useRef for State
|
|
- Never use useRef for state management
|
|
- Use proper state management tools
|
|
```typescript
|
|
// ✅ Correct
|
|
const [count, setCount] = useState(0);
|
|
// or
|
|
const [count, setCount] = useRecoilState(countState);
|
|
|
|
// ❌ Incorrect
|
|
const countRef = useRef(0);
|
|
```
|
|
|
|
### Data Fetching
|
|
- Extract data fetching to sibling components
|
|
- Keep components focused on presentation
|
|
```typescript
|
|
// ✅ Correct
|
|
const UserProfileContainer = () => {
|
|
const { data, loading } = useQuery(GET_USER);
|
|
if (loading) return <LoadingSpinner />;
|
|
return <UserProfile user={data.user} />;
|
|
};
|
|
|
|
const UserProfile = ({ user }: UserProfileProps) => {
|
|
return <div>{user.name}</div>;
|
|
};
|
|
|
|
// ❌ Incorrect
|
|
const UserProfile = () => {
|
|
const { data, loading } = useQuery(GET_USER);
|
|
if (loading) return <LoadingSpinner />;
|
|
return <div>{data.user.name}</div>;
|
|
};
|
|
```
|
|
|
|
### Hook Usage
|
|
- Use appropriate hooks for state access
|
|
- Choose between useRecoilValue and useRecoilState based on needs
|
|
```typescript
|
|
// ✅ Correct - Read-only access
|
|
const theme = useRecoilValue(themeState);
|
|
|
|
// ✅ Correct - Read-write access
|
|
const [theme, setTheme] = useRecoilState(themeState);
|
|
|
|
// ❌ Incorrect - Using state setter when only reading
|
|
const [theme, _] = useRecoilState(themeState);
|
|
```
|
|
|
|
## Performance Considerations
|
|
|
|
### Selector Usage
|
|
- Use selectors for derived state
|
|
- Memoize complex calculations
|
|
```typescript
|
|
// ✅ Correct
|
|
const filteredUsersState = selector({
|
|
key: 'filteredUsersState',
|
|
get: ({ get }) => {
|
|
const users = get(usersState);
|
|
const filter = get(userFilterState);
|
|
return users.filter(user =>
|
|
user.name.toLowerCase().includes(filter.toLowerCase())
|
|
);
|
|
},
|
|
});
|
|
|
|
// ❌ Incorrect - Calculating in component
|
|
const UserList = () => {
|
|
const users = useRecoilValue(usersState);
|
|
const filter = useRecoilValue(userFilterState);
|
|
const filteredUsers = users.filter(user =>
|
|
user.name.toLowerCase().includes(filter.toLowerCase())
|
|
);
|
|
return <List users={filteredUsers} />;
|
|
};
|
|
```
|
|
|
|
### Cache Management
|
|
- Configure appropriate cache policies
|
|
- Handle cache invalidation properly
|
|
```typescript
|
|
// ✅ Correct
|
|
const [updateUser] = useMutation(UPDATE_USER, {
|
|
update: (cache, { data }) => {
|
|
cache.modify({
|
|
id: cache.identify(data.updateUser),
|
|
fields: {
|
|
name: () => data.updateUser.name,
|
|
},
|
|
});
|
|
},
|
|
});
|
|
``` |