17 Frontend code reviews in real project (react, typescript)
Tags: javascript, typescript, beginners, react
Published: 2023-04-25 07:33:11

mainly javascript/typescript and react, let's get start it

## 💎 1. Use `date-fns` instead of `moment`
moment weights 241kb which is so heavy, and before checking this PR, I've known that `date-fns` exists in this project so proposed this one.
Besides in our slack, I found engineers had already talked about it even long time ago, like `We wanna remove moment someday` 

you can use `no-restricted-imports` option of ESLint in your project [just like this](https://calibreapp.com/blog/bundle-size-optimization) 

## 💎 2. Don't use magic number
there was like this code
```javascript
const handleHogeChange = () => {
  setIsOpen(false)
  setTimeout(() => {
    // do something
  }, 170)
}
```
magic number in this code is `170`, if other engineer see this code at first time, they couldn't get what it is asap.
↓
```javascript
const HOGE_TRANSITION = 170

const handleHogeChange = () => {
  setIsOpen(false)
  setTimeout(() => {
    // do something
  }, HOGE_TRANSITION)
}
```
(actually this `HOGE_TRANSITION` is assigned in other const define file, but now I just wrote like that to get easily)

## 💎 3. DRY
**Don't Repeat Yourself**
If you don't know this one, you would find easily on the internet what it is

## 💎 4. Don't pass useState to child component
when you want to change state in child component, you shouldn't pass `useState` to child component directory. Because it makes so hard to get in which component this state is controlled.

↓ so just use callback function like this
https://www.webtips.dev/solutions/pass-data-from-child-to-parent-in-react

## 💎 5. Don't use type `any` in event argument in React
I found like this code
```typescript
onClick={(e: any) => handleActionClick(e, action.id)}
```
and I thought engineer have tried code like that once

![Image description](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/kktj0xstpddc89p4pgjx.png)

but as you can see error appeared and he gave up.
(just my imagination though lol)

React prepares own event for support cross browser, so you should use it
https://reactjs.org/docs/events.html#mouse-events

```typescript
onClick={(e: React.MouseEvent) => handleActionClick(e, action.id)}
```

by the way, library `preact` which is similar to react doesn't implement like this code. It's one of the reason why it's lighter than react
https://preactjs.com/guide/v8/differences-to-react/
> Synthetic Events: Preact's browser support target does not require this extra overhead

## 💎 6. Avoid hard coding
What is hard coding?
it means practice of embedding values directly into the source code of a program instead of using variables or other abstractions
```javascript
useState('admin_user')
```
↓
```javascript
const ADMIN_USER = 'admin_user'
useState(ADMIN_USER)
```
it is too simple to get for what I defined variable, but if you write code in big project, it would cause bug.

When I see hard code in project, my brain reacts instinctively like "🤨🤨🤨🤨🤨🤨🤨 hmmmmmmmm", even in train, bathroom, toilet, bed. So avoid it please lol

## 💎 7. UseState() of toggle button
```javascript
// write in this way, it's simpler than write "true" and "false" each time
useState(prev => !prev)
```

## 💎 8. Use object not case in like this condition
I've modified like this way ↓
[when conditionally rendering component based on some string enum (like a role), use an object to map enum values to components.](https://mobile.twitter.com/DavidKPiano/status/1564950527477252098)

## 💎 9. Use meaningful Names
```javascript
// (before)
const m = moment(value, valueFormat[type])
```
↓
```javascript
// (after)
const inputDate = moment(value, valueFormat[type])
```

## 💎 10. Set default argument value, if it is possible. Specially in utility function

I found like this code about debounce
```javascript
const DELAY = 200
.
.
.
const debouncedQuery = useDebounce(name, DELAY)
```
I was like "What is 200? Where is it from??", this related to **Magic number** which I've already written before.
After searching using places `debouncedQuery`, I realised the reason why he wrote 200 is that `DELAY = 200` is written in 5 other files as well.
So I proposed adding default argument in useDebounce function like this
```javascript
const useDebounce = (value, delay = 200) => {
....(some codes)
}
```

## 💎 11. Don't set JSX in useState value
I found like this code, but JSX shouldn't be managed in useState.
React decides to re-render or not by observing state(=data), but if JSX which is result of rendering sets in state, it would be like destroying react concept itself.
It works without any problem though

```javascript
const [hogeComponent, setHogeComponent] = React.useState<JSX.Element>(hogeComponentRenderer)

```


## 💎 12. Add meaningful name in if condition
Actually it's not only if condition, but people somehow add meaningless name in if condition easily
```javascript
// 🤔 What's that ...?
if(template.id > 0) {
...
}
```
↓
```javascript
const existsTemplate = template.id > 0
if(existsTemplate) {
...
}
```

## 💎 13. Don't use negative condition, if you don't have reason
```javascript
!isAdmin ? first() : second() 
```
↓
```javascript
isAdmin ? second() : first()
```

## 💎 14. Eliminate unnecessary rendering 
I've often seen people forgot adding `useCallback` in proper places.
Or one one big object of useState manages a lot of form.
If I found unnecessary rendering components which are caused by `onChange` event, I would warn it. It could be heavy action for browser.  

### How I find it?
I use[React developper tools](https://chrome.google.com/webstore/detail/react-developer-tools/fmkadmapgofadopljbjfkapdkoienihi?hl=en) in chrome.
↓ Check **Highlight updates when components render**, and you can see which component render
![Image description](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/3ctzdxfj732m0bgd41gk.png)

## 💎 15. use debounce
For example, when you implement `input` which request to server each `onChange` event, you have to add debounce. Otherwise, you know server cries.

### In my case
In my project, I've found like this input(↑), and debounce time was 200ms. But Even if I wrote so quickly, requests were sent on each onChange event. So I proposed changing this time to 500ms

There are some examples, which you need to add debounce like animation, changing window size ....

## 💎 16. Use controlled and uncontrolled component depending on situation 
When the form appears in code, I start to think about it.
Simply put ↓
```
- controlled
  you need to watch and render on every onChange event
- uncontrolled
  1. you don't need to watch and render on every onChange event, in case of simple form
  2. If user can add number of form, also you should use uncontrolled
```

## 💎 17. Don't manipulate DOM directly without special reason
I found like this code
```javascript
const currentVal = form.querySelector('input[type="text"]').value
```
If you need to manipulate DOM, you should use [useRef](https://react.dev/reference/react/useRef#useref) in react.js.
Specially when you need to change/get something that has no related to react life cycle, for example *getting dom width, height or scroll height*

You shouldn't use useRef only because you want to change some data

## 💎 (I will keep writing each time when I find interesting code review)
.
.
.
😀