Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

(BSR) docs(PR): add best practices to template #7230

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+12 −0
Open

(BSR) docs(PR): add best practices to template #7230

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,23 @@ I have:
<details>
<summary>Click to expand</summary>

- Remove type assertions with `as` (type assertions are removed at compile-time, there is no runtime checking associated with a type assertion. There won’t be an exception or `null` generated if the type assertion is wrong). In certain cases `as const` is acceptable (for example when defining readonly arrays/objects). **MUST BE DISCUSSED: using `as` in tests**
- Remove bypass type checking with `any` (when you want to accept anything because you will be blindly passing it through without interacting with it, you can use `unknown`). **MUST BE DISCUSSED: when defining functions that can take literally anything, it would seem that it is not practical to use `unknown`. Also we should discuss the usage on `any` in tests.**
- Remove non-null assertion operators (just like other type assertions, this doesn’t change the runtime behavior of your code, so it’s important to only use `!` when you know that the value can’t be `null` or `undefined`).
- When creating a new component, start by creating it in StoryBook. **MUST BE DISCUSSED: what do we consider a component? Should all components be in StoryBook?**
- When creating a new logic, start by writing tests (TDD). **MUST BE DISCUSSED**
- Remove all `@ts-expect-error` and `@eslint-disable`. If not avoidable, specify why with a comment. **MUST BE DISCUSSED**
- Remove all warnings, and "normal errors" (`yarn test:lint`, `yarn test:types`, `yarn start:web`...). **MUST BE DISCUSSED**
- Write tests for existing code if there were none. **MUST BE DISCUSSED**
- Use `gap` instead of `<Spacer.Column />` or `<Spacer.Row />` when it is more optimal. **MUST BE DISCUSSED**

Test specific:

- Mock external calls, but don't mock internal parts of our code. **MUST BE DISCUSSED**
- When you see a local variable that is over-written in every test, mock it.
- Prefer `user` to `fireEvent`.
- Replace `await waitFor(/* ... */)` or `await act(async () => {})` by `await screen.findByTruc()`. **MUST BE DISCUSSED**
- Add a snapshot test for pages and modals ONLY. **MUST BE DISCUSSED: snapshots for web?**
- Add an a11y test for the web. **MUST BE DISCUSSED**

</details>
Loading