Skip to content

Commit

Permalink
(BSR) docs(PR): add best practices to template
Browse files Browse the repository at this point in the history
  • Loading branch information
cgerrard-pass committed Nov 20, 2024
1 parent 2c8e584 commit 5258b53
Showing 1 changed file with 12 additions and 0 deletions.
12 changes: 12 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
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>

0 comments on commit 5258b53

Please sign in to comment.